Add option to symbolize inline frames for InternalSymbolizer

[dgg5503]

Summary

Currently, there is no way to let the InternalSymbolizer implemented
functions know if inline frames should be symbolized. This patch updates
the function __sanitizer_symbolize_code to include a parameter for
this ASAN option and toggle between LLVM symbolization functions when
appropriate.

Fixes the following two failing tests when internal symbolization is
enabled:

SanitizerCommon-*-x86_64-Linux :: print-stack-trace.cpp
SanitizerCommon-*-x86_64-Linux :: symbolize_pc_inline.cpp

[dgg5503]

compiler-rt/lib/sanitizer_common/symbolizer/sanitizer_symbolize.cpp

14

This space appears when I run clang-format using the style file in compiler-rt/lib/sanitizer_common.

[dgg5503]

Ran git-clang-format HEAD to resolve CI errors.

[vitalybuka]

How are those tests are broken? We run InternalSymbolizer on the http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp

325

I afraid we can break stuff if we change this function

[dgg5503]

In D79280#2019393, @vitalybuka wrote:

How are those tests are broken? We run InternalSymbolizer on the http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux

Hi @vitalybuka ,

Thanks for pointing out this build-bot configuration, I was not aware of it. With that in mind, I am a bit concerned that something in my environment is causing tons of errors when testing with the internal symbolizer enabled. I was pretty confident that the tests I mentioned in the description of this review were failing because the internal symbolizer is unable to distinguish between symbolizing with inline frames enabled or disabled (hence this patch). In my local environment, here's what happens if I run these tests without & with the internal symbolizer using the same revision from sanitizer-x86_64-linux #27102:

> cd ~/Documents/llvm-project
> git pull && git checkout 3d76824b7f493e722ae4c160b47978834226c43c
> cd /data/clang-builds/compiler-rt-embed-symbolizer-tk2
> ninja -j96
> ninja check-sanitizer
Testing Time: 43.04s
  Unsupported Tests: 286
  Expected Passes  : 715
  Expected Failures:  25

> python bin/llvm-lit -sv projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/print-stack-trace.cpp projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/symbolize_pc_inline.cpp
Testing Time: 0.82s
  Expected Passes: 2

> CLANG=bin/clang ZLIB_SRC=../../projects/zlib/zlib-1.2.11/ ~/Documents/llvm-project/compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh  /data/clang-builds/compiler-rt-embed-symbolizer-tk2/lib/clang/11.0.0/lib/linux/
...
Success!

> python bin/llvm-lit -sv projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/print-stack-trace.cpp projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/symbolize_pc_inline.cpp
FAIL: SanitizerCommon-asan-x86_64-Linux :: symbolize_pc_inline.cpp (1 of 2)
******************** TEST 'SanitizerCommon-asan-x86_64-Linux :: symbolize_pc_inline.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';      /data/clang-builds/compiler-rt-embed-symbolizer-tk2/./bin/clang  --driver-mode=g++ -gline-tables-only -fsanitize=address  -m64  -ldl -O3  /home/dgliner/Documents/llvm-project/compiler-rt/test/sanitizer_common/TestCases/symbolize_pc_inline.cpp -o /data/clang-builds/compiler-rt-embed-symbolizer-tk2/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/symbolize_pc_inline.cpp.tmp
: 'RUN: at line 2';   env ASAN_OPTIONS=strip_path_prefix=/TestCases/  /data/clang-builds/compiler-rt-embed-symbolizer-tk2/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/symbolize_pc_inline.cpp.tmp 2>&1 | FileCheck /home/dgliner/Documents/llvm-project/compiler-rt/test/sanitizer_common/TestCases/symbolize_pc_inline.cpp
: 'RUN: at line 3';   env ASAN_OPTIONS=strip_path_prefix=/TestCases/:symbolize_inline_frames=0     /data/clang-builds/compiler-rt-embed-symbolizer-tk2/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/symbolize_pc_inline.cpp.tmp 2>&1 | FileCheck /home/dgliner/Documents/llvm-project/compiler-rt/test/sanitizer_common/TestCases/symbolize_pc_inline.cpp --check-prefixes=CHECK-NOINLINE
--
Exit Code: 1

