Yeah, we can get some gtests in for this. I'll tail back on it. It'll be a bit before I can put cycles on it but it's on my list.
On Mon, Nov 9, 2015 at 9:46 PM, Zachary Turner <ztur...@google.com> wrote: > 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 >> > -- -Todd
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
