tarantool / tarantool

Get your data in RAM. Get compute close to data. Enjoy the performance.
https://www.tarantool.io
Other
3.42k stars 378 forks source link

memprof cannot parse corrupted data #9308

Open ligurio opened 1 year ago

ligurio commented 1 year ago

Bug description

During investigation of #9283 I've recorded memory profile and recording was stopped by crashed Tarantool. memprof cannot parse such profile.

Tarantool Enterprise 3.0.0-alpha3-35-g5d1e03a

Steps to reproduce

see orginal steps in https://github.com/tarantool/tarantool/issues/9283#issuecomment-1777022897

Actual behavior

memprof cannot parse profile

[1] ~/sources/MRG/tarantool/third_party/luajit$ ./build/src/luajit -e 'require("memprof")(arg)' - --leak-only /home/sergeyb/sources/MRG/tarantool-ee/src/lua/etcd-client/memprof.bin 
./build/src/luajit: ./tools/memprof/parse.lua:159: attempt to compare nil with number
stack traceback:
        ./tools/memprof/parse.lua:159: in function 'ev_header_is_valid'
        ./tools/memprof/parse.lua:171: in function 'parse_event'
        ./tools/memprof/parse.lua:212: in function 'parse'
        ./tools/memprof.lua:103: in function <./tools/memprof.lua:100>
        (command line):1: in main chunk
        [C]: at 0x562b4bfc28f0

Expected behavior

memprof is able to parse profile

ligurio commented 1 year ago

Patch prepared by Max fixes the problem:

diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua
index d865267b..a932f290 100644
--- a/tools/memprof/parse.lua
+++ b/tools/memprof/parse.lua
@@ -179,9 +179,7 @@ local function parse_event(reader, events, symbols)

   assert(parser, "Bad aevent "..aevent)

-  parser.parse(reader, asource, events[parser.evname], events.heap, symbols)
-
-  return true
+  return pcall(parser.parse, reader, asource, events[parser.evname], events.heap, symbols)
 end

 function M.parse(reader, symbols)
diff --git a/tools/utils/bufread.lua b/tools/utils/bufread.lua
index 34bae9a6..100f771d 100644
--- a/tools/utils/bufread.lua
+++ b/tools/utils/bufread.lua
@@ -62,7 +62,7 @@ end

 function M.read_octet(reader)
   if not _read_stream(reader, 1) then
-    return nil
+    error('corrupted stream')
   end

   local oct = reader._buf[reader._pos]
mkokryashkin commented 1 year ago

Important: this is a fast and ugly fix, so it should be reconsidered thoroughly.