[PATCH] D12153: Support inline functions symbolization in Addr2Line symbolizer.

[Alexey Samsonov via llvm-commits]

samsonov added inline comments.

================
Comment at: lib/asan/scripts/asan_symbolize.py:159
@@ -157,3 +158,3 @@
       print >> self.pipe.stdin, offset
-      function_name = self.pipe.stdout.readline().rstrip()
-      file_name = self.pipe.stdout.readline().rstrip()
+      print >> self.pipe.stdin, "0xbadbadbad"
+      result = []
----------------
Make this a named constant. I also wonder if different value would work better (smth. that will definitely be outside of .text section).

================
Comment at: lib/asan/scripts/asan_symbolize.py:163
@@ +162,3 @@
+      frame = 0
+      while frame < self.inline_frames_depth:
+        function_name = self.pipe.stdout.readline().rstrip()
----------------
Why can't this be while-true loop?

================
Comment at: lib/asan/scripts/asan_symbolize.py:171
@@ -160,5 +170,3 @@
     except Exception:
-      function_name = ''
-      file_name = ''
-    file_name = fix_filename(file_name)
-    return ['%s in %s %s' % (addr, function_name, file_name)]
+      lines = (('??', '??:?'))
+    for frame in lines:
----------------
Should this be a list of pairs
  [('??', '??:?')]
? Or just declare `lines` before try-catch block and call .append here

================
Comment at: lib/asan/scripts/asan_symbolize.py:176
@@ -165,1 +175,3 @@
+      result.append('%s in %s %s' % (addr, function_name, file_name))
+    return result
 
----------------
  return ['%s in %s %s' % (addr, frame[0], fix_filename(frame[1])) for frame in lines]

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_internal.h:78
@@ -77,3 +77,3 @@
  protected:
-  virtual bool ReachedEndOfOutput(const char *buffer, uptr length) const {
+  virtual uptr GetEffectiveLenOfOutput(const char *buffer, uptr length) const {
     UNIMPLEMENTED();
----------------
The semantics of this function is very confusing. If `GetEffectiveLenOfOutput` is 0, it means that we should continue to fetch the output, which makes little sense.

Also, this is incorrect. Suppose you have passed
  0x400400
  0xbadbadbad
to addr2line, it had printed the following to stdout:
  foo
  /tmp/foo.cc:42
  ??
  ??:?
Suppose the first call to `ReadFromFile` in `ReadFromSymbolizer` has fecthed:
  foo
  /tmp/foo.cc:42
  ??
In your implementation, `GetEffectiveLenOfOutput` would return non-zero, you will break from the loop and fail to consume the last line.

You should fix it at the higher level - consume the output of `addr2line` as you do now, but change the way you handle this buffer: either teach ParseSymbolizePCOutput to drop the last couple of lines, or (better) trim the buffer in `Addr2LineSymbolizer` before calling that function.


Repository:
  rL LLVM

http://reviews.llvm.org/D12153