Command Output (stderr):
--
/home/dgliner/Documents/llvm-project/compiler-rt/test/sanitizer_common/TestCases/symbolize_pc_inline.cpp:21:20: error: CHECK-NOINLINE: expected string not found in input
// CHECK-NOINLINE: {{0x[0-9a-f]+}} in main symbolize_pc_inline.cpp:[[@LINE+2]]
                   ^
<stdin>:1:1: note: scanning from here
0x5d28c5 in C2 symbolize_pc_inline.cpp:23:27
^
<stdin>:1:1: note: with "@LINE+2" equal to "23"
0x5d28c5 in C2 symbolize_pc_inline.cpp:23:27
^
<stdin>:4:1: note: possible intended match here
0x5d28c5 in main symbolize_pc_inline.cpp:32:14
^

--

********************
FAIL: SanitizerCommon-asan-x86_64-Linux :: print-stack-trace.cpp (2 of 2)
******************** TEST 'SanitizerCommon-asan-x86_64-Linux :: print-stack-trace.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';      /data/clang-builds/compiler-rt-embed-symbolizer-tk2/./bin/clang  --driver-mode=g++ -gline-tables-only -fsanitize=address  -m64  -ldl -O0 /home/dgliner/Documents/llvm-project/compiler-rt/test/sanitizer_common/TestCases/print-stack-trace.cpp -o /data/clang-builds/compiler-rt-embed-symbolizer-tk2/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/print-stack-trace.cpp.tmp && env ASAN_OPTIONS=stack_trace_format=DEFAULT  /data/clang-builds/compiler-rt-embed-symbolizer-tk2/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/print-stack-trace.cpp.tmp 2>&1 | FileCheck /home/dgliner/Documents/llvm-project/compiler-rt/test/sanitizer_common/TestCases/print-stack-trace.cpp
: 'RUN: at line 2';      /data/clang-builds/compiler-rt-embed-symbolizer-tk2/./bin/clang  --driver-mode=g++ -gline-tables-only -fsanitize=address  -m64  -ldl -O3 /home/dgliner/Documents/llvm-project/compiler-rt/test/sanitizer_common/TestCases/print-stack-trace.cpp -o /data/clang-builds/compiler-rt-embed-symbolizer-tk2/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/print-stack-trace.cpp.tmp && env ASAN_OPTIONS=stack_trace_format=DEFAULT  /data/clang-builds/compiler-rt-embed-symbolizer-tk2/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/print-stack-trace.cpp.tmp 2>&1 | FileCheck /home/dgliner/Documents/llvm-project/compiler-rt/test/sanitizer_common/TestCases/print-stack-trace.cpp
: 'RUN: at line 3';   env ASAN_OPTIONS=stack_trace_format=frame%n_lineno%l  /data/clang-builds/compiler-rt-embed-symbolizer-tk2/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/print-stack-trace.cpp.tmp 2>&1 | FileCheck /home/dgliner/Documents/llvm-project/compiler-rt/test/sanitizer_common/TestCases/print-stack-trace.cpp --check-prefix=CUSTOM
: 'RUN: at line 4';   env ASAN_OPTIONS=symbolize_inline_frames=false:stack_trace_format=DEFAULT  /data/clang-builds/compiler-rt-embed-symbolizer-tk2/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/print-stack-trace.cpp.tmp 2>&1 | FileCheck /home/dgliner/Documents/llvm-project/compiler-rt/test/sanitizer_common/TestCases/print-stack-trace.cpp --check-prefix=NOINLINE
--
Exit Code: 1

Command Output (stderr):
--
/home/dgliner/Documents/llvm-project/compiler-rt/test/sanitizer_common/TestCases/print-stack-trace.cpp:29:14: error: NOINLINE: expected string not found in input
// NOINLINE: #1 0x{{.*}} in main{{.*}}print-stack-trace.cpp:[[@LINE-15]]
             ^
<stdin>:1:47: note: scanning from here
 #0 0x4be298 in __sanitizer_print_stack_trace /home/dgliner/Documents/llvm-project/compiler-rt/lib/asan/asan_stack.cpp:86
                                              ^
