[Lldb-commits] FW: [lldb] r190812 - Fixes symbol resolution for a function with a tail call because the PC

Thirumurthi, Ashok ashok.thirumurthi at intel.com
Mon Sep 16 15:10:42 PDT 2013


FYI Jason,

I believe that I handled the cases correctly that were lowered from the RegisterContextLLDB, however the OS/X test suite has reached the point where it's difficult to identify regressions.

Incidentally, I preserved the behavior of RegisterContextLLDB in the case where m_current_pc.GetOffset() is the start of a function (using the following code), however I'm not sure what test case triggers this need to decrement the PC in this case...
  so_addr.GetOffset() == addr_range.GetBaseAddress().GetOffset()

Also, I debated adding eSymbolContextTailCall, but really we don't resolve sc.function without function DIEs, so most users can use eSymbolContextSymbol if they are concerned about the resolution depth.

Cheers,

- Ashok

-----Original Message-----
From: lldb-commits-bounces at cs.uiuc.edu [mailto:lldb-commits-bounces at cs.uiuc.edu] On Behalf Of Ashok Thirumurthi
Sent: Monday, September 16, 2013 6:00 PM
To: lldb-commits at cs.uiuc.edu
Subject: [Lldb-commits] [lldb] r190812 - Fixes symbol resolution for a function with a tail call because the PC

Author: athirumu
Date: Mon Sep 16 17:00:17 2013
New Revision: 190812

URL: http://llvm.org/viewvc/llvm-project?rev=190812&view=rev
Log:
Fixes symbol resolution for a function with a tail call because the PC for the frame is one past the address range of the calling function.
- Lowers the fix from RegisterContextLLDB for use with disassembly
- Fixes one of three issues in the disassembly test in TestInferiorAssert.py

Also adds documentation that explains the resolution depths and interface.

Note: This change affects the resolution scope for eSymbolContextFunction without impacting the performance of eSymbolContextSymbol.

Thanks to Matt Kopec for his review.

Modified:
    lldb/trunk/include/lldb/Core/Module.h
    lldb/trunk/source/Core/Module.cpp
    lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp

Modified: lldb/trunk/include/lldb/Core/Module.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Module.h?rev=190812&r1=190811&r2=190812&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Core/Module.h (original)
+++ lldb/trunk/include/lldb/Core/Module.h Mon Sep 16 17:00:17 2013
@@ -749,6 +749,38 @@ public:
     bool
     ResolveFileAddress (lldb::addr_t vm_addr, Address& so_addr);
 
+    //------------------------------------------------------------------
+    /// Resolve the symbol context for the given address.
+    ///
+    /// Tries to resolve the matching symbol context based on a lookup
+    /// from the current symbol vendor.  If the lazy lookup fails,
+    /// an attempt is made to parse the eh_frame section to handle 
+    /// stripped symbols.  If this fails, an attempt is made to resolve
+    /// the symbol to the previous address to handle the case of a 
+    /// function with a tail call.
+    ///
+    /// Use properties of the modified SymbolContext to inspect any
+    /// resolved target, module, compilation unit, symbol, function,
+    /// function block or line entry.  Use the return value to determine
+    /// which of these properties have been modified.
+    ///
+    /// @param[in] so_addr
+    ///     A load address to resolve.
+    ///
+    /// @param[in] resolve_scope
+    ///     The scope that should be resolved (see SymbolContext::Scope).
+    ///     A combination of flags from the enumeration SymbolContextItem
+    ///     requesting a resolution depth.  Note that the flags that are
+    ///     actually resolved may be a superset of the requested flags.
+    ///     For instance, eSymbolContextSymbol requires resolution of
+    ///     eSymbolContextModule, and eSymbolContextFunction requires
+    ///     eSymbolContextSymbol.
+    ///
+    /// @return
+    ///     The scope that has been resolved (see SymbolContext::Scope).
+    ///
+    /// @see SymbolContext::Scope
+    
+ //------------------------------------------------------------------
     uint32_t
     ResolveSymbolContextForAddress (const Address& so_addr, uint32_t resolve_scope, SymbolContext& sc);
 

