[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:172
@@ -166,1 +171,3 @@
+      lines.append(('??', '??:?'))
+    return ['%s in %s %s' % (addr, line, fix_filename(file)) for (line, file) in lines]
 
----------------
s/line/function

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:199
@@ +198,3 @@
+    const size_t terminator_len = sizeof(output_terminator_) - 1;
+    // Skip, if we red just TERMINATOR_LEN bytes, because Addr2Line output
+    // should consist at least of two pairs of lines:
----------------
s/red/read

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:202
@@ +201,3 @@
+    // 1. First one, corresponding to given offset to be symbolized
+    // (may be equal to OUTPUT_TERMINATOR_ if offset is not valid).
+    // 2. Second one for OUTPUT_TERMINATOR_ itself to mark the end of output.
----------------
Please fix variable names in the comment - there are no OUTPUT_TERMINATOR_ and TERMINATOR_LEN

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:204
@@ +203,3 @@
+    // 2. Second one for OUTPUT_TERMINATOR_ itself to mark the end of output.
+    if (length == terminator_len) return false;
+    // Addr2Line output should end up with OUTPUT_TERMINATOR_.
----------------
Should this be <= ?

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:209
@@ +208,3 @@
+    if (is_terminated) {
+      effective_length = length - terminator_len;
+      return true;
----------------
class members should end with _

Use of static class variable here is really weird - it adds implicit restriction that you will *always* call ReachedEndOfOutput, followed by TrimOutputBuffer with the same buffer, and that there will be no several instances of this class. I think the latter is false even now.

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:229
@@ -218,1 +228,3 @@
   const char *module_name_;  // Owned, leaked.
+  static size_t effective_length;
+  static const char output_terminator_[9];
----------------
I don't like hardcoded 9 either. You can just move this constant into the body of `ReachedEndOfOutput`
  static const char kOutputTerminator[] = "??\n??:0\n";



http://reviews.llvm.org/D12153