<stdin>:1:47: note: with "@LINE-15" equal to "14"
 #0 0x4be298 in __sanitizer_print_stack_trace /home/dgliner/Documents/llvm-project/compiler-rt/lib/asan/asan_stack.cpp:86
                                              ^
<stdin>:5:123: note: possible intended match here
 #4 0x41f1a9 in _start (/data/clang-builds/compiler-rt-embed-symbolizer-tk2/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Output/print-stack-trace.cpp.tmp+0x41f1a9)
                                                                                                                          ^

--

********************
********************
Failing Tests (2):
  SanitizerCommon-asan-x86_64-Linux :: print-stack-trace.cpp
  SanitizerCommon-asan-x86_64-Linux :: symbolize_pc_inline.cpp

> ninja check-sanitizer
...
********************
^C  interrupted by user, skipping remaining tests
********************
Failing Tests (25):
  SanitizerCommon-asan-x86_64-Linux :: Linux/soft_rss_limit_mb_test.cpp
  SanitizerCommon-asan-x86_64-Linux :: print-stack-trace.cpp
  SanitizerCommon-asan-x86_64-Linux :: symbolize_debug_argv.cpp
  SanitizerCommon-asan-x86_64-Linux :: symbolize_pc_inline.cpp
  SanitizerCommon-lsan-x86_64-Linux :: print-stack-trace.cpp
  SanitizerCommon-lsan-x86_64-Linux :: symbolize_debug_argv.cpp
  SanitizerCommon-lsan-x86_64-Linux :: symbolize_pc_inline.cpp
  SanitizerCommon-msan-x86_64-Linux :: print-stack-trace.cpp
  SanitizerCommon-msan-x86_64-Linux :: symbolize_debug_argv.cpp
  SanitizerCommon-msan-x86_64-Linux :: symbolize_pc_inline.cpp
  SanitizerCommon-tsan-x86_64-Linux :: Linux/aligned_alloc-alignment.cpp
  SanitizerCommon-tsan-x86_64-Linux :: Linux/allow_user_segv.cpp
  SanitizerCommon-tsan-x86_64-Linux :: Linux/assert.cpp
  SanitizerCommon-tsan-x86_64-Linux :: Linux/ill.cpp
  SanitizerCommon-tsan-x86_64-Linux :: Linux/mmap_write_exec.cpp
  SanitizerCommon-tsan-x86_64-Linux :: Linux/pvalloc-overflow.cpp
  SanitizerCommon-tsan-x86_64-Linux :: Linux/signal_line.cpp
  SanitizerCommon-tsan-x86_64-Linux :: Posix/dedup_token_length_test.cpp
  SanitizerCommon-tsan-x86_64-Linux :: Posix/dump_instruction_bytes.cpp
  SanitizerCommon-tsan-x86_64-Linux :: Posix/fpe.cpp
  SanitizerCommon-tsan-x86_64-Linux :: Posix/posix_memalign-alignment.cpp
  SanitizerCommon-tsan-x86_64-Linux :: Posix/sanitizer_set_death_callback_test.cpp
  SanitizerCommon-tsan-x86_64-Linux :: allocator_returns_null.cpp
  SanitizerCommon-tsan-x86_64-Linux :: max_allocation_size.cpp
  SanitizerCommon-tsan-x86_64-Linux :: onprint.cpp


Testing Time: 187.90s
  Skipped Tests      : 185
  Unsupported Tests  : 217
  Expected Passes    : 591
  Expected Failures  :   8
  Unexpected Failures:  25

Is there a step in the build-bot job that I'm missing when running locally?

[vitalybuka]

These tests makes me think that maybe our bot is not running what is expected. I'll check.

[dgg5503]

In D79280#2021148, @vitalybuka wrote:

These tests makes me think that maybe our bot is not running what is expected. I'll check.

Hi @vitalybuka, any updates on this?

[vitalybuka]

In D79280#2074013, @dgg5503 wrote:

In D79280#2021148, @vitalybuka wrote:

