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 28E95727D0D; Wed, 6 Dec 2023 21:58:29 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 28E95727D0D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1701889109; bh=Xgef1V9CdiGkHiF87xFrwxuse3BTkikTSH6ncg1ilO4=; 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=UMjF4vub84x0cJ8vKG7C1yyaPbyyM/b41zyJs7RILnpOcGS/xPvc4TIT91UGtzO+9 HwLwmLtrrEm6ROMIf5E89qv0QbqbnDXA1ft0H6/xZgsIMa2D0bVy3ntDIhz02MBjb4 2LjjIj8FbPwyyG76lkp0QPEIpOPeP3Y49ZIlIn28= Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com [209.85.167.52]) (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 CCDB0727D12 for ; Wed, 6 Dec 2023 21:55:43 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org CCDB0727D12 Received: by mail-lf1-f52.google.com with SMTP id 2adb3069b0e04-50bffb64178so115541e87.2 for ; Wed, 06 Dec 2023 10:55:43 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701888943; x=1702493743; 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=7Ai5F98sz/RvtvhMhpi3DD1mkJwK4S60UHB3/3AFPA8=; b=hGZXy61+x205ND6QukEYgia0Cf6TN2lE5FXuTR8x2ix1JqCercxML68CXCajroF9MS 8G8/xKGaS3lXRf8wH0RbPCDC7uQ5CPu8+1aXdN2AFFDNCm8zW1vsAnwCd4i8TZhbtFQH UA8DGCgmjjNqpW1ABubw9HarTMkKYvU9QAtN7hGGp3wluyoUDRoSzMcramxCCcJ4TuOj nUzRTQvFAPUU+3ItmA+4IYPfZPzCIfFjmqnxx58oyNfCrQNRXrHS3Csy+8zAbAJlpQOn 8QNqg3tpj9vqfBnVE6unaHwEYDGu+XieIpM+Jze1OIVBUiXSIzn7XPp/bxHpdnXn8aJn QIlA== X-Gm-Message-State: AOJu0YylGnA0gpFvrOhzhBul/naVzutbN0tEgsQo0rrXahy7FCRYN/Ds ZnoNavu7DyJ78EP2l5MfONfq3y6Th+ef5g== X-Google-Smtp-Source: AGHT+IGzj8lJqZedv+oSJXMwh6C8r1KeQxB/yw8N3ub7a+14hAwLNy4XSoP0Gbm8F2iYjB0iR2zCfg== X-Received: by 2002:a19:f60f:0:b0:50b:f486:a65e with SMTP id x15-20020a19f60f000000b0050bf486a65emr833586lfe.106.1701888942734; Wed, 06 Dec 2023 10:55:42 -0800 (PST) Received: from localhost.localdomain (95-24-1-238.broadband.corbina.ru. [95.24.1.238]) by smtp.gmail.com with ESMTPSA id t23-20020a199117000000b0050c0320f167sm606367lfd.245.2023.12.06.10.55.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Dec 2023 10:55:42 -0800 (PST) X-Google-Original-From: Maxim Kokryashkin To: tarantool-patches@dev.tarantool.org, skaplun@tarantool.org, sergeyb@tarantool.org Date: Wed, 6 Dec 2023 21:55:21 +0300 Message-ID: <4fb82034fcac21359f79b81cc6643054fc432be3.1701888856.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 v2 6/6] 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 --- Makefile.original | 2 +- .../gh-5688-tool-cli-flag.test.lua | 4 +- ...17-profile-parsers-error-handling.test.lua | 90 +++++++++++++++++++ tools/CMakeLists.txt | 4 + tools/memprof.lua | 11 +-- tools/sysprof.lua | 10 +-- tools/sysprof/parse.lua | 2 +- tools/utils/safe_event_reader.lua | 34 +++++++ tools/utils/symtab.lua | 2 +- 9 files changed, 144 insertions(+), 15 deletions(-) create mode 100644 test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua create mode 100644 tools/utils/safe_event_reader.lua diff --git a/Makefile.original b/Makefile.original index 2a56d648..1ef6fbe6 100644 --- a/Makefile.original +++ b/Makefile.original @@ -97,7 +97,7 @@ FILES_JITLIB= bc.lua bcsave.lua dump.lua p.lua v.lua zone.lua \ dis_x86.lua dis_x64.lua dis_arm.lua dis_arm64.lua \ dis_arm64be.lua dis_ppc.lua dis_mips.lua dis_mipsel.lua \ dis_mips64.lua dis_mips64el.lua vmdef.lua -FILES_UTILSLIB= avl.lua bufread.lua symtab.lua +FILES_UTILSLIB= avl.lua bufread.lua safe_event_reader.lua symtab.lua FILES_MEMPROFLIB= parse.lua humanize.lua process.lua FILES_SYSPROFLIB= parse.lua FILES_TOOLSLIB= memprof.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..c2e0f0a6 --- /dev/null +++ b/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua @@ -0,0 +1,90 @@ +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, +}) + +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 = { + { + path = BAD_PATH, + err_msg = 'Failed to open' + }, + { + path = NON_PROFILE_DATA, + err_msg = 'Failed to parse symtab from' + }, + { + path = CORRUPT_PROFILE, + err_msg = 'Failed to parse profile data from' + }, +} + +test:plan(2 * #TEST_CASES) + +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 _, case in ipairs(TEST_CASES) do + for profiler, parser in pairs(PARSERS) do + local path = case.path + local err_msg = case.err_msg + 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/CMakeLists.txt b/tools/CMakeLists.txt index a4adc15b..7f728787 100644 --- a/tools/CMakeLists.txt +++ b/tools/CMakeLists.txt @@ -18,6 +18,7 @@ else() memprof.lua utils/avl.lua utils/bufread.lua + utils/safe_event_reader.lua utils/symtab.lua ) list(APPEND LUAJIT_TOOLS_DEPS tools-parse-memprof) @@ -36,6 +37,7 @@ else() install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/utils/avl.lua ${CMAKE_CURRENT_SOURCE_DIR}/utils/bufread.lua + ${CMAKE_CURRENT_SOURCE_DIR}/utils/safe_event_reader.lua ${CMAKE_CURRENT_SOURCE_DIR}/utils/symtab.lua DESTINATION ${LUAJIT_DATAROOTDIR}/utils PERMISSIONS @@ -65,6 +67,7 @@ else() sysprof.lua utils/avl.lua utils/bufread.lua + utils/safe_event_reader.lua utils/symtab.lua ) list(APPEND LUAJIT_TOOLS_DEPS tools-parse-sysprof) @@ -81,6 +84,7 @@ else() install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/utils/avl.lua ${CMAKE_CURRENT_SOURCE_DIR}/utils/bufread.lua + ${CMAKE_CURRENT_SOURCE_DIR}/utils/safe_event_reader.lua ${CMAKE_CURRENT_SOURCE_DIR}/utils/symtab.lua DESTINATION ${LUAJIT_DATAROOTDIR}/utils PERMISSIONS diff --git a/tools/memprof.lua b/tools/memprof.lua index 537cb869..d491b3dc 100644 --- a/tools/memprof.lua +++ b/tools/memprof.lua @@ -10,10 +10,9 @@ -- 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 safe_event_reader = require "utils.safe_event_reader" local view = require "memprof.humanize" local stdout, stderr = io.stdout, io.stderr @@ -108,9 +107,11 @@ local function parseargs(args) 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(memprof.parse, 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..9c0c23c9 100644 --- a/tools/sysprof.lua +++ b/tools/sysprof.lua @@ -1,6 +1,5 @@ -local bufread = require "utils.bufread" +local safe_event_reader = require "utils.safe_event_reader" local sysprof = require "sysprof.parse" -local symtab = require "utils.symtab" local stdout, stderr = io.stdout, io.stderr local match, gmatch = string.match, string.gmatch @@ -78,9 +77,10 @@ local function parseargs(args) 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(sysprof.parse, 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/safe_event_reader.lua b/tools/utils/safe_event_reader.lua new file mode 100644 index 00000000..39246a9d --- /dev/null +++ b/tools/utils/safe_event_reader.lua @@ -0,0 +1,34 @@ +local bufread = require('utils.bufread') +local symtab = require('utils.symtab') + +local function make_error_handler(fmt, inputfile) + return function(err) + io.stderr:write(string.format(fmt, inputfile)) + io.stderr:write(string.format('\nOriginal error: %s', err)) + os.exit(1, true) + end +end + +local function safe_event_reader(parser, inputfile) + local _, reader = xpcall( + bufread.new, + make_error_handler('Failed to open %s.', inputfile), + inputfile + ) + + local _, symbols = xpcall( + symtab.parse, + make_error_handler('Failed to parse symtab from %s.', inputfile), + reader + ) + + local _, events = xpcall( + parser, + make_error_handler('Failed to parse profile data from %s.', inputfile), + reader, + symbols + ) + return events, symbols +end + +return safe_event_reader 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