This is really the kind of thing that would be good to write a unit test for. There's a lot of institutional knowledge hidden away in these kinds of deep low level stuff, and a unit test is good documentation for it.
I suspect this is almost guaranteed to break at some point in the future without an explicit test (especially since it's not immediately obvious why a Target should behave that way) It should be really easy to write one for this. You'd need to make a TargetUnitTests target, create an empty target, set the triple to one thing, set it to another thing, and ensure it retains the original value. +todd in case you're interested in trying, he can probably help On Mon, Nov 9, 2015 at 8:14 PM Jason Molenda via lldb-commits < lldb-commits@lists.llvm.org> wrote: > Author: jmolenda > Date: Mon Nov 9 22:11:37 2015 > New Revision: 252583 > > URL: http://llvm.org/viewvc/llvm-project?rev=252583&view=rev > Log: > The other half of a change made by Enrico for trying to get a correct > triple for a process. He writes, "Changes to the way setting the > triple works on a target so that if the target has passed a fully > specified triple, and the newly passed triple is not a revamp of > the current one, and the current one is fully specified, then do > not replace the existing triple." > > Triple handling got a bit more complicated on mac with the addition > of ios/watchos/tvos and their simulators, and tracking the correct > os versions for them so expressions are compiled with the expected > APIs available to the user. > > <rdar://problem/19820698> > > Modified: > lldb/trunk/source/Target/Target.cpp > > Modified: lldb/trunk/source/Target/Target.cpp > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Target.cpp?rev=252583&r1=252582&r2=252583&view=diff > > ============================================================================== > --- lldb/trunk/source/Target/Target.cpp (original) > +++ lldb/trunk/source/Target/Target.cpp Mon Nov 9 22:11:37 2015 > @@ -1243,44 +1243,70 @@ bool > Target::SetArchitecture (const ArchSpec &arch_spec) > { > Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_TARGET)); > - if (m_arch.IsCompatibleMatch(arch_spec) || !m_arch.IsValid()) > + bool missing_local_arch = (false == m_arch.IsValid()); > + bool replace_local_arch = true; > + bool compatible_local_arch = false; > + ArchSpec other(arch_spec); > + > + if (!missing_local_arch) > + { > + if (m_arch.IsCompatibleMatch(arch_spec)) > + { > + other.MergeFrom(m_arch); > + > + if (m_arch.IsCompatibleMatch(other)) > + { > + compatible_local_arch = true; > + bool arch_changed, vendor_changed, os_changed, > os_ver_changed, env_changed; > + > + m_arch.PiecewiseTripleCompare(other, > + arch_changed, > + vendor_changed, > + os_changed, > + os_ver_changed, > + env_changed); > + > + if (!arch_changed && !vendor_changed && !os_changed) > + replace_local_arch = false; > + } > + } > + } > + > + if (compatible_local_arch || missing_local_arch) > { > - // If we haven't got a valid arch spec, or the architectures are > - // compatible, so just update the architecture. Architectures can > be > - // equal, yet the triple OS and vendor might change, so we need > to do > - // the assignment here just in case. > - m_arch = arch_spec; > + // If we haven't got a valid arch spec, or the architectures are > compatible > + // update the architecture, unless the one we already have is > more specified > + if (replace_local_arch) > + m_arch = other; > if (log) > - log->Printf ("Target::SetArchitecture setting architecture to > %s (%s)", arch_spec.GetArchitectureName(), > arch_spec.GetTriple().getTriple().c_str()); > + log->Printf ("Target::SetArchitecture set architecture to %s > (%s)", m_arch.GetArchitectureName(), > m_arch.GetTriple().getTriple().c_str()); > return true; > } > - else > + > + // If we have an executable file, try to reset the executable to the > desired architecture > + if (log) > + log->Printf ("Target::SetArchitecture changing architecture to %s > (%s)", arch_spec.GetArchitectureName(), > arch_spec.GetTriple().getTriple().c_str()); > + m_arch = other; > + ModuleSP executable_sp = GetExecutableModule (); > + > + ClearModules(true); > + // Need to do something about unsetting breakpoints. > + > + if (executable_sp) > { > - // If we have an executable file, try to reset the executable to > the desired architecture > if (log) > - log->Printf ("Target::SetArchitecture changing architecture to > %s (%s)", arch_spec.GetArchitectureName(), > arch_spec.GetTriple().getTriple().c_str()); > - m_arch = arch_spec; > - ModuleSP executable_sp = GetExecutableModule (); > - > - ClearModules(true); > - // Need to do something about unsetting breakpoints. > - > - if (executable_sp) > + log->Printf("Target::SetArchitecture Trying to select > executable file architecture %s (%s)", arch_spec.GetArchitectureName(), > arch_spec.GetTriple().getTriple().c_str()); > + ModuleSpec module_spec (executable_sp->GetFileSpec(), other); > + Error error = ModuleList::GetSharedModule (module_spec, > + executable_sp, > + > &GetExecutableSearchPaths(), > + NULL, > + NULL); > + > + if (!error.Fail() && executable_sp) > { > - if (log) > - log->Printf("Target::SetArchitecture Trying to select > executable file architecture %s (%s)", arch_spec.GetArchitectureName(), > arch_spec.GetTriple().getTriple().c_str()); > - ModuleSpec module_spec (executable_sp->GetFileSpec(), > arch_spec); > - Error error = ModuleList::GetSharedModule (module_spec, > - executable_sp, > - > &GetExecutableSearchPaths(), > - NULL, > - NULL); > - > - if (!error.Fail() && executable_sp) > - { > - SetExecutableModule (executable_sp, true); > - return true; > - } > + SetExecutableModule (executable_sp, true); > + return true; > } > } > return false; > > > _______________________________________________ > lldb-commits mailing list > lldb-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
