[Lldb-commits] [lldb] r252583 - The other half of a change made by Enrico for trying to get a correct

Todd Fiala via lldb-commits lldb-commits at lists.llvm.org
Tue Nov 10 12:39:04 PST 2015


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 <zturner at 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 at 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 at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>
>


-- 
-Todd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20151110/0a30c3c7/attachment.html>


More information about the lldb-commits mailing list