[Lldb-commits] [PATCH] D26804: Fix step-over when SymbolContext.function is missing and symbol is present.

[Jim Ingham via lldb-commits]

Yes, comp unit equal, no function and symbol equal should be treated as equivalent.

However, your check would call two SymbolContexts equivalent that had different CompUnits but the same symbol.  I can’t see how that would happen in practice, but if it did something odd has gone on, and we probably should stop and let the user have a look.  So can you change this to add a check for symbol equal inside the “have a comp and it’s equal” instead?

Jim  


> On Nov 17, 2016, at 9:24 AM, Sam McCall <sammccall at google.com> wrote:
> 
> sammccall created this revision.
> sammccall added a reviewer: jingham.
> sammccall added a subscriber: lldb-commits.
> 
> Fix step-over when SymbolContext.function is missing and symbol is present.
> 
> With targets from our build configuration,
> ThreadPlanStepOverRange::IsEquivalentContext fails to fire for relevant frames,
> leading to ShouldStop() returning true prematurely.
> 
> The frame's SymbolContext, and m_addr_context have:
> 
> - comp_unit set and matching
> - function = nullptr
> - symbol set and matching (but this is never checked)
> 
> My naive guess is that the context should be equivalent in this case :-)
> 
> 
> https://reviews.llvm.org/D26804
> 
> Files:
>  source/Target/ThreadPlanStepOverRange.cpp
> 
> 
> Index: source/Target/ThreadPlanStepOverRange.cpp
> ===================================================================
> --- source/Target/ThreadPlanStepOverRange.cpp
> +++ source/Target/ThreadPlanStepOverRange.cpp
> @@ -106,22 +106,22 @@
>   // in so I left out the target check.  And sometimes the module comes in as
>   // the .o file from the
>   // inlined range, so I left that out too...
> -  if (m_addr_context.comp_unit) {
> -    if (m_addr_context.comp_unit == context.comp_unit) {
> -      if (m_addr_context.function &&
> -          m_addr_context.function == context.function) {
> -        // It is okay to return to a different block of a straight function, we
> -        // only have to
> -        // be more careful if returning from one inlined block to another.
> -        if (m_addr_context.block->GetInlinedFunctionInfo() == nullptr &&
> -            context.block->GetInlinedFunctionInfo() == nullptr)
> -          return true;
> -
> -        if (m_addr_context.block && m_addr_context.block == context.block)
> -          return true;
> -      }
> +  if (m_addr_context.comp_unit &&
> +      m_addr_context.comp_unit == context.comp_unit) {
> +    if (m_addr_context.function &&
> +        m_addr_context.function == context.function) {
> +      // It is okay to return to a different block of a straight function, we
> +      // only have to
> +      // be more careful if returning from one inlined block to another.
> +      if (m_addr_context.block->GetInlinedFunctionInfo() == nullptr &&
> +          context.block->GetInlinedFunctionInfo() == nullptr)
> +        return true;
> +
> +      if (m_addr_context.block && m_addr_context.block == context.block)
> +        return true;
>     }
> -  } else if (m_addr_context.symbol && m_addr_context.symbol == context.symbol) {
> +  }
> +  if (m_addr_context.symbol && m_addr_context.symbol == context.symbol) {
>     return true;
>   }
>   return false;
> 
> 
> <D26804.78374.patch>