[Tarantool-patches] [PATCH luajit v2 6/6] profilers: print user-friendly errors
Maxim Kokryashkin
max.kokryashkin at gmail.com
Wed Dec 6 21:55:21 MSK 2023
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
More information about the Tarantool-patches
mailing list