zequanwu added a comment.

In D137873#3931888 <https://reviews.llvm.org/D137873#3931888>, @mstorsjo wrote:

> In D137873#3930683 <https://reviews.llvm.org/D137873#3930683>, @labath wrote:
>
>> If that doesn't work then (besides knowing why), I'd like us try some kind 
>> of a single-setting solution, as I don't think it makes sense to have two 
>> different settings like this (happy to hear counterarguments to that)
>
> Overall, the ideal is if the user usually needs to twiddle one single 
> setting. For a majority of cases, picking the default format (like is done 
> for the objectfile setting, and afaik this one does too even though I didn't 
> quite clearly see the codepath for it) will be what you'd want: If you're 
> working with mingw tools, debugging with a mingw built lldb, then your app 
> that you're debugging also probably uses mingw ABIs. But it's clearly not 
> implausible to want to debug executables from the other ecosystem, so being 
> able to change it with a single setting is good too.
>
> And for the objectfile setting, it's possible to set it even on a per-DLL 
> basis. This may sound contrieved, but it's actually totally reasonable. While 
> the C++ ABIs are incompatible, such DLLs can interoperate over C APIs - this 
> is not an uncommon scenario. Plus, every mingw executable links against 
> msvcrt.dll or ucrtbase.dll (provided by the system) which use the MSVC C++ 
> ABIs internally (if you use the versions of them with debug info).
>
> So the ability to set it per DLL for the objectfile part is totally 
> warranted. How that fits in towards minidump reading, I don't really know 
> though (since the whole minidump contains stuff for the whole process). In 
> which cases does the C++ ABI form matter for the minidump btw - since the 
> actual interpretation of things still would be based on per-DLL debug info 
> (either as DWARF or PDB)?

lldb creates AST for user input expression which uses the abi from minidump 
process. That AST's class layout info is imported from another AST created from 
debug info that uses the abi specified in debug info. The current situation is 
that minidump process uses itanium ABI (because it's not specified in minidump, 
falls back to use itanium), but the debug info uses msvc ABI. The inconsistent 
causes crash when lowering class.

It looks like it's impossible to make it works with DLLs that have different 
ABIs, no matter which ABI we set for minidump. Unless we change how clang 
interacts with external layout: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/RecordLayoutBuilder.cpp#L1357.
 Maybe change it so that if using external layout failed due to mismatched ABI, 
try again with another ABI 
https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/RecordLayoutBuilder.cpp#L3302?



================
Comment at: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp:452
 ArchSpec ProcessMinidump::GetArchitecture() {
+  // "settings set plugin.process.minidump.abi" overrides the env in minidump.
+  ArchSpec arch = m_minidump_parser->GetArchitecture();
----------------
labath wrote:
> Is "overrides" the correct word here? Are there any circumstances in which we 
> are able to determine the environment from the minidump file? Because if it 
> is, then I would expect this to be the other way around (that the environment 
> from a specific file overrides the generic catch-all setting)...
Yeah, the word is not correct. It should be "sets". 
I think minidump file just doesn't have that environment info: 
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/BinaryFormat/Minidump.h#L161-L179.
 lldb sets the environment based on platform for android: 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp#L195.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137873

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to