These tests makes me think that maybe our bot is not running what is expected. I'll check.

Hi @vitalybuka, any updates on this?

Hi,
Sorry for delay.
So it's true, the test is not executed on the bot. I'll fix the bot if you decide to land the patch.

The patch LGTM.

[dgg5503]

Thanks for the review & approval @vitalybuka! Before I request someone to commit this patch, can you please take a look at my response to your concern regarding the changes made to __sanitizer_symbolize_code?

Out of curiosity, where are the steps executed by the bot stored? Is this something that contributors can submit changes to or is it only managed by the bot creator? I'd be interested in helping if possible.

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp

325

I did a grep of the entire LLVM code base as of 2565581e3704a9418e024e4fb8575ec8b14dce5d and couldn't find anything calling this function that isn't covered in this patch aside from maybe these binary files:

$ grep -r "__sanitizer_symbolize_code" .
Binary file ./llvm/test/tools/sancov/Inputs/test-linux_x86_64 matches
Binary file ./llvm/test/tools/llvm-xray/X86/Inputs/elf64-sample-o2.bin matches
Binary file ./llvm/test/tools/llvm-xray/X86/Inputs/elf64-badentrysizes.bin matches
Binary file ./llvm/test/tools/llvm-xray/X86/Inputs/elf64-example.bin matches
...

Is the concern here regarding those who defined this function in their own implementations outside of LLVM? If so, should I pass SymbolizeInlineFrames as a standalone variable which can be imported by the library implementing these functions?

[vitalybuka]

In D79280#2107319, @dgg5503 wrote:

Thanks for the review & approval @vitalybuka! Before I request someone to commit this patch, can you please take a look at my response to your concern regarding the changes made to __sanitizer_symbolize_code?

Out of curiosity, where are the steps executed by the bot stored? Is this something that contributors can submit changes to or is it only managed by the bot creator? I'd be interested in helping if possible.

https://github.com/llvm/llvm-zorg/tree/master/zorg/buildbot/builders/sanitizers

More exactly it's https://github.com/llvm/llvm-zorg/blob/3489cb51be6b0f4924c8e889808db79ec75d33ab/zorg/buildbot/builders/sanitizers/buildbot_cmake.sh#L268

This place should cover them, but looks like I switched to something else and forgot :(
https://github.com/llvm/llvm-zorg/blob/3489cb51be6b0f4924c8e889808db79ec75d33ab/zorg/buildbot/builders/sanitizers/buildbot_cmake.sh#L323

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp

325

Concern is about some external users of LLVM, they may redefine __sanitizer_symbolize_code with the current interface. However I suspect probability is close to 0. E.g. we had one legacy place in google code and I just removed that reference.

[MaskRay]

Hi, your git commit contains extra Phabricator tags. You can drop Reviewers: Subscribers: Tags: and the text Summary: from the git commit with the following script:

arcfilter () {
        arc amend
        git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: /,"");print}' | git commit --amend --date=now -F -
}

Reviewed By: is considered important by some people. Please keep the tag. (I have updated my script to use --date=now (setting author date to committer date))

https://reviews.llvm.org/D80978 contains a git pre-push hook to automate this.

[vitalybuka]

In D79280#2110634, @MaskRay wrote:

Hi, your git commit contains extra Phabricator tags. You can drop Reviewers: Subscribers: Tags: and the text Summary: from the git commit with the following script:

arcfilter () {
        arc amend
        git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: /,"");print}' | git commit --amend --date=now -F -
}

Reviewed By: is considered important by some people. Please keep the tag. (I have updated my script to use --date=now (setting author date to committer date))

https://reviews.llvm.org/D80978 contains a git pre-push hook to automate this.

thanks, installed the hook

compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp

