[PATCH] D88988: [llvm-symbolizer] Add inline stack traces for Windows.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: lld/test/COFF/symbolizer-inline.s:3-5
+# RUN: lld-link -entry:main -nodefaultlib %t.obj -out:%t.exe -pdb:%t.pdb -debug
+# RUN: llvm-symbolizer --obj=%t.exe --use-native-pdb-reader --relative-address \
+# RUN:   0x1014 0x1018 0x101c 0x1023 0x1024 | FileCheck %s
----------------
dblaikie wrote:
> After updated the dependencies (adding a dependency on llvm-symbolizer from lld's tests so this test could run 27e73816d6f9a7e627db73c445c4329db2ecfeaf ) that got  me looking at this/thinking: If this is the first use of llvm-symbolizer in lld, maybe it's out of place here?
> 
> Indeed this patch made no changes to lld, so it seems unsuitable that tests be added to lld - changes to llvm should be tested from within llvm. For ELF, at least, we're leaning towards writing hand-crafted assembly and the assembling that (with llvm-mc) and running llvm-symbolizer on the assembled file. If that model would work for COFF that'd be great - but otherwise it is acceptable to include source and repro steps in a file, and checkin a binary file for running llvm-symbolizer over.
IMO it is important for test readability that we start with assembly, not a checked in binary file. It allows us to come up with creative .cv_loc transitions from one instruction to the next, and validate that we get the right source location at each instruction boundary.

There is prior art for using llvm-mc to produce an object file in llvm and then using llvm-symbolizer on that object file, but it's impossible to do the same for COFF. llvm-symbolizer expects to operate on a PDB file. The only tool capable of making a PDB from an object right now is LLD.

While it's unfortunate that the test lives in the wrong repo, the great increase in testability makes it worth it to me. Debug info is historically undertested or only tested via interactive debugger integration tests. I think there is a huge amount of value to this level (medium size integration?) of testing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88988