[Lldb-commits] [PATCH] D127048: [lldb] Set COFF and PDB module env from default target triple

Mon Jun 6 05:53:30 PDT 2022

alvinhochun added a comment.

In D127048#3559918 <https://reviews.llvm.org/D127048#3559918>, @mstorsjo wrote:

> In D127048#3559866 <https://reviews.llvm.org/D127048#3559866>, @DavidSpickett wrote:
>
>>> This changes the PE/COFF and PDB plugins to set the module triple according to the default target triple used to build LLDB.
>>
>> I'm missing some context here but using triple used when *building* lldb seems to conflict with the idea that the same lldb (client at least) can be used to debug many architectures.
>
> Yes, that's true. The idea here is that when doing local debugging in either the mingw or msvc ecosystems, the flavour of the lldb build itself could be a decent hint - but it's not indeed not a correct and faultproof fix.

Yes, the aim is to at least have a default that at least works natively for the default target in local debugging. Before this change, C++ debugging for mingw target just doesn't work at all from what I understand so far.

> @alvinhochun has got another patch in progress - https://reviews.llvm.org/D127053 (which isn't yet submitted for review as it lacks tests), which tries to decude the same based on whether a binary contains dwarf debug info.
>
> That's more fault proof, but is also not an entirely complete fix - while finding dwarf debug sections mostly can imply using the itanium C++ ABI, you can also use PDB files with itanium C++ ABI (i.e. in mingw setups). It's less common but still a viable scenario. And reversely, I think I've heard about special cases where people use dwarf debug info with MSVC-ecosystem builds too...
>
> So in short, what would be needed is a good enough heuristic that works for the vast majority of cases, and an option that lets users specify it for the cases that can't easily be autodetected.

I'm feeling wishy-washy on that patch because of the reason you stated. It may still be a good heuristic, but I think having an option to specify the ABI is a requirement before that patch can be landed.

I still don't have a clue how I can make it an option. It seems impossible to pass the option into `ObjectFilePECOFF::GetModuleSpecifications` cleanly.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127048