[llvm-symbolizer] - Fix the crash in GNU output style with --no-inlines and missing input file.

[grimar]

Summary

Fixes https://bugs.llvm.org/show_bug.cgi?id=48882.

If the input file does not exist (or has a reading error), the
following code will crash if there are two or more input addresses.

auto ResOrErr = Symbolizer.symbolizeInlinedCode(
  ModuleName, {Offset, object::SectionedAddress::UndefSection});
Printer << (error(ResOrErr) ? DILineInfo() : ResOrErr.get().getFrame(0));

For the first address, symbolizeInlinedCode returns an error.
For the second address, symbolizeInlinedCode returns an empty result
(not an error) and .getFrame(0) will crash.

[grimar]

llvm/test/tools/llvm-symbolizer/output-style-inlined.test

39

This might need an update if D95246 will be landed first.

[jhenderson]

llvm/test/tools/llvm-symbolizer/output-style-inlined.test

32

You probably need to explain in the comment why you use --no-inlines, as it's not obvious.

[MaskRay]

Perhaps clarify the description as following (I had had trouble understanding it until I debugged it).

If the input file does not exist (or has a reading error), the following code will crash if there are two or more input addresses.

auto ResOrErr = Symbolizer.symbolizeInlinedCode(
  ModuleName, {Offset, object::SectionedAddress::UndefSection});
Printer << (error(ResOrErr) ? DILineInfo() : ResOrErr.get().getFrame(0));

For the first address, symbolizeInlinedCode returns an error.
For the second address, symbolizeInlinedCode returns an empty result (not an error) and .getFrame(0) will crash.

I thought about whether non-first runs should return an error as well, but then I realized that the error cannot be easily serialized and stored, so returning an empty result should be fine.

[grimar]

• Addressed review comments.

[jhenderson]

LGTM. I've also approved D95246. That's a larger change, so perhaps let that land first, if you can then rebase and commit. If it hasn't landed before you need to be stopping work, feel free to land this first though, given you're away soon.

[grimar]

In D95609#2530142, @jhenderson wrote:

LGTM. I've also approved D95246. That's a larger change, so perhaps let that land first, if you can then rebase and commit. If it hasn't landed before you need to be stopping work, feel free to land this first though, given you're away soon.

Thanks! I can commit this patch during this weekends I think. Hope D95246 will be landed today and then worth giving it a bit of time to let bots to check it I guess.

[MaskRay]

Thanks!

[jhenderson]

@MaskRay, this is another that I think could benefit being in the LLVM 12 release branch. What do you think? If so, could you make the request to do so, please, given I'll be away the rest of this week.

[MaskRay]

In D95609#2568082, @jhenderson wrote:

@MaskRay, this is another that I think could benefit being in the LLVM 12 release branch. What do you think? If so, could you make the request to do so, please, given I'll be away the rest of this week.

Sounds good. Filed https://bugs.llvm.org/show_bug.cgi?id=49227

llvm/test/tools/llvm-symbolizer/output-style-inlined.test

 RUN: llvm-addr2line --output-style=GNU -f -e %p/Inputs/addr.exe 0x40054d \
 RUN:   | FileCheck %s --check-prefix=GNU --implicit-check-not=main
 
 RUN: llvm-addr2line --output-style=LLVM -f -e %p/Inputs/addr.exe 0x40054d \
 RUN:   | FileCheck %s --check-prefix=LLVM --implicit-check-not=inctwo
 
 LLVM: main
 GNU: inctwo
+
+## Check that we are able to produce an output properly when the --no-inlines option
+## is specified, but a file doesn't exist. Check we report an error.
+
+RUN: llvm-symbolizer --output-style=GNU --obj=%p/Inputs/not.exist 0x1 0x2 --no-inlines 2>&1 \
+RUN:   | FileCheck %s --check-prefix=NOT-EXIST-GNU -DMSG=%errc_ENOENT
+RUN: llvm-symbolizer --output-style=LLVM --obj=%p/Inputs/not.exist 0x1 0x2 --no-inlines 2>&1 \
+RUN:   | FileCheck %s --check-prefix=NOT-EXIST-LLVM -DMSG=%errc_ENOENT
+
+# NOT-EXIST-GNU:      LLVMSymbolizer: error reading file: [[MSG]]
+# NOT-EXIST-GNU-NEXT: ??
+# NOT-EXIST-GNU-NEXT: ??:0
+# NOT-EXIST-GNU-NEXT: ??
+# NOT-EXIST-GNU-NEXT: ??:0
+
+# NOT-EXIST-LLVM:       LLVMSymbolizer: error reading file: [[MSG]]
+# NOT-EXIST-LLVM-NEXT:  ??
+# NOT-EXIST-LLVM-NEXT:  ??:0:0
+# NOT-EXIST-LLVM-EMPTY:
+# NOT-EXIST-LLVM-NEXT:  ??
+# NOT-EXIST-LLVM-NEXT:  ??:0:0

llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp

if (Cmd == Command::Data) {
     // With PrintFunctions == FunctionNameKind::LinkageName (default)
     // and UseSymbolTable == true (also default), Symbolizer.symbolizeCode()
     // may override the name of an inlined function with the name of the topmost
     // caller function in the inlining chain. This contradicts the existing
     // behavior of addr2line. Symbolizer.symbolizeInlinedCode() overrides only
     // the topmost function, which suits our needs better.
     auto ResOrErr = Symbolizer.symbolizeInlinedCode(
         ModuleName, {Offset, object::SectionedAddress::UndefSection});
-    Printer << (error(ResOrErr) ? DILineInfo() : ResOrErr.get().getFrame(0));
+    if (!ResOrErr || ResOrErr->getNumberOfFrames() == 0) {
+      error(ResOrErr);
+      Printer << DILineInfo();
+    } else {
+      Printer << ResOrErr->getFrame(0);
+    }
   } else {
     auto ResOrErr = Symbolizer.symbolizeCode(
         ModuleName, {Offset, object::SectionedAddress::UndefSection});
     Printer << (error(ResOrErr) ? DILineInfo() : ResOrErr.get());
   }
   if (OutputStyle == DIPrinter::OutputStyle::LLVM)
     outs() << "\n";
 }