From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id B9F3C8BCDDD; Mon, 4 Dec 2023 16:27:11 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B9F3C8BCDDD DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1701696431; bh=eAIaHl6PHOqevLfKX5qWUVFeC1C65oTpLlaJUKWBbI0=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=R643hTPQndDwfQb53xKWCZgfz0Lj9nmuKZdUYT9IH9vdlIjC+Mb0Et5s+zry3kgRz 6Pd7ny/kmkvCDNAwRBNvIW54JzkUlgUSvdm4p0hMF507dVocAeRtg8BV0KCvswQwXN 4UI6zCvcNfBM5vEO3UV8h1y6A4G28IVxpE+gawkY= Received: from mail-lf1-f53.google.com (mail-lf1-f53.google.com [209.85.167.53]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 9D50D8BCDDD for ; Mon, 4 Dec 2023 16:25:19 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 9D50D8BCDDD Received: by mail-lf1-f53.google.com with SMTP id 2adb3069b0e04-50bef9b7a67so1805247e87.1 for ; Mon, 04 Dec 2023 05:25:19 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701696319; x=1702301119; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=cEMHdM46ZlIJ8bPLYjUjjRproFd0vHI/wTPbAeZOqLM=; b=A0dNBkVkpLrPKmw7EfyLV0XtNYaDcwkkDR33zt9pLzETOaC8w66Z0/kl7JvPmcoJVt aUycajHFWVeOeLfi+uRzITri+LIP0d1dse5JeEruD4ZZy1DJ7cqAIR0Vk9ARRza+Yx1/ 8b7Tnd3AMeQFmZMRZCLBQ3PtvfeWtEzDnjiHjBRErGwOqtER4KdRa7A/YHXKoxx5GkJV jY/cB2w+piyPUvMl4FzUGhc3pLIEgoe9HECRARWfe7NNiefwIZbLfhu0yujOvE5LNyud H4kpGM7x4+HfHvenz+dcAVZmxvFloTatC7vt/Y+uGq8piprvI0x8kjZI4ShOjvhnAJ7j JYlg== X-Gm-Message-State: AOJu0YyiGFiWCbAEF6+4yFFkn/FmhEX3h2TFKCc6iI2eqN7lFEYeWj8O gsXB6NO4elmPoaRPizdyr9Svu5zou3Xssw== X-Google-Smtp-Source: AGHT+IFVL8vOyYlPNvllka++ow/PVeiRQvihCOVn+0qyCKKWX5oTbQUkyT621p43kgtu3przcXaKJg== X-Received: by 2002:a2e:b0f8:0:b0:2ca:f17:dd60 with SMTP id h24-20020a2eb0f8000000b002ca0f17dd60mr53117ljl.62.1701696318607; Mon, 04 Dec 2023 05:25:18 -0800 (PST) Received: from fckxorg.mail.msk ([2a00:1148:b0ba:16:a3e8:bdc1:dbed:dbc8]) by smtp.gmail.com with ESMTPSA id x1-20020a2e9dc1000000b002ca09599e28sm196597ljj.107.2023.12.04.05.25.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Dec 2023 05:25:18 -0800 (PST) X-Google-Original-From: Maxim Kokryashkin To: tarantool-patches@dev.tarantool.org, skaplun@tarantool.org, sergeyb@tarantool.org Date: Mon, 4 Dec 2023 16:25:02 +0300 Message-ID: <471c196302e1153f4493d429ad3b3d19b60b8fd5.1701696044.git.m.kokryashkin@tarantool.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH luajit 4/4] profilers: print user-friendly errors X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Prior to this patch, there was no error-checking in profilers, which resulted in raw Lua errors being reported in cases of non-existing paths or corrupt file structures. This patch adds error handling, so all parsing errors are now reported in a user-friendly manner. Event parsing is now moved into a separate profiler-agnostic module. Tool CLI flag tests are adapted correspondingly to error message changes. Resolves tarantool/tarantool#9217 Part of tarantool/tarantool#5994 --- .../gh-5688-tool-cli-flag.test.lua | 4 +- ...17-profile-parsers-error-handling.test.lua | 79 +++++++++++++++++++ tools/memprof.lua | 47 +++++++++-- tools/sysprof.lua | 42 ++++++++-- tools/sysprof/parse.lua | 2 +- tools/utils/symtab.lua | 2 +- 6 files changed, 158 insertions(+), 18 deletions(-) create mode 100644 test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua diff --git a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua index 75293f11..ec958031 100644 --- a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua +++ b/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua @@ -42,7 +42,7 @@ local SMOKE_CMD_SET = { local MEMPROF_CMD_SET = { { cmd = MEMPROF_PARSER .. BAD_PATH, - like = 'fopen, errno: 2', + like = 'Failed to open', }, { cmd = MEMPROF_PARSER .. TMP_BINFILE_MEMPROF, @@ -61,7 +61,7 @@ local MEMPROF_CMD_SET = { local SYSPROF_CMD_SET = { { cmd = SYSPROF_PARSER .. BAD_PATH, - like = 'fopen, errno: 2', + like = 'Failed to open', }, { cmd = SYSPROF_PARSER .. TMP_BINFILE_SYSPROF, diff --git a/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua b/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua new file mode 100644 index 00000000..9a818086 --- /dev/null +++ b/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua @@ -0,0 +1,79 @@ +local tap = require('tap') +local test = tap.test('gh-9217-profile-parsers-error-handling'):skipcond({ + ['Profile tools are implemented for x86_64 only'] = jit.arch ~= 'x86' and + jit.arch ~= 'x64', + ['Profile tools are implemented for Linux only'] = jit.os ~= 'Linux', + -- XXX: Tarantool integration is required to run this test properly. + -- luacheck: no global + ['No profile tools CLI option integration'] = _TARANTOOL, +}) + +test:plan(6) + +jit.off() +jit.flush() + +local table_new = require('table.new') +local utils = require('utils') + +local BAD_PATH = utils.tools.profilename('bad-path-tmp.bin') +local NON_PROFILE_DATA = utils.tools.profilename('not-profile-data.tmp.bin') +local CORRUPT_PROFILE = utils.tools.profilename('profdata.tmp.bin') + +local EXECUTABLE = utils.exec.luacmd(arg) +local PARSERS = { + memprof = EXECUTABLE .. ' -tm ', + sysprof = EXECUTABLE .. ' -ts ', +} +local REDIRECT_OUTPUT = ' 2>&1' + +local TABLE_SIZE = 20 + +local TEST_CASES = { + [BAD_PATH] = 'Failed to open', + [NON_PROFILE_DATA] = 'Failed to parse symtab from', + [CORRUPT_PROFILE] = 'Failed to parse profile data from', +} + +local function generate_non_profile_data(path) + local file = io.open(path, 'w') + file:write('data') + file:close() +end + +local function generate_corrupt_profile(path) + local res, err = misc.memprof.start(path) + -- Should start successfully. + assert(res, err) + + local _ = table_new(TABLE_SIZE, 0) + _ = nil + collectgarbage() + + res, err = misc.memprof.stop() + -- Should stop successfully. + assert(res, err) + + local file = io.open(path, 'r') + local content = file:read('*all') + file:close() + local index = string.find(content, 'ljm') + + file = io.open(path, 'w') + file:write(string.sub(content, 1, index - 1)) + file:close() +end + +generate_non_profile_data(NON_PROFILE_DATA) +generate_corrupt_profile(CORRUPT_PROFILE) + +for path, err_msg in pairs(TEST_CASES) do + for profiler, parser in pairs(PARSERS) do + local output = io.popen(parser .. path .. REDIRECT_OUTPUT):read('*all') + test:like(output, err_msg, string.format('%s: %s', profiler, err_msg)) + end +end + +os.remove(NON_PROFILE_DATA) +os.remove(CORRUPT_PROFILE) +test:done(true) diff --git a/tools/memprof.lua b/tools/memprof.lua index acadbc17..a04608b8 100644 --- a/tools/memprof.lua +++ b/tools/memprof.lua @@ -10,11 +10,11 @@ -- Major portions taken verbatim or adapted from the LuaVela. -- Copyright (C) 2015-2019 IPONWEB Ltd. -local bufread = require "utils.bufread" -local memprof = require "memprof.parse" -local process = require "memprof.process" -local symtab = require "utils.symtab" -local view = require "memprof.humanize" +local bufread = require('utils.bufread') +local symtab = require('utils.symtab') +local memprof = require('memprof.parse') +local process = require('memprof.process') +local view = require('memprof.humanize') local stdout, stderr = io.stdout, io.stderr local match, gmatch = string.match, string.gmatch @@ -106,10 +106,41 @@ local function parseargs(args) return args[args.argn] end +local function make_error_handler(inputfile, fmt) + return function() + io.stderr:write(string.format(fmt, inputfile)) + os.exit(1, true) + end +end + +local function safe_event_reader(inputfile) + local _, reader = xpcall( + bufread.new, + make_error_handler(inputfile, 'Failed to open %s.'), + inputfile + ) + + local _, symbols = xpcall( + symtab.parse, + make_error_handler(inputfile, 'Failed to parse symtab from %s.'), + reader + ) + + local _, events = xpcall( + memprof.parse, + make_error_handler(inputfile, 'Failed to parse profile data from %s.'), + reader, + symbols + ) + return events, symbols +end + local function dump(inputfile) - local reader = bufread.new(inputfile) - local symbols = symtab.parse(reader) - local events = memprof.parse(reader, symbols) + -- XXX: This function exits with a non-zero exit code and + -- prints an error message if it encounters any failure during + -- the process of parsing. + local events, symbols = safe_event_reader(inputfile) + if not config.leak_only then view.profile_info(events, config) end diff --git a/tools/sysprof.lua b/tools/sysprof.lua index 8449b23f..d2efcd7f 100644 --- a/tools/sysprof.lua +++ b/tools/sysprof.lua @@ -1,6 +1,6 @@ -local bufread = require "utils.bufread" -local sysprof = require "sysprof.parse" -local symtab = require "utils.symtab" +local bufread = require('utils.bufread') +local symtab = require('utils.symtab') +local sysprof = require('sysprof.parse') local stdout, stderr = io.stdout, io.stderr local match, gmatch = string.match, string.gmatch @@ -77,10 +77,40 @@ local function parseargs(args) return args[args.argn] end +local function make_error_handler(inputfile, fmt) + return function() + io.stderr:write(string.format(fmt, inputfile)) + os.exit(1, true) + end +end + +local function safe_event_reader(inputfile) + local _, reader = xpcall( + bufread.new, + make_error_handler(inputfile, 'Failed to open %s.'), + inputfile + ) + + local _, symbols = xpcall( + symtab.parse, + make_error_handler(inputfile, 'Failed to parse symtab from %s.'), + reader + ) + + local _, events = xpcall( + sysprof.parse, + make_error_handler(inputfile, 'Failed to parse profile data from %s.'), + reader, + symbols + ) + return events, symbols +end + local function dump(inputfile) - local reader = bufread.new(inputfile) - local symbols = symtab.parse(reader) - local events = sysprof.parse(reader, symbols) + -- XXX: This function exits with a non-zero exit code and + -- prints an error message if it encounters any failure during + -- the process of parsing. + local events = safe_event_reader(inputfile) for stack, count in pairs(events) do print(stack, count) diff --git a/tools/sysprof/parse.lua b/tools/sysprof/parse.lua index 64c4a455..749f70db 100755 --- a/tools/sysprof/parse.lua +++ b/tools/sysprof/parse.lua @@ -237,7 +237,7 @@ function M.parse(reader, symbols) local _ = reader:read_octets(3) if magic ~= LJP_MAGIC then - error("Bad LJP format prologue: "..magic) + error("Bad LJP format prologue: " .. tostring(magic)) end if string.byte(version) ~= LJP_CURRENT_VERSION then diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua index 0c878e7a..c4aefef7 100644 --- a/tools/utils/symtab.lua +++ b/tools/utils/symtab.lua @@ -95,7 +95,7 @@ function M.parse(reader) local _ = reader:read_octets(3) if magic ~= LJS_MAGIC then - error("Bad LJS format prologue: "..magic) + error("Bad LJS format prologue: " .. tostring(magic)) end if string.byte(version) ~= LJS_CURRENT_VERSION then -- 2.43.0