When loading a module as a result of the "target create" command, there is the potential for ObjectFilePECOFF to stomp the triple. This can confuse later code which depends on the triple. Ultimately this fixes load address resolution on Windows because the correct triple is plumbed all the way through, so DynamicLoaderWindows can now correctly recognize that it should be used.
Close. A couple changed as annotated in the source. There are two categories of unknown:
- specified "unknown" for the vendor or OS where the user actually typed "unknown" in the triple
- unspecified "unknown" for the vendor or OS where the user didn't specify it.
Add the quick fixes I specify above and this patch is good to go.
|1314 ↗||(On Diff #18586)|
We need to make sure the vendor wasn't specified as "unknown" by also calling:
bool ArchSpec::TripleVendorWasSpecified(). So the above line should be:
if (m_arch.GetTriple().getVendor() == llvm::Triple::UnknownVendor && m_arch.TripleVendorWasSpecified())
|1316 ↗||(On Diff #18586)|
Save thing as the previous, make sure "unknown" wasn't specified:
if (m_arch.GetTriple().getOS() == llvm::Triple::UnknownOS && m_arch.TripleOSWasSpecified())
Just to be clear, we should be checking that the triple vendor was NOT specified right? The code you wrote checks that it was specified.
Would it be worthwhile to make a function called ArchSpec::MergeFrom(const ArchSpec &other), which basically does what I've done here (with a comment explaining how it works)?
Fixed suggested issues, and moved the logic into a function in ArchSpec, since I thought it might be useful.
I reversed your conditional though because it looked like a thinko.
LMK if this looks good.