private:
   LowLevelAllocator *allocator_;
   InternalMmapVector<Addr2LineProcess*> addr2line_pool_;
   static const uptr dummy_address_ =
       FIRST_32_SECOND_64(UINT32_MAX, UINT64_MAX);
 };
 
 #if SANITIZER_SUPPORTS_WEAK_HOOKS
 extern "C" {
-SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
-bool __sanitizer_symbolize_code(const char *ModuleName, u64 ModuleOffset,
-                                char *Buffer, int MaxLength);
+SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE bool
+__sanitizer_symbolize_code(const char *ModuleName, u64 ModuleOffset,
+                           char *Buffer, int MaxLength,
+                           bool SymbolizeInlineFrames);
 SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
 bool __sanitizer_symbolize_data(const char *ModuleName, u64 ModuleOffset,
                                 char *Buffer, int MaxLength);
 SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
 void __sanitizer_symbolize_flush();
 SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
 int __sanitizer_symbolize_demangle(const char *Name, char *Buffer,
                                    int MaxLength);
 }  // extern "C"
 
 class InternalSymbolizer : public SymbolizerTool {
  public:
   static InternalSymbolizer *get(LowLevelAllocator *alloc) {
     if (__sanitizer_symbolize_code != 0 &&
         __sanitizer_symbolize_data != 0) {
       return new(*alloc) InternalSymbolizer();
     }
     return 0;
   }
 
   bool SymbolizePC(uptr addr, SymbolizedStack *stack) override {
     bool result = __sanitizer_symbolize_code(
-        stack->info.module, stack->info.module_offset, buffer_, kBufferSize);
+        stack->info.module, stack->info.module_offset, buffer_, kBufferSize,
+        common_flags()->symbolize_inline_frames);
     if (result) ParseSymbolizePCOutput(buffer_, stack);
     return result;
   }
 
   bool SymbolizeData(uptr addr, DataInfo *info) override {
     bool result = __sanitizer_symbolize_data(info->module, info->module_offset,
                                              buffer_, kBufferSize);
     if (result) {

compiler-rt/lib/sanitizer_common/symbolizer/sanitizer_symbolize.cpp

 //===-- sanitizer_symbolize.cpp ---------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Implementation of weak hooks from sanitizer_symbolizer_posix_libcdep.cpp.
 //
 //===----------------------------------------------------------------------===//
 
 #include <stdio.h>
+
 #include <string>
 
 #include "llvm/DebugInfo/Symbolize/DIPrinter.h"
 #include "llvm/DebugInfo/Symbolize/Symbolize.h"
 
 static llvm::symbolize::LLVMSymbolizer *getDefaultSymbolizer() {
   static llvm::symbolize::LLVMSymbolizer *DefaultSymbolizer =
       new llvm::symbolize::LLVMSymbolizer();
   return DefaultSymbolizer;
 }
 
 namespace __sanitizer {
 int internal_snprintf(char *buffer, unsigned long length, const char *format,
                       ...);
 }  // namespace __sanitizer
 
 extern "C" {
 
 typedef uint64_t u64;
 
 bool __sanitizer_symbolize_code(const char *ModuleName, uint64_t ModuleOffset,
-                                char *Buffer, int MaxLength) {
+                                char *Buffer, int MaxLength,
+                                bool SymbolizeInlineFrames) {
   std::string Result;
   {
     llvm::raw_string_ostream OS(Result);
     llvm::symbolize::DIPrinter Printer(OS);
     // TODO: it is neccessary to set proper SectionIndex here.
     // object::SectionedAddress::UndefSection works for only absolute addresses.
+    if (SymbolizeInlineFrames) {
       auto ResOrErr = getDefaultSymbolizer()->symbolizeInlinedCode(
           ModuleName,
           {ModuleOffset, llvm::object::SectionedAddress::UndefSection});
       Printer << (ResOrErr ? ResOrErr.get() : llvm::DIInliningInfo());
+    } else {
+      auto ResOrErr = getDefaultSymbolizer()->symbolizeCode(
+          ModuleName,
+          {ModuleOffset, llvm::object::SectionedAddress::UndefSection});
+      Printer << (ResOrErr ? ResOrErr.get() : llvm::DILineInfo());
+    }
   }
   return __sanitizer::internal_snprintf(Buffer, MaxLength, "%s",
                                         Result.c_str()) < MaxLength;
 }
 
 bool __sanitizer_symbolize_data(const char *ModuleName, uint64_t ModuleOffset,
                                 char *Buffer, int MaxLength) {
   std::string Result;