[symbolizer] Exit early if input file is absent

[sepavloff]

Summary

If binary file specified as input with option --obj or -e is absent,
now llvm-addr2line exits immediately. This patch extends this behavior to
llvm-symbolizer. Previously llvm-symbolizer waited addresses from input
stream or command line in this case.

[jhenderson]

@MaskRay/@dblaikie, do you have any thoughts on this behaviour change?

llvm/test/tools/llvm-symbolizer/errors.test

16 ↗

(On Diff #532450)

Nit: delete blank line at EOF.

llvm/test/tools/llvm-symbolizer/output-style-json-code.test

7

This is possibly a case where we DO want the existing behaviour. The reasoning is that JSON output is intended to be 100% machine-readable.

[dblaikie]

In D153219#4431907, @jhenderson wrote:

@MaskRay/@dblaikie, do you have any thoughts on this behaviour change?

No strong feelings from me - doesn't seem like a super important thing to diverge from addr2line on, so probably god to be consistent?

[MaskRay]

LGTM. llvm-symbolizer --obj=unexisting's previous behavior of waiting on stdin seems a bit weird. Perhaps it's that way to mimic

% cat non-existent -
cat: non-existent: No such file or directory
<wait on stdin>

but I not sure that is a useful/desirable behavior...

There is a time-of-check-to-time-of-use issue, but probably not important.

[MaskRay]

LGTM.

llvm/test/tools/llvm-symbolizer/errors.test

1 ↗

(On Diff #532450)

Note: I've moved this to input-file-err.test as the new name seems more specific about we use this test file.

llvm/test/tools/llvm-symbolizer/output-style-json-code.test

7

If we want to keep the test but make it more useful, we can specify two addresses on the command line and see how the error is presented. With the changes, the output is one JSON object with Error.

[MaskRay]

If binary file specified as input with option --obj or -e was absent, [...]

The paragraph can be changed to say that now llvm-addr2line exits immediately. This patch extends the behavior to llvm-symbolizer.
Consider mentioning the behavior change for --output-style=JSON as well.

llvm-symbolizer --output-style=JSON -e nonexistent

[sepavloff]

Rebase the patch

[sepavloff]

llvm/test/tools/llvm-symbolizer/output-style-json-code.test

7

Both variants (old and new) are valid JSON files. When llvm-symbolizer processes address specified in command line, it also produces such JSON. I think this change in output could be considered as minor.

[sepavloff]

Fix PDB test

[jhenderson]

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

34–40

1. In this test, we don't use a comment marker before the CHECK lines
2. There's no need for two different check patterns. The output is identical.

llvm/test/tools/llvm-symbolizer/pdb/missing_pdb.test

1–3

I think the point of this test was (in part) to show llvm-symbolizer's stdout output too. If that is now empty, you should check that explicitly.

[sepavloff]

Fixed tests

[sepavloff]

llvm/test/tools/llvm-symbolizer/pdb/missing_pdb.test

1–3

Previously the error message went to stderr, and the result of recognition to stdout. Now only error message presents. Attempt to feed empty stream to FileCheck results in error: FileCheck error: '<stdin>' is empty.

[MaskRay]

llvm/test/tools/llvm-symbolizer/pdb/missing_pdb.test

1–3

If we want to explicitly test that stdout output is empty, we can use:

not llvm-symbolizer ... 2> %t.err | count 0
FileCheck --input-file=%t.err

[jhenderson]

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

34–40

Super-nit: delete the extra spacing between NOT-EXIST and llvm-symbolizer as per my previous inline edit.

llvm/test/tools/llvm-symbolizer/pdb/missing_pdb.test

4–5

This comment looks stale?

6

Rather than checking that some arbitrary error message is not emitted, better would be to make sure nothing is printed beyond what you expect, with --implicit-check-not={{.}}.

(This would have the added benefit of not needing the count 0 call, if you wanted, although I think @MaskRay's suggestion is cleaner for checking that stdout is empty.)

[sepavloff]

Fixed test

[sepavloff]

llvm/test/tools/llvm-symbolizer/pdb/missing_pdb.test

6

--implicit-check-not does not work, because input is empty and FileCheck complains, so used the method proposed by @MaskRay .

[jhenderson]

llvm/test/tools/llvm-symbolizer/pdb/missing_pdb.test

3

As @MaskRay suggested, please use --input-file rather than stdin redirection: it is easier to manually run this command using the suggested syntax.

6

You misunderstand me. I meant adding --implicit-check-not in the FileCheck line that checked the combined stderr and stdout (which of necessity wouldn't be empty). However, I don't think it's necessary given you've adopted @MaskRay's suggestion.

[sepavloff]

Fixed test

[jhenderson]

LGTM.

[sepavloff]

Thanks!

llvm/test/DebugInfo/Symbolize/ELF/symtab-file2.yaml

Symbols:
   - Name:    weak
     Binding: STB_WEAK
     Section: .text
     Value:   2
 
 ## If st_name of the STT_FILE symbols is invalid, the symbol name is lost as well.
 ## TODO Keep the symbol name.
 # RUN: yaml2obj --docnum=2 %s -o %t2
-# RUN: llvm-symbolizer --obj=%t2 0 0 2>&1 | FileCheck %s --check-prefix=CHECK2
+# RUN: not llvm-symbolizer --obj=%t2 0 0 2>&1 | FileCheck %s --check-prefix=CHECK2
 
 # CHECK2:      llvm-symbolizer{{.*}}: error: '{{.*}}symtab-file2.yaml.tmp2': st_name (0xffff) is past the end of the string table of size
-# CHECK2-NEXT: ??
-# CHECK2-NEXT: ??:0:0
-# CHECK2-EMPTY:
-# CHECK2-NEXT: ??
-# CHECK2-NEXT: ??:0:0
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_DYN
   Machine: EM_X86_64
 Sections:

llvm/test/DebugInfo/symbolize-macho-universal-unknown-arch.test

-RUN: llvm-symbolizer --obj=%p/Inputs/macho-universal 0x1f84 | FileCheck %s
+RUN: not llvm-symbolizer --obj=%p/Inputs/macho-universal 0x1f84 2>&1 | FileCheck %s
 
 CHECK-NOT: main
-CHECK: ??
+CHECK: {{.*}}llvm-symbolizer{{.*}}: error: '{{.*}}macho-universal': Unknown architecture
 CHECK-NOT: main

llvm/test/DebugInfo/symbolize-missing-file.test

-RUN: llvm-symbolizer --obj=unexisting-file 0x1234 2>&1 | FileCheck -DMSG=%errc_ENOENT %s
-
-CHECK: llvm-symbolizer{{.*}}: error: 'unexisting-file': [[MSG]]

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

 # llvm-symbolizer infers the number base from the form of the address.
-RUN: llvm-symbolizer -e /dev/null -a 0x1234 | FileCheck %s
-RUN: llvm-symbolizer -e /dev/null -a 0X1234 | FileCheck %s
-RUN: llvm-symbolizer -e /dev/null -a 4660 | FileCheck %s
-RUN: llvm-symbolizer -e /dev/null -a 011064 | FileCheck %s
-RUN: llvm-symbolizer -e /dev/null -a 0b1001000110100 | FileCheck %s
-RUN: llvm-symbolizer -e /dev/null -a 0B1001000110100 | FileCheck %s
-RUN: llvm-symbolizer -e /dev/null -a 0o11064 | FileCheck %s
+RUN: llvm-symbolizer -e %p/Inputs/addr.exe -a 0x1234 | FileCheck %s
+RUN: llvm-symbolizer -e %p/Inputs/addr.exe -a 0X1234 | FileCheck %s
+RUN: llvm-symbolizer -e %p/Inputs/addr.exe -a 4660 | FileCheck %s
+RUN: llvm-symbolizer -e %p/Inputs/addr.exe -a 011064 | FileCheck %s
+RUN: llvm-symbolizer -e %p/Inputs/addr.exe -a 0b1001000110100 | FileCheck %s
+RUN: llvm-symbolizer -e %p/Inputs/addr.exe -a 0B1001000110100 | FileCheck %s
+RUN: llvm-symbolizer -e %p/Inputs/addr.exe -a 0o11064 | FileCheck %s
 
 # llvm-symbolizer / StringRef::getAsInteger only accepts the 0o prefix in lowercase.
-RUN: llvm-symbolizer -e /dev/null -a 0O1234 | FileCheck %s --check-prefix=INVALID-NOT-OCTAL-UPPER
+RUN: llvm-symbolizer -e %p/Inputs/addr.exe -a 0O1234 | FileCheck %s --check-prefix=INVALID-NOT-OCTAL-UPPER
 
 # llvm-addr2line always requires hexadecimal, but accepts an optional 0x prefix.
 RUN: llvm-addr2line -e %p/Inputs/addr.exe -a 0x1234 | FileCheck %s
 RUN: llvm-addr2line -e %p/Inputs/addr.exe -a 0X1234 | FileCheck %s
 RUN: llvm-addr2line -e %p/Inputs/addr.exe -a 1234 | FileCheck %s
 RUN: llvm-addr2line -e %p/Inputs/addr.exe -a 01234 | FileCheck %s
 RUN: llvm-addr2line -e %p/Inputs/addr.exe -a 0b1010 | FileCheck %s --check-prefix=HEXADECIMAL-NOT-BINARY
 RUN: llvm-addr2line -e %p/Inputs/addr.exe -a 0B1010 | FileCheck %s --check-prefix=HEXADECIMAL-NOT-BINARY

llvm/test/tools/llvm-symbolizer/input-file-err.test

 Failing on AIX due to D153595. The test expects a different error message from the one given on AIX.
 XFAIL: target={{.*}}-aix{{.*}}
 RUN: not llvm-addr2line -e %p/Inputs/nonexistent 0x12 2>&1 | FileCheck %s --check-prefix=CHECK-NONEXISTENT-A2L -DMSG=%errc_ENOENT
 RUN: not llvm-addr2line -e %p/Inputs/nonexistent 2>&1 | FileCheck %s --check-prefix=CHECK-NONEXISTENT-A2L -DMSG=%errc_ENOENT
 CHECK-NONEXISTENT-A2L: llvm-addr2line{{.*}}: error: '{{.*}}Inputs/nonexistent': [[MSG]]
 
+RUN: not llvm-symbolizer -e %p/Inputs/nonexistent 0x12 2>&1 | FileCheck %s --check-prefix=CHECK-NONEXISTENT -DMSG=%errc_ENOENT
+RUN: not llvm-symbolizer -e %p/Inputs/nonexistent 2>&1 | FileCheck %s --check-prefix=CHECK-NONEXISTENT -DMSG=%errc_ENOENT
+CHECK-NONEXISTENT: llvm-symbolizer{{.*}}: error: '{{.*}}Inputs/nonexistent': [[MSG]]
+
 RUN: not llvm-addr2line -e %p/Inputs 0x12 2>&1 | FileCheck %s --check-prefix=CHECK-DIRECTORY-A2L -DMSG=%errc_EISDIR
 RUN: not llvm-addr2line -e %p/Inputs 2>&1 | FileCheck %s --check-prefix=CHECK-DIRECTORY-A2L -DMSG=%errc_EISDIR
 CHECK-DIRECTORY-A2L: llvm-addr2line{{.*}}: error: '{{.*}}Inputs': [[MSG]]
 
-not llvm-addr2line --output-style=JSON -e %p/Inputs/nonexistent 2>&1 | FileCheck %s --check-prefix=NONEXIST-JSON -DMSG=%errc_ENOENT
-NONEXIST-JSON: {"Address":"0x0","Error":{"Message":"[[MSG]]"},"ModuleName":"{{.*}}nonexistent"}
+RUN: not llvm-symbolizer -e %p/Inputs 0x12 2>&1 | FileCheck %s --check-prefix=CHECK-DIRECTORY -DMSG=%errc_EISDIR
+RUN: not llvm-symbolizer -e %p/Inputs 2>&1 | FileCheck %s --check-prefix=CHECK-DIRECTORY -DMSG=%errc_EISDIR
+CHECK-DIRECTORY: llvm-symbolizer{{.*}}: error: '{{.*}}Inputs': [[MSG]]

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
+RUN: llvm-symbolizer --obj=%p/Inputs/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
 

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

 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:      llvm-symbolizer{{.*}}: error: '{{.*}}Inputs/not.exist': [[MSG]]
-# NOT-EXIST-GNU-NEXT: ??
-# NOT-EXIST-GNU-NEXT: ??:0
-# NOT-EXIST-GNU-NEXT: ??
-# NOT-EXIST-GNU-NEXT: ??:0
-
-# NOT-EXIST-LLVM:       llvm-symbolizer{{.*}}: error: '{{.*}}Inputs/not.exist': [[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
+RUN: not llvm-symbolizer --output-style=GNU --obj=%p/Inputs/not.exist 0x1 0x2 --no-inlines 2>&1 \
+RUN:   | FileCheck %s --check-prefix=NOT-EXIST -DMSG=%errc_ENOENT
+RUN: not llvm-symbolizer --output-style=LLVM --obj=%p/Inputs/not.exist 0x1 0x2 --no-inlines 2>&1 \
+RUN:   | FileCheck %s --check-prefix=NOT-EXIST -DMSG=%errc_ENOENT
+
+NOT-EXIST: llvm-symbolizer{{.*}}: error: '{{.*}}Inputs/not.exist': [[MSG]]

llvm/test/tools/llvm-symbolizer/output-style-json-code.test

 ## This test checks JSON output for CODE.
 
 ## If the addresses are specified on the command-line, the output JSON will
 ## contain an array of the results for all of the given addresses.
 
 ## Show how library errors are reported in the output.
-# RUN: llvm-symbolizer --output-style=JSON -e %p/no-file.exe 0 | \
+# RUN: not llvm-symbolizer --output-style=JSON -e %p/no-file.exe 0 | \
 # RUN:   FileCheck %s -DMSG=%errc_ENOENT --check-prefix=NO-FILE --strict-whitespace --match-full-lines --implicit-check-not={{.}}
-# NO-FILE:[{"Address":"0x0","Error":{"Message":"[[MSG]]"},"ModuleName":"{{.*}}/no-file.exe"}]
+# NO-FILE:{"Address":"0x0","Error":{"Message":"[[MSG]]"},"ModuleName":"{{.*}}/no-file.exe"}
 
 ## Resolve out of range address.
 ## Expected a list with one empty object with default values.
 # RUN: llvm-symbolizer --output-style=JSON -e %p/Inputs/addr.exe 0x10000000 | \
 # RUN:   FileCheck %s --check-prefix=NOT-FOUND --strict-whitespace --match-full-lines --implicit-check-not={{.}}
 # NOT-FOUND:[{"Address":"0x10000000","ModuleName":"{{.*}}/Inputs/addr.exe","Symbol":[{"Column":0,"Discriminator":0,"FileName":"","FunctionName":"","Line":0,"StartAddress":"","StartFileName":"","StartLine":0}]}]
 
 ## Check a non-zero discriminator.

llvm/test/tools/llvm-symbolizer/pdb/missing_pdb.test

-RUN: llvm-symbolizer 0x401000 0x401001 --obj="%p/Inputs/missing_pdb.exe" 2>%t.err \
-RUN:    | FileCheck %s
-RUN: FileCheck -DMSG=%errc_ENOENT --check-prefix=ERROR %s < %t.err
+RUN: not llvm-symbolizer 0x401000 0x401001 --obj="%p/Inputs/missing_pdb.exe" 2> %t.err \
+RUN:    | count 0
+RUN: FileCheck -DMSG=%errc_ENOENT %s --input-file=%t.err
 
-llvm-symbolizer should print one error and two unknown line info records.
-
-ERROR: llvm-symbolizer{{.*}}: error: '{{.*}}missing_pdb.pdb': [[MSG]]
-ERROR-NOT: error reading file
-
-CHECK: ??
-CHECK: ??:0:0
-CHECK: ??
-CHECK: ??:0:0
+CHECK: llvm-symbolizer{{.*}}: error: '{{.*}}missing_pdb.pdb': [[MSG]]

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

 RUN: llvm-symbolizer --inlines --print-address --pretty-print --obj=%p/Inputs/addr.exe < %p/Inputs/addr.inp | FileCheck -check-prefix="PRETTY" %s
 RUN: llvm-symbolizer --inlines --print-address -p --obj=%p/Inputs/addr.exe < %p/Inputs/addr.inp | FileCheck -check-prefix="PRETTY" %s
 RUN: llvm-symbolizer -i --print-address --pretty-print --obj=%p/Inputs/addr.exe < %p/Inputs/addr.inp | FileCheck -check-prefix="PRETTY" %s
 RUN: llvm-symbolizer -i --print-address -p --obj=%p/Inputs/addr.exe < %p/Inputs/addr.inp | FileCheck -check-prefix="PRETTY" %s
 ## Before 2020-08-04, asan_symbolize.py passed --inlining=true.
 ## Support this compatibility alias for a while.
 RUN: llvm-symbolizer --inlining=true --print-address -p --obj=%p/Inputs/addr.exe < %p/Inputs/addr.inp | FileCheck -check-prefix="PRETTY" %s
 
-RUN: echo "0x1" > %t.input
-RUN: llvm-symbolizer --obj=%p/Inputs/zero < %t.input | FileCheck -check-prefix="ZERO" %s
-
 RUN: llvm-addr2line --obj=%p/Inputs/addr.exe < %p/Inputs/addr.inp | FileCheck -check-prefix=A2L %s
 RUN: llvm-addr2line -a --obj=%p/Inputs/addr.exe < %p/Inputs/addr.inp | FileCheck -check-prefixes=A2L,A2L_A %s
 RUN: llvm-addr2line -f --obj=%p/Inputs/addr.exe < %p/Inputs/addr.inp | FileCheck -check-prefixes=A2L,A2L_F %s
 RUN: llvm-addr2line -i --obj=%p/Inputs/addr.exe < %p/Inputs/addr.inp | FileCheck -check-prefixes=A2L,A2L_I %s
 RUN: llvm-addr2line -fi --obj=%p/Inputs/addr.exe < %p/Inputs/addr.inp | FileCheck -check-prefixes=A2L,A2L_F,A2L_I,A2L_FI %s
 
 RUN: llvm-addr2line -pa --obj=%p/Inputs/addr.exe < %p/Inputs/addr.inp | FileCheck -check-prefixes=A2LP,A2LP_A %s
 RUN: llvm-addr2line -pf --obj=%p/Inputs/addr.exe < %p/Inputs/addr.inp | FileCheck -check-prefixes=A2LP,A2LP_F %s

 # CHECK-NEXT: some text2
 #
 #PRETTY: some text
 #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_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{{$}}

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

#endif
   else if (Style == OutputStyle::JSON)
     Printer = std::make_unique<JSONPrinter>(outs(), Config);
   else
     Printer = std::make_unique<LLVMPrinter>(outs(), printError, Config);
 
   // When an input file is specified, exit immediately if the file cannot be
   // read. If getOrCreateModuleInfo succeeds, symbolizeInput will reuse the
   // cached file handle.
-  if (auto *Arg = Args.getLastArg(OPT_obj_EQ); Arg && IsAddr2Line) {
+  if (auto *Arg = Args.getLastArg(OPT_obj_EQ); Arg) {
     auto Status = Symbolizer.getOrCreateModuleInfo(Arg->getValue());
     if (!Status) {
       Request SymRequest = {Arg->getValue(), 0};
       handleAllErrors(Status.takeError(), [&](const ErrorInfoBase &EI) {
         Printer->printError(SymRequest, EI);
       });
       return EXIT_FAILURE;
     }