[PATCH] D60147: [DWARF] check whether the DIE is valid before querying for information

Tue Apr 2 16:55:46 PDT 2019

dblaikie added inline comments.

================
Comment at: llvm/test/DebugInfo/dwarfdump-bad-lookup-address.test:1
+RUN: llvm-dwarfdump -lookup 1161 %p/Inputs/dwarfdump-test.macho-i386.o
----------------
dblaikie wrote:
> Testing that the program doesn't crash is a fairly broad test (what does it do instead? It could print all sorts of odd/weird/incorrect output while still passing this test) - could you test for the specific output that was hidden behind this crash previously?
Hmm, could you help me understand under what conditions the crash occurs? And why printing the compile_unit is the correct behavior here?

In other cases I've just tested, passing lookup a value it can't find - had a simple file with two functions ([0-6) and [16-22)). Querying for zero actually seems (probably unrelatedly) buggy - it prints all 3 DIEs (the CU and both subprograms). 

0: prints all 3 DIEs
[1-5]: prints the first subprogram DIE (& the CU parent)
[6-15]: crashes (probably this bug)
[16-21]: prints the second subprogram DIE (& the CU parent)
[22-...: prints the file format header and nothing else.

Hmm, I suppose, then, it makes sense to print the CU - because the address is within the range of the CU but not within the range of anything inside of it.

Maybe add a "CHECK-NOT: DW_TAG" at the end of the checks?
(& really it might not be necessary to check all the attributes - we know all that dumping works)
& a comment in the test file describing the purpose of the test value?

(& not sure - but perhaps this test would be easier to understand if it had its own test file, rather than using a larger/more complicated one from another test)

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60147/new/

https://reviews.llvm.org/D60147