Modified: lldb/trunk/source/Core/Module.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Module.cpp?rev=190812&r1=190811&r2=190812&view=diff
==============================================================================
--- lldb/trunk/source/Core/Module.cpp (original)
+++ lldb/trunk/source/Core/Module.cpp Mon Sep 16 17:00:17 2013
@@ -468,6 +468,10 @@ Module::ResolveSymbolContextForAddress (
         sc.module_sp = shared_from_this();
         resolved_flags |= eSymbolContextModule;
 
+        SymbolVendor* sym_vendor = GetSymbolVendor();
+        if (!sym_vendor)
+            return resolved_flags;
+
         // Resolve the compile unit, function, block, line table or line
         // entry if requested.
         if (resolve_scope & eSymbolContextCompUnit    ||
@@ -475,25 +479,51 @@ Module::ResolveSymbolContextForAddress (
             resolve_scope & eSymbolContextBlock       ||
             resolve_scope & eSymbolContextLineEntry   )
         {
-            SymbolVendor *symbols = GetSymbolVendor ();
-            if (symbols)
-                resolved_flags |= symbols->ResolveSymbolContext (so_addr, resolve_scope, sc);
+            resolved_flags |= sym_vendor->ResolveSymbolContext 
+ (so_addr, resolve_scope, sc);
         }
 
         // Resolve the symbol if requested, but don't re-look it up if we've already found it.
         if (resolve_scope & eSymbolContextSymbol && !(resolved_flags & eSymbolContextSymbol))
         {
-            SymbolVendor* sym_vendor = GetSymbolVendor();
-            if (sym_vendor)
+            Symtab *symtab = sym_vendor->GetSymtab();
+            if (symtab && so_addr.IsSectionOffset())
             {
-                Symtab *symtab = sym_vendor->GetSymtab();
-                if (symtab)
+                sc.symbol = symtab->FindSymbolContainingFileAddress(so_addr.GetFileAddress());
+                if (sc.symbol)
+                    resolved_flags |= eSymbolContextSymbol;
+            }
+        }
+
+        // For function symbols, so_addr may be off by one.  This is a convention consistent
+        // with FDE row indices in eh_frame sections, but requires extra logic here to permit
+        // symbol lookup for disassembly and unwind.
+        if (resolve_scope & eSymbolContextSymbol && !(resolved_flags & eSymbolContextSymbol) &&
+            resolve_scope & eSymbolContextFunction && !(resolved_flags & eSymbolContextFunction) &&
+            so_addr.GetOffset() > 0)
+        {
+            Address previous_addr = so_addr;
+            previous_addr.SetOffset(so_addr.GetOffset() - 1);
+
+            const uint32_t flags = sym_vendor->ResolveSymbolContext (previous_addr, resolve_scope, sc);
+            if (flags & eSymbolContextSymbol)
+            {
+                AddressRange addr_range;
+                if (sc.GetAddressRange (eSymbolContextFunction | 
+ eSymbolContextSymbol, 0, false, addr_range))
                 {
-                    if (so_addr.IsSectionOffset())
+                    if (addr_range.GetBaseAddress().GetSection() == so_addr.GetSection())
+                    {
+                        // If the requested address is one past the address range of a function (i.e. a tail call),
+                        // or the decremented address is the start of a function (i.e. some forms of trampoline),
+                        // indicate that the symbol has been resolved.
+                        if (so_addr.GetOffset() == addr_range.GetBaseAddress().GetOffset() ||
+                            so_addr.GetOffset() == addr_range.GetBaseAddress().GetOffset() + addr_range.GetByteSize())
+                        {
+                            resolved_flags |= flags;
+                        }
+                    }
+                    else
                     {
-                        sc.symbol = symtab->FindSymbolContainingFileAddress(so_addr.GetFileAddress());
-                        if (sc.symbol)
-                            resolved_flags |= eSymbolContextSymbol;
+                        sc.symbol = nullptr; // Don't trust the symbol if the sections didn't match.
                     }
                 }
             }

Modified: lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp?rev=190812&r1=190811&r2=190812&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp 
+++ Mon Sep 16 17:00:17 2013
@@ -387,35 +387,18 @@ RegisterContextLLDB::InitializeNonZeroth
     if (m_sym_ctx_valid == false)
        decr_pc_and_recompute_addr_range = true;
 
-    // Or if we're in the middle of the stack (and not "above" an asynchronous event like sigtramp),
-    // and our "current" pc is the start of a function...
+    // Or if we're in the middle of the stack (and not "above" an asynchronous event like sigtramp), and
+    // our "current" pc is the start of a function or our "current" pc is one past the end of a function...
     if (m_sym_ctx_valid
         && GetNextFrame()->m_frame_type != eSigtrampFrame
         && GetNextFrame()->m_frame_type != eDebuggerFrame
         && addr_range.GetBaseAddress().IsValid()
-        && addr_range.GetBaseAddress().GetSection() == m_current_pc.GetSection()
-        && addr_range.GetBaseAddress().GetOffset() == m_current_pc.GetOffset())
+        && addr_range.GetBaseAddress().GetSection() == 
+ m_current_pc.GetSection())
     {
-        decr_pc_and_recompute_addr_range = true;
-    }
-
-    // We need to back up the pc by 1 byte and re-search for the Symbol to handle the case where the "saved pc"
-    // value is pointing to the next function, e.g. if a function ends with a CALL instruction.
-    // FIXME this may need to be an architectural-dependent behavior; if so we'll need to add a member function
-    // to the ABI plugin and consult that.
-    if (decr_pc_and_recompute_addr_range)
-    {
-        Address temporary_pc(m_current_pc);
-        temporary_pc.SetOffset(m_current_pc.GetOffset() - 1);
-        m_sym_ctx.Clear(false);
-        m_sym_ctx_valid = false;
-        if ((pc_module_sp->ResolveSymbolContextForAddress (temporary_pc, eSymbolContextFunction| eSymbolContextSymbol, m_sym_ctx) & eSymbolContextSymbol) == eSymbolContextSymbol)
-        {
-            m_sym_ctx_valid = true;
-        }
-        if (!m_sym_ctx.GetAddressRange (eSymbolContextFunction | eSymbolContextSymbol, 0, false,  addr_range))
+        if (addr_range.GetBaseAddress().GetOffset() == m_current_pc.GetOffset() ||
+            addr_range.GetBaseAddress().GetOffset() + 
+ addr_range.GetByteSize() == m_current_pc.GetOffset())
         {
-            m_sym_ctx_valid = false;
+            decr_pc_and_recompute_addr_range = true;
         }
     }
 


_______________________________________________
lldb-commits mailing list
lldb-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits




More information about the lldb-commits mailing list