[symbolizer] Treat invalid address as addr2line does

[sepavloff]

Summary

llvm-symbolizer echoed input if it was not recognized as a valid address.
This behavior was extended to llvm-addr2line as well. GNU addr2line in
this case optputs "??:0". This difference prevents implementation of
symbol+offset lookup available in the recent versions of GNU binutils.
In that case a string that is not an address may be a symbol.

This change make reaction of llvm-addr2line on unrecognized input closer
to GNU addr2line.

[MaskRay]

Thanks. I have verified that GNU addr2line has the ??:0 behavior.

[sepavloff]

Thanks!

[jhenderson]

Mostly looks good to me, but please see inline comments.

llvm/test/tools/llvm-symbolizer/input-base.test

29–30

These checks should be deleted, right?

llvm/test/tools/llvm-symbolizer/invalid-input-address.test

20

If I'm not mistaken, the different behaviour is controlled by the --output-style option and llvm-addr2line just happens to have the right default value for this to work. Is it worth explicitly testing llvm-symbolizer --output-style=GNU for this case?

[sepavloff]

Add tests for llvm-symbolizer in GNU output mode

[sepavloff]

llvm/test/tools/llvm-symbolizer/input-base.test

29–30

These are used in test on line 11.

llvm/test/tools/llvm-symbolizer/invalid-input-address.test

20

Added such tests (lines 17-20).

[jhenderson]

Thanks for the new cases, but their output actually surprised me. I'd expect GNU output style to render invalid inputs as ??:0 (and equivalently, llvm-addr2line with non-GNU output to echo the input). I'm not too fussed by it, but I would have thought that this comes under "output style" really.

[sepavloff]

Committed in: https://reviews.llvm.org/rG27c4777f41d2.

