[Lldb-commits] [PATCH] D126367: [lldb] Add gnu-debuglink support for Windows PE/COFF

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun added a comment.

The test https://github.com/llvm/llvm-project/blob/main/lldb/test/Shell/Minidump/Windows/find-module.test fails with this patch. That test opens a minidump and checks that the associated exe is loaded. The minidump contains a codeview PDB record for the exe which is used to generate the UUID, but the same codeview PDB record is missing from the exe. The test still worked because LLDB ignores the empty UUID (it skips the UUID equality check). However, when this patch adds a UUID using the CRC, that test fails because the UUIDs no longer match up.

I see two ways of fixing it:

1. Add the codeview PDB record to the test exe. I already prepared the change, but I don't know if it is the expected behaviour (that the exe should have a matching codeview PDB record).

  diff
  diff --git a/lldb/test/Shell/Minidump/Windows/Inputs/find-module.exe.yaml b/lldb/test/Shell/Minidump/Windows/Inputs/find-module.exe.yaml
  index 42ccc6d6e053..c4e2b931c4a3 100644
  --- a/lldb/test/Shell/Minidump/Windows/Inputs/find-module.exe.yaml
  +++ b/lldb/test/Shell/Minidump/Windows/Inputs/find-module.exe.yaml
  @@ -16,6 +16,9 @@ OptionalHeader:
     SizeOfStackCommit: 4096
     SizeOfHeapReserve: 1048576
     SizeOfHeapCommit: 4096
  +  Debug:
  +    RelativeVirtualAddress: 20480
  +    Size:            28
   header:
     Machine:         IMAGE_FILE_MACHINE_I386
     Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_32BIT_MACHINE ]
  @@ -28,5 +31,10 @@ sections:
       Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
       VirtualAddress:  16384
       VirtualSize:     48
  +  - Name:            .buildid
  +    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
  +    VirtualAddress:  20480
  +    VirtualSize:     194
  +    SectionData:     00000000E5038E620000000002000000A60000001C5000001C400000525344533ED87D89C8A8184197F3A925EE4BF74101000000433A5C70726F6A656374735C746573745F6170705C436F6E736F6C654170706C69636174696F6E315C44656275675C436F6E736F6C654170706C69636174696F6E312E70646200
   symbols:         []
   ...



2. Don't set the UUID with the CRC. In SymbolVendorCOFF, don't rely on the UUID but instead implement a separate CRC check for debuglink (only necessary if there is no codeview PDB record). This may be problematic though -- `Symbols::LocateExecutableSymbolFile` has more than one search path and it can ignore files which doesn't match, but if I put the check in SymbolVendorCOFF it can only get the first matching filename, which may not actually be the correct match.

@labath I see you added the test in https://reviews.llvm.org/D65955, do you have any comment on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126367