[Tarantool-patches] [PATCH luajit 4/4] profilers: print user-friendly errors

[Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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