llvm/include/llvm/DebugInfo/Symbolize/DIPrinter.h

 
 class GNUPrinter : public PlainPrinterBase {
 private:
   void printSimpleLocation(StringRef Filename, const DILineInfo &Info) override;
 
 public:
   GNUPrinter(raw_ostream &OS, raw_ostream &ES, PrinterConfig &Config)
       : PlainPrinterBase(OS, ES, Config) {}
+
+  void printInvalidCommand(const Request &Request, StringRef Command) override;
 };
 
 class JSONPrinter : public DIPrinter {
 private:
   raw_ostream &OS;
   PrinterConfig Config;
   std::unique_ptr<json::Array> ObjectList;
 

llvm/lib/DebugInfo/Symbolize/DIPrinter.cpp

void GNUPrinter::printSimpleLocation(StringRef Filename,
   OS << Filename << ':' << Info.Line;
   if (Info.Discriminator)
     OS << " (discriminator " << Info.Discriminator << ')';
   OS << '\n';
   printContext(
       SourceCode(Filename, Info.Line, Config.SourceContextLines, Info.Source));
 }
 
+void GNUPrinter::printInvalidCommand(const Request &Request,
+                                     StringRef Command) {
+  OS << "??:0\n";
+}
+
 void PlainPrinterBase::printVerbose(StringRef Filename,
                                     const DILineInfo &Info) {
   OS << "  Filename: " << Filename << '\n';
   if (Info.StartLine) {
     OS << "  Function start filename: " << Info.StartFileName << '\n';
     OS << "  Function start line: " << Info.StartLine << '\n';
   }
   printStartAddress(Info);

llvm/test/tools/llvm-symbolizer/flush-output.s

 # REQUIRES: x86-registered-target
 
 ## If a process spawns llvm-symbolizer, and wishes to feed it addresses one at a
 ## time, llvm-symbolizer needs to flush its output after each input has been
 ## processed or the parent process will not be able to read the output and may
 ## deadlock. This test runs a script that simulates this situation for both a
 ## a good and bad input.
 
 foo:
     nop
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o -g
 # RUN: %python %p/Inputs/flush-output.py llvm-symbolizer %t.o \
 # RUN:   | FileCheck %s
 
 # CHECK: flush-output.s:10
-# CHECK: bad
+# CHECK: ??:0

llvm/test/tools/llvm-symbolizer/input-base.test

 
 # llvm-addr2line always requires hexadecimal, but accepts an optional 0x prefix.
 RUN: llvm-addr2line -e /dev/null -a 0x1234 | FileCheck %s
 RUN: llvm-addr2line -e /dev/null -a 0X1234 | FileCheck %s
 RUN: llvm-addr2line -e /dev/null -a 1234 | FileCheck %s
 RUN: llvm-addr2line -e /dev/null -a 01234 | FileCheck %s
 RUN: llvm-addr2line -e /dev/null -a 0b1010 | FileCheck %s --check-prefix=HEXADECIMAL-NOT-BINARY
 RUN: llvm-addr2line -e /dev/null -a 0B1010 | FileCheck %s --check-prefix=HEXADECIMAL-NOT-BINARY
-RUN: llvm-addr2line -e /dev/null -a 0o1234 | FileCheck %s --check-prefix=INVALID-NOT-OCTAL-LOWER
-RUN: llvm-addr2line -e /dev/null -a 0O1234 | FileCheck %s --check-prefix=INVALID-NOT-OCTAL-UPPER
+RUN: llvm-addr2line -e /dev/null -a 0o1234 | FileCheck %s --check-prefix=INVALID-NOT-OCTAL
+RUN: llvm-addr2line -e /dev/null -a 0O1234 | FileCheck %s --check-prefix=INVALID-NOT-OCTAL
 
 CHECK: 0x1234
 CHECK-NEXT: ??
 
 HEXADECIMAL-NOT-BINARY: 0xb1010
 HEXADECIMAL-NOT-BINARY: ??
 
-INVALID-NOT-OCTAL-LOWER: 0o1234
-INVALID-NOT-OCTAL-LOWER-NOT: ??
-
 INVALID-NOT-OCTAL-UPPER: 0O1234
 INVALID-NOT-OCTAL-UPPER-NOT: ??
+
+INVALID-NOT-OCTAL: ??:0

llvm/test/tools/llvm-symbolizer/invalid-input-address.test

 # Use address that can't fit in a 64-bit number. Show that llvm-symbolizer
 # simply echoes it as per other malformed input addresses.
 RUN: llvm-symbolizer --obj=addr.exe 0x10000000000000000 | FileCheck --check-prefix=LARGE-ADDR %s
 
 LARGE-ADDR-NOT: {{.}}
 LARGE-ADDR:     0x10000000000000000
 LARGE-ADDR-NOT: {{.}}
 
 RUN: echo '"some text"' '"some text2"' > %t.rsp
 RUN: echo -e 'some text\nsome text2\n' > %t.inp
 
 # Test bad input address values, via stdin, command line and response file.
 RUN: llvm-symbolizer --obj=%p/Inputs/addr.exe < %t.inp | FileCheck --check-prefix=BAD-INPUT %s
 RUN: llvm-symbolizer --obj=%p/Inputs/addr.exe "some text" "some text2" | FileCheck --check-prefix=BAD-INPUT %s
 RUN: llvm-symbolizer --obj=%p/Inputs/addr.exe @%t.rsp | FileCheck --check-prefix=BAD-INPUT %s
 
-# Test bad input address values for the GNU-compatible version.
-RUN: llvm-addr2line --obj=%p/Inputs/addr.exe < %t.inp | FileCheck --check-prefix=BAD-INPUT %s
-RUN: llvm-addr2line --obj=%p/Inputs/addr.exe "some text" "some text2" | FileCheck --check-prefix=BAD-INPUT %s
-RUN: llvm-addr2line --obj=%p/Inputs/addr.exe @%t.rsp | FileCheck --check-prefix=BAD-INPUT %s
-
 BAD-INPUT:      some text
 BAD-INPUT-NEXT: some text2
+
+# Test bad input address values for the GNU-compatible version.
+RUN: llvm-addr2line --obj=%p/Inputs/addr.exe < %t.inp | FileCheck --check-prefix=GNU-BAD-INPUT %s
+RUN: llvm-addr2line --obj=%p/Inputs/addr.exe "some text" "some text2" | FileCheck --check-prefix=GNU-BAD-INPUT %s
+RUN: llvm-addr2line --obj=%p/Inputs/addr.exe @%t.rsp | FileCheck --check-prefix=GNU-BAD-INPUT %s
+
+GNU-BAD-INPUT:      ??:0
+GNU-BAD-INPUT-NEXT: ??:0

llvm/test/tools/llvm-symbolizer/options-from-env.test

-# RUN: env LLVM_SYMBOLIZER_OPTS='0 1 --verbose' llvm-symbolizer 2 | FileCheck %s
-# RUN: env LLVM_ADDR2LINE_OPTS='0 1 --verbose' llvm-addr2line 2 | FileCheck %s
+# RUN: env LLVM_SYMBOLIZER_OPTS='0x400540 0x40054d --verbose' llvm-symbolizer -e %p/Inputs/addr.exe --no-inlines 0x40055e | FileCheck %s
+# RUN: env LLVM_ADDR2LINE_OPTS='400540 40054d --verbose' llvm-addr2line -e %p/Inputs/addr.exe 40055e | FileCheck %s
 
-# CHECK:      0
-# CHECK-NEXT: 1
-# CHECK-NEXT: 2
+# CHECK:       Filename: /tmp{{[\\/]}}x.c
+# CHECK-NEXT:  Function start filename: /tmp{{[\\/]}}x.c
+# CHECK-NEXT:  Function start line: 2
+# CHECK:       Line: 3
+# CHECK-NEXT:  Column: 3
+
+# CHECK:       Filename: /tmp{{[\\/]}}x.c
+# CHECK-NEXT:  Function start filename: /tmp{{[\\/]}}x.c
+# CHECK-NEXT:  Function start line: 2
+# CHECK:       Line: 3
+# CHECK-NEXT:  Column: 3
+
+# CHECK:       Filename: /tmp{{[\\/]}}x.c
+# CHECK-NEXT:  Function start filename: /tmp{{[\\/]}}x.c
+# CHECK-NEXT:  Function start line: 6
+# CHECK:       Line: 7
+# CHECK-NEXT:  Column: 3

llvm/test/tools/llvm-symbolizer/output-style-empty-line.test

 RUN: llvm-addr2line --output-style=LLVM -i -e %p/Inputs/addr.exe < %p/Inputs/addr.inp \
 RUN:   | FileCheck %s --check-prefix=LLVM
 
 LLVM: x.c:14:0
 LLVM-EMPTY:
 LLVM-NEXT: some text2
 
 GNU: x.c:14
-GNU-NEXT: some text2
+GNU-NEXT: ??:0

llvm/test/tools/llvm-symbolizer/sym.test

 #PRETTY: {{[0x]+}}40054d: inctwo at {{[/\]+}}tmp{{[/\]+}}x.c:3:3
 #PRETTY:  (inlined by) inc at {{[/\]+}}tmp{{[/\]+}}x.c:7:0
 #PRETTY:  (inlined by) main at {{[/\]+}}tmp{{[/\]+}}x.c:14:0
 #PRETTY: some text2
 #
 #ZERO: ??
 #ZERO: ??:0:0
 #
-#A2L:         some text
+#A2L:         ??:0
 #A2L_A-NEXT:  0x40054d
 #A2L_F-NEXT:  inctwo
 #A2L-NEXT:    {{[/\]+}}tmp{{[/\]+}}x.c:3{{$}}
 #A2L_FI-NEXT: inc{{$}}
 #A2L_I-NEXT:  {{[/\]+}}tmp{{[/\]+}}x.c:7{{$}}
 #A2L_FI-NEXT: main
 #A2L_I-NEXT:  {{[/\]+}}tmp{{[/\]+}}x.c:14{{$}}
-#A2L-NEXT:    some text2
+#A2L-NEXT:    ??:0
 
-#A2LP:          some text
+#A2LP:          ??:0
 #A2LP_A-NEXT:   0x40054d: {{[/\]+}}tmp{{[/\]+}}x.c:3{{$}}
 #A2LP_F-NEXT:   inctwo at {{[/\]+}}tmp{{[/\]+}}x.c:3{{$}}
 #A2LP_AF-NEXT:  0x40054d: inctwo at {{[/\]+}}tmp{{[/\]+}}x.c:3{{$}}
 #A2LP_I-NEXT:   {{[/\]+}}tmp{{[/\]+}}x.c:7{{$}}
 #A2LP_I-NEXT:   {{[/\]+}}tmp{{[/\]+}}x.c:14{{$}}
 #A2LP_FI-NEXT:   (inlined by) inc at {{[/\]+}}tmp{{[/\]+}}x.c:7{{$}}
 #A2LP_FI-NEXT:   (inlined by) main at {{[/\]+}}tmp{{[/\]+}}x.c:14{{$}}
-#A2LP-NEXT:     some text2
+#A2LP-NEXT:     ??:0