Skip to content

Commit

Permalink
profilers: print user-friendly errors
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mkokryashkin committed Jan 9, 2024
1 parent 517a1a4 commit 6f17023
Show file tree
Hide file tree
Showing 10 changed files with 149 additions and 20 deletions.
2 changes: 1 addition & 1 deletion Makefile.original
Original file line number Diff line number Diff line change
Expand Up @@ -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 evread.lua symtab.lua
FILES_MEMPROFLIB= humanize.lua parse.lua process.lua
FILES_SYSPROFLIB= parse.lua
FILES_TOOLSLIB= memprof.lua sysprof.lua
Expand Down
4 changes: 2 additions & 2 deletions test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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.*fopen, errno: 2',
},
{
cmd = MEMPROF_PARSER .. TMP_BINFILE_MEMPROF,
Expand All @@ -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.*fopen, errno: 2',
},
{
cmd = SYSPROF_PARSER .. TMP_BINFILE_SYSPROF,
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
4 changes: 4 additions & 0 deletions tools/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ else()
memprof.lua
utils/avl.lua
utils/bufread.lua
utils/evread.lua
utils/symtab.lua
)
list(APPEND LUAJIT_TOOLS_DEPS tools-parse-memprof)
Expand All @@ -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/evread.lua
${CMAKE_CURRENT_SOURCE_DIR}/utils/symtab.lua
DESTINATION ${LUAJIT_DATAROOTDIR}/utils
PERMISSIONS
Expand Down Expand Up @@ -65,6 +67,7 @@ else()
sysprof.lua
utils/avl.lua
utils/bufread.lua
utils/evread.lua
utils/symtab.lua
)
list(APPEND LUAJIT_TOOLS_DEPS tools-parse-sysprof)
Expand All @@ -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/evread.lua
${CMAKE_CURRENT_SOURCE_DIR}/utils/symtab.lua
DESTINATION ${LUAJIT_DATAROOTDIR}/utils
PERMISSIONS
Expand Down
11 changes: 6 additions & 5 deletions tools/memprof.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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 evread = require "utils.evread"
local view = require "memprof.humanize"

local stdout, stderr = io.stdout, io.stderr
Expand Down Expand Up @@ -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 = evread(memprof.parse, inputfile)

if not config.leak_only then
view.profile_info(events, config)
end
Expand Down
5 changes: 3 additions & 2 deletions tools/memprof/parse.lua
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,13 @@ function M.parse(reader, symbols)
local _ = reader:read_octets(3)

if magic ~= LJM_MAGIC then
error("Bad LJM format prologue: "..magic)
error("Bad memprof event format prologue: "..magic)
end

if string.byte(version) ~= LJM_CURRENT_VERSION then
error(string_format(
"LJM format version mismatch: the tool expects %d, but your data is %d",
"Memprof event format version mismatch:"..
" the tool expects %d, but your data is %d",
LJM_CURRENT_VERSION,
string.byte(version)
))
Expand Down
10 changes: 5 additions & 5 deletions tools/sysprof.lua
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
local bufread = require "utils.bufread"
local evread = require "utils.evread"
local sysprof = require "sysprof.parse"
local symtab = require "utils.symtab"

local stdout, stderr = io.stdout, io.stderr
local match, gmatch = string.match, string.gmatch
Expand Down Expand Up @@ -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 = evread(sysprof.parse, inputfile)

for stack, count in pairs(events) do
print(stack, count)
Expand Down
5 changes: 3 additions & 2 deletions tools/sysprof/parse.lua
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,13 @@ function M.parse(reader, symbols)
local _ = reader:read_octets(3)

if magic ~= LJP_MAGIC then
error("Bad LJP format prologue: "..magic)
error("Bad sysprof event format prologue: " .. tostring(magic))
end

if string.byte(version) ~= LJP_CURRENT_VERSION then
error(string_format(
"LJP format version mismatch: the tool expects %d, but your data is %d",
"Sysprof event format version mismatch:"..
" the tool expects %d, but your data is %d",
LJP_CURRENT_VERSION,
string.byte(version)
))
Expand Down
32 changes: 32 additions & 0 deletions tools/utils/evread.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
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('\n\t%s\n', err))
os.exit(1, true)
end
end

return function(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
6 changes: 3 additions & 3 deletions tools/utils/symtab.lua
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,13 @@ function M.parse(reader)
local _ = reader:read_octets(3)

if magic ~= LJS_MAGIC then
error("Bad LJS format prologue: "..magic)
error("Bad LuaJIT symbol table format prologue: " .. tostring(magic))
end

if string.byte(version) ~= LJS_CURRENT_VERSION then
error(string_format(
"LJS format version mismatch:"..
"the tool expects %d, but your data is %d",
"LuaJIT symbol table format version mismatch:"..
" the tool expects %d, but your data is %d",
LJS_CURRENT_VERSION,
string.byte(version)
))
Expand Down

0 comments on commit 6f17023

Please sign in to comment.