[compiler-rt] Check for empty buffer in Addr2LineProcess::ReadFromSymbolizer

[david-y-lam]

Summary

This fixes a bug in the ReadFromSymbolizer method of the Addr2LineProcess class; if the input is too large, the returned buffer will be null and will consequently fail the CHECK. The proposed fix is to simply check if the buffer consists of only a null-terminator and return if so (in effect skipping that frame). I tested by running one of the unit tests both before and after my change.

Before:

/data/users/davidlam/llvm/build 19:51$ ASAN_OPTIONS=allow_addr2line=true:external_symbolizer_path=/usr/local/bin/addr2line ASAN_SYMBOLIZER_PATH=../build/bin/llvm-symbolizer  ../build/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/symbolize_stack.cc.tmp
    #0 0x4ffd23 in __sanitizer_print_stack_trace /data/users/davidlam/llvm/llvm/projects/compiler-rt/lib/asan/asan_stack.cc:38
==1180931==WARNING: Symbolizer buffer too small==1180931==AddressSanitizer CHECK failed: /data/users/davidlam/llvm/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:282 "((garbage)) != (0)(0x0, 0x0)

After:

/data/users/davidlam/llvm/build 21:04$ ASAN_OPTIONS=allow_addr2line=true:external_symbolizer_path=/usr/local/bin/addr2line ../build/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/symbolize_stack.cc.tmp
    #0 0x4ffd23 in __sanitizer_print_stack_trace /data/users/davidlam/llvm/llvm/projects/compiler-rt/lib/asan/asan_stack.cc:38
==2250158==WARNING: Symbolizer buffer too small
    #1 0x52dc84  (/data/users/davidlam/llvm/build/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/symbolize_stack.cc.tmp+0x52dc84)
==2250158==WARNING: Symbolizer buffer too small
    #2 0x52d93f  (/data/users/davidlam/llvm/build/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/symbolize_stack.cc.tmp+0x52d93f)
==2250158==WARNING: Symbolizer buffer too small
    #3 0x52d4ff  (/data/users/davidlam/llvm/build/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/symbolize_stack.cc.tmp+0x52d4ff)
==2250158==WARNING: Symbolizer buffer too small
    #4 0x52d0bf  (/data/users/davidlam/llvm/build/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/symbolize_stack.cc.tmp+0x52d0bf)
    #5 0x52cc7f in > >, std::allocator<std::vector<int, std::allocator<int> > > > > >, std::allocator<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > > > > > > > >, std::allocator<std::vector<std::vector<std::vector<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > >, std::allocator<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > > > >, std::allocator<std::vector<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > >, std::allocator<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > > > > > >, std::allocator<std::vector<std::vector<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > >, std::allocator<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > > > >, std::allocator<std::vector<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > >, std::allocator<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > > > > > > > > > >, std::allocator<std::vector<std::vector<std::vector<std::vector<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > >, std::allocator<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > > > >, std::allocator<std::vector<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > >, std::allocator<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > > > > > >, std::allocator<std::vector<std::vector<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > >, std::allocator<std::vector<std::
==2250158==WARNING: Symbolizer buffer too small
    #6 0x52c83f  (/data/users/davidlam/llvm/build/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/symbolize_stack.cc.tmp+0x52c83f)
==2250158==WARNING: Symbolizer buffer too small
    #7 0x52c3ff  (/data/users/davidlam/llvm/build/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/symbolize_stack.cc.tmp+0x52c3ff)
==2250158==WARNING: Symbolizer buffer too small
    #8 0x52bfbf  (/data/users/davidlam/llvm/build/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/symbolize_stack.cc.tmp+0x52bfbf)
==2250158==WARNING: Symbolizer buffer too small
    #9 0x52bb7f  (/data/users/davidlam/llvm/build/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/symbolize_stack.cc.tmp+0x52bb7f)
    #10 0x52b72f in t, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > > > >, std::allocator<std::vector<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > >, std::allocator<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > > > > > > > > >(std::vector<std::vector<std::vector<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > >, std::allocator<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > > > >, std::allocator<std::vector<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > >, std::allocator<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > > > > > >, std::allocator<std::vector<std::vector<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > >, std::allocator<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > > > >, std::allocator<std::vector<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > >, std::allocator<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > > > > > > > > const&) /data/users/davidlam/llvm/llvm/projects/compiler-rt/test/sanitizer_common/TestCases/symbolize_stack.cc:16
    #11 0x52b4ef in void A<7>::RecursiveTemplateFunction<std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > > >(std::vector<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >, std::allocator<std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > > > > const&) /data/users/davidlam/llvm/llvm/projects/compiler-rt/test/sanitizer_common/TestCases/symbolize_stack.cc:16
    #12 0x52b372 in void A<10>::RecursiveTemplateFunction<int>(int const&) /data/users/davidlam/llvm/llvm/projects/compiler-rt/test/sanitizer_common/TestCases/symbolize_stack.cc:16
    #13 0x7fbf7d47fb34 in __libc_start_main ??:?
    #14 0x41ad6b in main /data/users/davidlam/llvm/llvm/projects/compiler-rt/test/sanitizer_common/TestCases/symbolize_stack.cc:27

[david-y-lam]

First time contributor here, please let me know if I'm missing anything. Thanks!

[david-y-lam]

I used the symbolize_stack.cc test for checking my changes and would like to add a unit test to cover this condition. Would it be okay to add ASAN_OPTIONS=allow_addr2line=true to that test or should I find another way?

[alekseyshl]

addr2line is not necessary present on the system, I don't think you should test for it.

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

276 ↗

(On Diff #109269)

// The returned buffer is empty when output is valid, but exceeds max_length.

We're not skipping the frame, we just not looking for output_terminator_ in it.

[david-y-lam]

lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

276 ↗

(On Diff #109269)

that's a better comment, will amend.

[david-y-lam]

amend comment. land-ready. could someone commit this for me? I don't have commit access.

compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc

while (true) {
     if (!success || just_read == 0) {
       Report("WARNING: Can't read from symbolizer at fd %d\n", input_fd_);
       return false;
     }
     read_len += just_read;
     if (ReachedEndOfOutput(buffer, read_len))
       break;
     if (read_len + 1 == max_length) {
-      Report("WARNING: Symbolizer buffer too small");
+      Report("WARNING: Symbolizer buffer too small\n");
       read_len = 0;
       break;
     }
   }
   buffer[read_len] = '\0';
   return true;
 }
 

compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc

void GetArgV(const char *path_to_binary,
     argv[i++] = nullptr;
   }
 
   bool ReachedEndOfOutput(const char *buffer, uptr length) const override;
 
   bool ReadFromSymbolizer(char *buffer, uptr max_length) override {
     if (!SymbolizerProcess::ReadFromSymbolizer(buffer, max_length))
       return false;
+    // The returned buffer is empty when output is valid, but exceeds
+    // max_length.
+    if (*buffer == '\0')
+      return true;
     // We should cut out output_terminator_ at the end of given buffer,
     // appended by addr2line to mark the end of its meaningful output.
     // We cannot scan buffer from it's beginning, because it is legal for it
     // to start with output_terminator_ in case given offset is invalid. So,
     // scanning from second character.
     char *garbage = internal_strstr(buffer + 1, output_terminator_);
     // This should never be NULL since buffer must end up with
     // output_terminator_.