[PATCH] D137873: [LLDB][Minidump] Set abi environment for windows.

Wed Nov 16 05:27:32 PST 2022

labath added a comment.

Adding a new setting isn't exactly what I had in mind. I was actually hoping that it would be enough to fiddle with that setting and leave the minidump environment blank (because we have code <https://github.com/llvm/llvm-project/blob/main/lldb/source/Target/Target.cpp#L1582> to fill in the missing pieces of a target ArchSpec from other sources.

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). I'd consider either moving that environment setting to some place more generic (so it can be accessed from both plugins) or just having the minidump plugin fetch the setting from inside ObjectFilePECOFF (I'd say that a process->object dependency is kinda OK, and we already have ProcessMachCore depending on ObjectFileMachO).

================
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();
----------------
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)...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137873