[Lldb-commits] [lldb] f012055 - [DWARF] Emit DW_AT_call_return_pc as an address

Vedant Kumar via lldb-commits lldb-commits at lists.llvm.org
Wed Jan 15 13:13:41 PST 2020


Author: Vedant Kumar
Date: 2020-01-15T13:02:23-08:00
New Revision: f0120556c7e2ef14ff3da5bd7d5717cedf94b767

URL: https://github.com/llvm/llvm-project/commit/f0120556c7e2ef14ff3da5bd7d5717cedf94b767
DIFF: https://github.com/llvm/llvm-project/commit/f0120556c7e2ef14ff3da5bd7d5717cedf94b767.diff

LOG: [DWARF] Emit DW_AT_call_return_pc as an address

This reverts D53469, which changed llvm's DWARF emission to emit
DW_AT_call_return_pc as a function-local offset. Such an encoding is not
compatible with post-link block re-ordering tools and isn't standards-
compliant.

In addition to reverting back to the original DW_AT_call_return_pc
encoding, teach lldb how to fix up DW_AT_call_return_pc when the address
comes from an object file pointed-to by a debug map. While doing this I
noticed that lldb's support for tail calls that cross a DSO/object file
boundary wasn't covered, so I added tests for that. This latter case
exercises the newly added return PC fixup.

The dsymutil changes in this patch were originally included in D49887:
the associated test should be sufficient to test DW_AT_call_return_pc
encoding purely on the llvm side.

Differential Revision: https://reviews.llvm.org/D72489

Added: 
    lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Makefile
    lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/One.mk
    lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/One/One.c
    lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/TestCrossDSOTailCalls.py
    lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Two.mk
    lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Two/Two.c
    lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/main.c
    lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/shared.h
    lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/Makefile
    lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/One.c
    lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/TestCrossObjectTailCalls.py
    lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/Two.c
    lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/main.c
    lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/shared.h
    llvm/test/tools/dsymutil/Inputs/call-site-entry.c
    llvm/test/tools/dsymutil/Inputs/call-site-entry.macho.x86_64
    llvm/test/tools/dsymutil/Inputs/call-site-entry.macho.x86_64.o
    llvm/test/tools/dsymutil/call-site-entry-linking.test

Modified: 
    lldb/include/lldb/Symbol/Function.h
    lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
    lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
    lldb/source/Symbol/Function.cpp
    llvm/include/llvm/CodeGen/DebugHandlerBase.h
    llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
    llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
    llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
    llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
    llvm/test/DebugInfo/X86/debug_addr.ll
    llvm/tools/dsymutil/DwarfLinkerForBinary.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Symbol/Function.h b/lldb/include/lldb/Symbol/Function.h
index f675b5fdffa6..c8f888c3bdd1 100644
--- a/lldb/include/lldb/Symbol/Function.h
+++ b/lldb/include/lldb/Symbol/Function.h
@@ -281,8 +281,7 @@ class CallEdge {
   /// made the call.
   lldb::addr_t GetReturnPCAddress(Function &caller, Target &target) const;
 
-  /// Like \ref GetReturnPCAddress, but returns an unslid function-local PC
-  /// offset.
+  /// Like \ref GetReturnPCAddress, but returns an unresolved file address.
   lldb::addr_t GetUnresolvedReturnPCAddress() const { return return_pc; }
 
   /// Get the call site parameters available at this call edge.
@@ -294,9 +293,8 @@ class CallEdge {
   CallEdge(lldb::addr_t return_pc, CallSiteParameterArray &&parameters)
       : return_pc(return_pc), parameters(std::move(parameters)) {}
 
-  /// An invalid address if this is a tail call. Otherwise, the function-local
-  /// PC offset. Adding this PC offset to the function's base load address
-  /// gives the return PC for the call.
+  /// An invalid address if this is a tail call. Otherwise, the return PC for
+  /// the call. Note that this is a file address which must be resolved.
   lldb::addr_t return_pc;
 
   CallSiteParameterArray parameters;

diff  --git a/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Makefile b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Makefile
new file mode 100644
index 000000000000..fd44eb4be9c3
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Makefile
@@ -0,0 +1,17 @@
+LD_EXTRAS := -L. -l$(LIB_PREFIX)One -l$(LIB_PREFIX)Two
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -g -O2 -glldb
+
+include Makefile.rules
+
+.PHONY:
+a.out: lib_One lib_Two
+
+lib_One: lib_Two
+
+lib_%:
+	$(MAKE) VPATH=$(SRCDIR)/$* -I $(SRCDIR) -f $(SRCDIR)/$*.mk DSYMUTIL=$(DSYMUTIL)
+
+clean::
+	$(MAKE) -f $(SRCDIR)/One.mk clean
+	$(MAKE) -f $(SRCDIR)/Two.mk clean

diff  --git a/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/One.mk b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/One.mk
new file mode 100644
index 000000000000..0d5f26fc31d7
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/One.mk
@@ -0,0 +1,7 @@
+DYLIB_NAME := One
+DYLIB_C_SOURCES := One.c
+DYLIB_ONLY := YES
+CFLAGS_EXTRAS := -g -O2 -glldb
+LD_EXTRAS := -L. -lTwo
+
+include Makefile.rules

diff  --git a/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/One/One.c b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/One/One.c
new file mode 100644
index 000000000000..50816530afa5
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/One/One.c
@@ -0,0 +1,11 @@
+#include "shared.h"
+
+__attribute__((noinline))
+static void helper_in_a() {
+  tail_called_in_b_from_a();
+}
+
+__attribute__((disable_tail_calls))
+void tail_called_in_a_from_main() {
+  helper_in_a();
+}

diff  --git a/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/TestCrossDSOTailCalls.py b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/TestCrossDSOTailCalls.py
new file mode 100644
index 000000000000..d0a4b69c27d4
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/TestCrossDSOTailCalls.py
@@ -0,0 +1,64 @@
+"""Test that backtraces can follow cross-DSO tail calls"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCrossDSOTailCalls(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def setUp(self):
+        TestBase.setUp(self)
+
+    @skipIf(compiler="clang", compiler_version=['<', '8.0'])
+    @skipIf(dwarf_version=['<', '4'])
+    @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
+    def test_cross_dso_tail_calls(self):
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+        target = self.dbg.CreateTarget(exe)
+        self.assertTrue(target, VALID_TARGET)
+
+        # Register our shared libraries for remote targets so they get
+        # automatically uploaded
+        environment = self.registerSharedLibrariesWithTarget(
+            target, ['One', 'Two'])
+
+        lldbutil.run_break_set_by_source_regexp(self, '// break here',
+                extra_options='-f Two.c')
+
+        process = target.LaunchSimple(
+            None, environment, self.get_process_working_directory())
+        self.assertTrue(process, PROCESS_IS_VALID)
+
+        # We should be stopped in the second dylib.
+        thread = lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint)
+
+        # Debug helper:
+        # self.runCmd("log enable -f /tmp/lldb.log lldb step")
+        # self.runCmd("bt")
+
+        # Check that the backtrace is what we expect:
+        #  frame #0: 0x000000010d5e5f94 libTwo.dylib`tail_called_in_b_from_b at Two.c:7:3 [opt]
+        #  frame #1: 0x000000010d5e5fa0 libTwo.dylib`tail_called_in_b_from_a [opt] [artificial]
+        #  frame #2: 0x000000010d5dcf80 libOne.dylib`helper_in_a [opt] [artificial]
+        #  frame #3: 0x000000010d5dcf79 libOne.dylib`tail_called_in_a_from_main at One.c:10:3 [opt]
+        #  frame #4: 0x000000010d5d3f80 a.out`helper [opt] [artificial]
+        #  frame #5: 0x000000010d5d3f79 a.out`main at main.c:10:3 [opt]
+        expected_frames = [
+                ("tail_called_in_b_from_b", False),
+                ("tail_called_in_b_from_a", True),
+                ("helper_in_a", True),
+                ("tail_called_in_a_from_main", False),
+                ("helper", True),
+                ("main", False)
+        ]
+        for idx, (name, is_artificial) in enumerate(expected_frames):
+            frame = thread.GetFrameAtIndex(idx)
+            self.assertTrue(name in frame.GetDisplayFunctionName())
+            self.assertEqual(frame.IsArtificial(), is_artificial)

diff  --git a/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Two.mk b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Two.mk
new file mode 100644
index 000000000000..c6020e0c298e
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Two.mk
@@ -0,0 +1,6 @@
+DYLIB_NAME := Two
+DYLIB_C_SOURCES := Two.c
+DYLIB_ONLY := YES
+CFLAGS_EXTRAS := -g -O2 -glldb
+
+include Makefile.rules

diff  --git a/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Two/Two.c b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Two/Two.c
new file mode 100644
index 000000000000..ecdffd0c72b7
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Two/Two.c
@@ -0,0 +1,12 @@
+#include "shared.h"
+
+volatile int x;
+
+__attribute__((noinline))
+void tail_called_in_b_from_b() {
+  ++x; // break here
+}
+
+void tail_called_in_b_from_a() {
+  tail_called_in_b_from_b();
+}

diff  --git a/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/main.c b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/main.c
new file mode 100644
index 000000000000..06e3521c1522
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/main.c
@@ -0,0 +1,12 @@
+#include "shared.h"
+
+__attribute__((noinline))
+static void helper() {
+  tail_called_in_a_from_main();
+}
+
+__attribute__((disable_tail_calls))
+int main() {
+  helper();
+  return 0;
+}

diff  --git a/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/shared.h b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/shared.h
new file mode 100644
index 000000000000..b0dda0a892a4
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/shared.h
@@ -0,0 +1,3 @@
+void tail_called_in_a_from_main();
+
+void tail_called_in_b_from_a();

diff  --git a/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/Makefile b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/Makefile
new file mode 100644
index 000000000000..aa3958bb4e98
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c One.c Two.c
+
+CFLAGS_EXTRAS := -g -O2 -glldb
+include Makefile.rules

diff  --git a/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/One.c b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/One.c
new file mode 100644
index 000000000000..50816530afa5
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/One.c
@@ -0,0 +1,11 @@
+#include "shared.h"
+
+__attribute__((noinline))
+static void helper_in_a() {
+  tail_called_in_b_from_a();
+}
+
+__attribute__((disable_tail_calls))
+void tail_called_in_a_from_main() {
+  helper_in_a();
+}

diff  --git a/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/TestCrossObjectTailCalls.py b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/TestCrossObjectTailCalls.py
new file mode 100644
index 000000000000..80f20a874f06
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/TestCrossObjectTailCalls.py
@@ -0,0 +1,59 @@
+"""Test that backtraces can follow cross-object tail calls"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCrossObjectTailCalls(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def setUp(self):
+        TestBase.setUp(self)
+
+    @skipIf(compiler="clang", compiler_version=['<', '8.0'])
+    @skipIf(dwarf_version=['<', '4'])
+    @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
+    def test_cross_object_tail_calls(self):
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+        target = self.dbg.CreateTarget(exe)
+        self.assertTrue(target, VALID_TARGET)
+
+        lldbutil.run_break_set_by_source_regexp(self, '// break here',
+                extra_options='-f Two.c')
+
+        process = target.LaunchSimple(
+            None, None, self.get_process_working_directory())
+        self.assertTrue(process, PROCESS_IS_VALID)
+
+        # We should be stopped in the second dylib.
+        thread = lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint)
+
+        # Debug helper:
+        # self.runCmd("log enable -f /tmp/lldb.log lldb step")
+        # self.runCmd("bt")
+
+        # Check that the backtrace is what we expect:
+        #  frame #0: 0x000000010be73f94 a.out`tail_called_in_b_from_b at Two.c:7:3 [opt]
+        #  frame #1: 0x000000010be73fa0 a.out`tail_called_in_b_from_a at Two.c:8:1 [opt] [artificial]
+        #  frame #2: 0x000000010be73f80 a.out`helper_in_a at One.c:11:1 [opt] [artificial]
+        #  frame #3: 0x000000010be73f79 a.out`tail_called_in_a_from_main at One.c:10:3 [opt]
+        #  frame #4: 0x000000010be73f60 a.out`helper at main.c:11:3 [opt] [artificial]
+        #  frame #5: 0x000000010be73f59 a.out`main at main.c:10:3 [opt]
+        expected_frames = [
+                ("tail_called_in_b_from_b", False),
+                ("tail_called_in_b_from_a", True),
+                ("helper_in_a", True),
+                ("tail_called_in_a_from_main", False),
+                ("helper", True),
+                ("main", False)
+        ]
+        for idx, (name, is_artificial) in enumerate(expected_frames):
+            frame = thread.GetFrameAtIndex(idx)
+            self.assertTrue(name in frame.GetDisplayFunctionName())
+            self.assertEqual(frame.IsArtificial(), is_artificial)

diff  --git a/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/Two.c b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/Two.c
new file mode 100644
index 000000000000..ecdffd0c72b7
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/Two.c
@@ -0,0 +1,12 @@
+#include "shared.h"
+
+volatile int x;
+
+__attribute__((noinline))
+void tail_called_in_b_from_b() {
+  ++x; // break here
+}
+
+void tail_called_in_b_from_a() {
+  tail_called_in_b_from_b();
+}

diff  --git a/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/main.c b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/main.c
new file mode 100644
index 000000000000..06e3521c1522
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/main.c
@@ -0,0 +1,12 @@
+#include "shared.h"
+
+__attribute__((noinline))
+static void helper() {
+  tail_called_in_a_from_main();
+}
+
+__attribute__((disable_tail_calls))
+int main() {
+  helper();
+  return 0;
+}

diff  --git a/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/shared.h b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/shared.h
new file mode 100644
index 000000000000..b0dda0a892a4
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/shared.h
@@ -0,0 +1,3 @@
+void tail_called_in_a_from_main();
+
+void tail_called_in_b_from_a();

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index d45a8b56efe4..7d0d5b16b7e5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -792,6 +792,13 @@ Function *SymbolFileDWARF::ParseFunction(CompileUnit &comp_unit,
   return dwarf_ast->ParseFunctionFromDWARF(comp_unit, die);
 }
 
+lldb::addr_t SymbolFileDWARF::FixupAddress(lldb::addr_t file_addr) {
+  SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
+  if (debug_map_symfile)
+    return debug_map_symfile->LinkOSOFileAddress(this, file_addr);
+  return file_addr;
+}
+
 bool SymbolFileDWARF::FixupAddress(Address &addr) {
   SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
   if (debug_map_symfile) {
@@ -3805,8 +3812,8 @@ CollectCallSiteParameters(ModuleSP module, DWARFDIE call_site_die) {
 }
 
 /// Collect call graph edges present in a function DIE.
-static std::vector<std::unique_ptr<lldb_private::CallEdge>>
-CollectCallEdges(ModuleSP module, DWARFDIE function_die) {
+std::vector<std::unique_ptr<lldb_private::CallEdge>>
+SymbolFileDWARF::CollectCallEdges(ModuleSP module, DWARFDIE function_die) {
   // Check if the function has a supported call site-related attribute.
   // TODO: In the future it may be worthwhile to support call_all_source_calls.
   uint64_t has_call_edges =
@@ -3882,6 +3889,11 @@ CollectCallEdges(ModuleSP module, DWARFDIE function_die) {
       continue;
     }
 
+    // Adjust the return PC. It needs to be fixed up if the main executable
+    // contains a debug map (i.e. pointers to object files), because we need a
+    // file address relative to the executable's text section.
+    return_pc = FixupAddress(return_pc);
+
     // Extract call site parameters.
     CallSiteParameterArray parameters =
         CollectCallSiteParameters(module, child);

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
index 23e26732453f..1336c8cb0220 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -417,6 +417,18 @@ class SymbolFileDWARF : public lldb_private::SymbolFile,
   bool ClassContainsSelector(const DWARFDIE &class_die,
                              lldb_private::ConstString selector);
 
+  /// Parse call site entries (DW_TAG_call_site), including any nested call site
+  /// parameters (DW_TAG_call_site_parameter).
+  std::vector<std::unique_ptr<lldb_private::CallEdge>>
+  CollectCallEdges(lldb::ModuleSP module, DWARFDIE function_die);
+
+  /// If this symbol file is linked to by a debug map (see
+  /// SymbolFileDWARFDebugMap), and \p file_addr is a file address relative to
+  /// an object file, adjust \p file_addr so that it is relative to the main
+  /// binary. Returns the adjusted address, or \p file_addr if no adjustment is
+  /// needed, on success and LLDB_INVALID_ADDRESS otherwise.
+  lldb::addr_t FixupAddress(lldb::addr_t file_addr);
+
   bool FixupAddress(lldb_private::Address &addr);
 
   typedef std::set<lldb_private::Type *> TypeSet;

diff  --git a/lldb/source/Symbol/Function.cpp b/lldb/source/Symbol/Function.cpp
index e92585ccfed7..a2309a194b82 100644
--- a/lldb/source/Symbol/Function.cpp
+++ b/lldb/source/Symbol/Function.cpp
@@ -123,8 +123,25 @@ size_t InlineFunctionInfo::MemorySize() const {
 
 lldb::addr_t CallEdge::GetReturnPCAddress(Function &caller,
                                           Target &target) const {
-  const Address &base = caller.GetAddressRange().GetBaseAddress();
-  return base.GetLoadAddress(&target) + return_pc;
+  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
+
+  const Address &caller_start_addr = caller.GetAddressRange().GetBaseAddress();
+
+  ModuleSP caller_module_sp = caller_start_addr.GetModule();
+  if (!caller_module_sp) {
+    LLDB_LOG(log, "GetReturnPCAddress: cannot get Module for caller");
+    return LLDB_INVALID_ADDRESS;
+  }
+
+  SectionList *section_list = caller_module_sp->GetSectionList();
+  if (!section_list) {
+    LLDB_LOG(log, "GetReturnPCAddress: cannot get SectionList for Module");
+    return LLDB_INVALID_ADDRESS;
+  }
+
+  Address return_pc_addr = Address(return_pc, section_list);
+  lldb::addr_t ret_addr = return_pc_addr.GetLoadAddress(&target);
+  return ret_addr;
 }
 
 void DirectCallEdge::ParseSymbolFileAndResolve(ModuleList &images) {

diff  --git a/llvm/include/llvm/CodeGen/DebugHandlerBase.h b/llvm/include/llvm/CodeGen/DebugHandlerBase.h
index 4008d597395e..d5195118cab4 100644
--- a/llvm/include/llvm/CodeGen/DebugHandlerBase.h
+++ b/llvm/include/llvm/CodeGen/DebugHandlerBase.h
@@ -124,10 +124,6 @@ class DebugHandlerBase : public AsmPrinterHandler {
   /// Return Label immediately following the instruction.
   MCSymbol *getLabelAfterInsn(const MachineInstr *MI);
 
-  /// Return the function-local offset of an instruction. A label for the
-  /// instruction \p MI should exist (\ref getLabelAfterInsn).
-  const MCExpr *getFunctionLocalOffsetAfterInsn(const MachineInstr *MI);
-
   /// If this type is derived from a base type then return base type size.
   static uint64_t getBaseTypeSize(const DIType *Ty);
 };

diff  --git a/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp b/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
index 22f458e4b03e..f95c9a3271bb 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp
@@ -124,21 +124,6 @@ MCSymbol *DebugHandlerBase::getLabelAfterInsn(const MachineInstr *MI) {
   return LabelsAfterInsn.lookup(MI);
 }
 
-// Return the function-local offset of an instruction.
-const MCExpr *
-DebugHandlerBase::getFunctionLocalOffsetAfterInsn(const MachineInstr *MI) {
-  MCContext &MC = Asm->OutContext;
-
-  MCSymbol *Start = Asm->getFunctionBegin();
-  const auto *StartRef = MCSymbolRefExpr::create(Start, MC);
-
-  MCSymbol *AfterInsn = getLabelAfterInsn(MI);
-  assert(AfterInsn && "Expected label after instruction");
-  const auto *AfterRef = MCSymbolRefExpr::create(AfterInsn, MC);
-
-  return MCBinaryExpr::createSub(AfterRef, StartRef, MC);
-}
-
 /// If this type is derived from a base type then return base type size.
 uint64_t DebugHandlerBase::getBaseTypeSize(const DIType *Ty) {
   assert(Ty);

diff  --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
index 38011102c7b3..8f42c91d3dd5 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -956,9 +956,11 @@ DwarfCompileUnit::getDwarf5OrGNULocationAtom(dwarf::LocationAtom Loc) const {
   }
 }
 
-DIE &DwarfCompileUnit::constructCallSiteEntryDIE(
-    DIE &ScopeDIE, const DISubprogram *CalleeSP, bool IsTail,
-    const MCSymbol *PCAddr, const MCExpr *PCOffset, unsigned CallReg) {
+DIE &DwarfCompileUnit::constructCallSiteEntryDIE(DIE &ScopeDIE,
+                                                 const DISubprogram *CalleeSP,
+                                                 bool IsTail,
+                                                 const MCSymbol *PCAddr,
+                                                 unsigned CallReg) {
   // Insert a call site entry DIE within ScopeDIE.
   DIE &CallSiteDIE = createAndAddDIE(getDwarf5OrGNUTag(dwarf::DW_TAG_call_site),
                                      ScopeDIE, nullptr);
@@ -984,8 +986,8 @@ DIE &DwarfCompileUnit::constructCallSiteEntryDIE(
     assert(PCAddr && "Missing PC information for a call");
     addLabelAddress(CallSiteDIE, dwarf::DW_AT_low_pc, PCAddr);
   } else if (!IsTail || DD->tuneForGDB()) {
-    assert(PCOffset && "Missing return PC information for a call");
-    addAddressExpr(CallSiteDIE, dwarf::DW_AT_call_return_pc, PCOffset);
+    assert(PCAddr && "Missing return PC information for a call");
+    addLabelAddress(CallSiteDIE, dwarf::DW_AT_call_return_pc, PCAddr);
   }
 
   return CallSiteDIE;
@@ -1285,12 +1287,6 @@ void DwarfCompileUnit::addExpr(DIELoc &Die, dwarf::Form Form,
   Die.addValue(DIEValueAllocator, (dwarf::Attribute)0, Form, DIEExpr(Expr));
 }
 
-void DwarfCompileUnit::addAddressExpr(DIE &Die, dwarf::Attribute Attribute,
-                                      const MCExpr *Expr) {
-  Die.addValue(DIEValueAllocator, Attribute, dwarf::DW_FORM_addr,
-               DIEExpr(Expr));
-}
-
 void DwarfCompileUnit::applySubprogramAttributesToDefinition(
     const DISubprogram *SP, DIE &SPDie) {
   auto *SPDecl = SP->getDeclaration();

diff  --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
index 8491d078ed89..d659746ca4b7 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
@@ -242,17 +242,12 @@ class DwarfCompileUnit final : public DwarfUnit {
   /// Construct a call site entry DIE describing a call within \p Scope to a
   /// callee described by \p CalleeSP.
   /// \p IsTail specifies whether the call is a tail call.
-  /// \p PCAddr (used for GDB + DWARF 4 tuning) points to the PC value after
-  /// the call instruction.
-  /// \p PCOffset (used for cases other than GDB + DWARF 4 tuning) must be
-  /// non-zero for non-tail calls (in the case of non-gdb tuning, since for
-  /// GDB + DWARF 5 tuning we still generate PC info for tail calls) or be the
-  /// function-local offset to PC value after the call instruction.
+  /// \p PCAddr points to the PC value after the call instruction.
   /// \p CallReg is a register location for an indirect call. For direct calls
   /// the \p CallReg is set to 0.
   DIE &constructCallSiteEntryDIE(DIE &ScopeDIE, const DISubprogram *CalleeSP,
                                  bool IsTail, const MCSymbol *PCAddr,
-                                 const MCExpr *PCOffset, unsigned CallReg);
+                                 unsigned CallReg);
   /// Construct call site parameter DIEs for the \p CallSiteDIE. The \p Params
   /// were collected by the \ref collectCallSiteParameters.
   /// Note: The order of parameters does not matter, since debuggers recognize
@@ -340,9 +335,6 @@ class DwarfCompileUnit final : public DwarfUnit {
   /// Add a Dwarf expression attribute data and value.
   void addExpr(DIELoc &Die, dwarf::Form Form, const MCExpr *Expr);
 
-  /// Add an attribute containing an address expression to \p Die.
-  void addAddressExpr(DIE &Die, dwarf::Attribute Attribute, const MCExpr *Expr);
-
   void applySubprogramAttributesToDefinition(const DISubprogram *SP,
                                              DIE &SPDie);
 

diff  --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index fa6800de7955..4c96f0aff819 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -719,7 +719,6 @@ void DwarfDebug::constructCallSiteEntryDIEs(const DISubprogram &SP,
 
   const TargetInstrInfo *TII = MF.getSubtarget().getInstrInfo();
   assert(TII && "TargetInstrInfo not found: cannot label tail calls");
-  bool ApplyGNUExtensions = getDwarfVersion() == 4 && tuneForGDB();
 
   // Emit call site entries for each call or tail call in the function.
   for (const MachineBasicBlock &MBB : MF) {
@@ -781,25 +780,16 @@ void DwarfDebug::constructCallSiteEntryDIEs(const DISubprogram &SP,
       const MachineInstr *TopLevelCallMI =
           MI.isInsideBundle() ? &*getBundleStart(MI.getIterator()) : &MI;
 
-      // For tail calls, for non-gdb tuning, no return PC information is needed.
+      // For tail calls, no return PC information is needed.
       // For regular calls (and tail calls in GDB tuning), the return PC
       // is needed to disambiguate paths in the call graph which could lead to
       // some target function.
-      const MCExpr *PCOffset =
+      const MCSymbol *PCAddr =
           (IsTail && !tuneForGDB())
               ? nullptr
-              : getFunctionLocalOffsetAfterInsn(TopLevelCallMI);
-
-      // Return address of a call-like instruction for a normal call or a
-      // jump-like instruction for a tail call. This is needed for
-      // GDB + DWARF 4 tuning.
-      const MCSymbol *PCAddr =
-          ApplyGNUExtensions
-              ? const_cast<MCSymbol *>(getLabelAfterInsn(TopLevelCallMI))
-              : nullptr;
+              : const_cast<MCSymbol *>(getLabelAfterInsn(TopLevelCallMI));
 
-      assert((IsTail || PCOffset || PCAddr) &&
-             "Call without return PC information");
+      assert((IsTail || PCAddr) && "Call without return PC information");
 
       LLVM_DEBUG(dbgs() << "CallSiteEntry: " << MF.getName() << " -> "
                         << (CalleeDecl ? CalleeDecl->getName()
@@ -808,9 +798,8 @@ void DwarfDebug::constructCallSiteEntryDIEs(const DISubprogram &SP,
                                                        ->getName(CallReg)))
                         << (IsTail ? " [IsTail]" : "") << "\n");
 
-      DIE &CallSiteDIE =
-            CU.constructCallSiteEntryDIE(ScopeDIE, CalleeSP, IsTail, PCAddr,
-                                         PCOffset, CallReg);
+      DIE &CallSiteDIE = CU.constructCallSiteEntryDIE(ScopeDIE, CalleeSP,
+                                                      IsTail, PCAddr, CallReg);
 
       // GDB and LLDB support call site parameter debug info.
       if (Asm->TM.Options.EnableDebugEntryValues &&

diff  --git a/llvm/test/DebugInfo/X86/debug_addr.ll b/llvm/test/DebugInfo/X86/debug_addr.ll
index 424313ac2e64..1fa797efd5fb 100644
--- a/llvm/test/DebugInfo/X86/debug_addr.ll
+++ b/llvm/test/DebugInfo/X86/debug_addr.ll
@@ -9,6 +9,7 @@
 ; }
 ;
 ; void bar() {
+;   foo();
 ; }
 
 ; DWARF4: .debug_info contents:
@@ -18,11 +19,14 @@
 ; DWARF4-NOT: DW_TAG_{{.*}}
 ; DWARF4: DW_AT_GNU_dwo_name{{.*}}test.dwo
 ; DWARF4: DW_AT_GNU_addr_base{{.*}}0x00000000
+; DWARF4: DW_TAG_GNU_call_site
+; DWARF4:   DW_AT_low_pc [DW_FORM_GNU_addr_index] (indexed (00000002) address = 0x0000000000000018 ".text")
 ; DWARF4: .debug_addr contents:
 ; DWARF4-NEXT: 0x00000000: Addr Section: length = 0x00000000, version = 0x0004, addr_size = 0x04, seg_size = 0x00
 ; DWARF4-NEXT: Addrs: [
 ; DWARF4-NEXT: 0x00000000
 ; DWARF4-NEXT: 0x00000010
+; DWARF4-NEXT: 0x00000018
 ; DWARF4-NEXT: ]
 
 ; DWARF5: .debug_info contents:
@@ -33,11 +37,13 @@
 ; DWARF5: DW_AT_dwo_name{{.*}}test.dwo
 ; DWARF5: DW_AT_addr_base{{.*}}0x00000008
 ; DWARF5: DW_AT_low_pc [DW_FORM_addrx] (indexed (00000000) address = 0x0000000000000000 ".text")
+; DWARF5: DW_AT_call_return_pc [DW_FORM_addrx] (indexed (00000002) address = 0x0000000000000018 ".text")
 ; DWARF5: .debug_addr contents:
-; DWARF5-NEXT: 0x00000000: Addr Section: length = 0x0000000c, version = 0x0005, addr_size = 0x04, seg_size = 0x00
+; DWARF5-NEXT: 0x00000000: Addr Section: length = 0x00000010, version = 0x0005, addr_size = 0x04, seg_size = 0x00
 ; DWARF5-NEXT: Addrs: [
 ; DWARF5-NEXT: 0x00000000
 ; DWARF5-NEXT: 0x00000010
+; DWARF5-NEXT: 0x00000018
 ; DWARF5-NEXT: ]
 
 ; ModuleID = './test.c'
@@ -54,6 +60,7 @@ entry:
 ; Function Attrs: noinline nounwind optnone
 define void @bar() #0 !dbg !13 {
 entry:
+  call void @foo(), !dbg !14
   ret void, !dbg !14
 }
 
@@ -76,5 +83,5 @@ attributes #0 = { noinline nounwind optnone }
 !10 = !DISubroutineType(types: !11)
 !11 = !{null}
 !12 = !DILocation(line: 2, column: 3, scope: !8)
-!13 = distinct !DISubprogram(name: "bar", scope: !9, file: !9, line: 5, type: !10, isLocal: false, isDefinition: true, scopeLine: 5, isOptimized: false, unit: !0)
+!13 = distinct !DISubprogram(name: "bar", scope: !9, file: !9, line: 5, type: !10, isLocal: false, isDefinition: true, flags: DIFlagAllCallsDescribed, scopeLine: 5, isOptimized: false, unit: !0)
 !14 = !DILocation(line: 6, column: 3, scope: !13)

diff  --git a/llvm/test/tools/dsymutil/Inputs/call-site-entry.c b/llvm/test/tools/dsymutil/Inputs/call-site-entry.c
new file mode 100644
index 000000000000..db39e2b906d4
--- /dev/null
+++ b/llvm/test/tools/dsymutil/Inputs/call-site-entry.c
@@ -0,0 +1,25 @@
+/*
+ * This file is used to test dsymutil support for call site entries
+ * (DW_TAG_call_site / DW_AT_call_return_pc).
+ *
+ * Instructions for regenerating binaries (on Darwin/x86_64):
+ *
+ * 1. Copy the source to a top-level directory to work around having absolute
+ *    paths in the symtab's OSO entries.
+ *
+ *    mkdir -p /Inputs/ && cp call-site-entry.c /Inputs && cd /Inputs
+ *
+ * 2. Compile with call site info enabled.
+ *
+ *    clang -g -O1 -Xclang -disable-llvm-passes call-site-entry.c -c -o call-site-entry.macho.x86_64.o
+ *    clang call-site-entry.macho.x86_64.o -o call-site-entry.macho.x86_64
+ *
+ * 3. Copy the binaries back into the repo's Inputs directory. You'll need
+ *    -oso-prepend-path=%p to link.
+ */
+
+__attribute__((optnone)) int f() {}
+
+int main() {
+  f();
+}

diff  --git a/llvm/test/tools/dsymutil/Inputs/call-site-entry.macho.x86_64 b/llvm/test/tools/dsymutil/Inputs/call-site-entry.macho.x86_64
new file mode 100755
index 000000000000..7f6dd2aa5eab
Binary files /dev/null and b/llvm/test/tools/dsymutil/Inputs/call-site-entry.macho.x86_64 
diff er

diff  --git a/llvm/test/tools/dsymutil/Inputs/call-site-entry.macho.x86_64.o b/llvm/test/tools/dsymutil/Inputs/call-site-entry.macho.x86_64.o
new file mode 100644
index 000000000000..527adc579c76
Binary files /dev/null and b/llvm/test/tools/dsymutil/Inputs/call-site-entry.macho.x86_64.o 
diff er

diff  --git a/llvm/test/tools/dsymutil/call-site-entry-linking.test b/llvm/test/tools/dsymutil/call-site-entry-linking.test
new file mode 100644
index 000000000000..7eef7af41f52
--- /dev/null
+++ b/llvm/test/tools/dsymutil/call-site-entry-linking.test
@@ -0,0 +1,4 @@
+RUN: dsymutil -oso-prepend-path=%p %p/Inputs/call-site-entry.macho.x86_64 -o %t.dSYM
+RUN: llvm-dwarfdump %t.dSYM | FileCheck %s -implicit-check-not=DW_AT_call_return_pc
+
+CHECK: DW_AT_call_return_pc  (0x0000000100000fa4)

diff  --git a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
index 8a961e2c6ba0..e6e7730651c8 100644
--- a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
+++ b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
@@ -1363,6 +1363,10 @@ unsigned DwarfLinkerForBinary::DIECloner::cloneAddressAttribute(
       // it. Otherwise (when no relocations where applied) just use the
       // one we just decoded.
       Addr = (Info.OrigHighPc ? Info.OrigHighPc : Addr) + Info.PCOffset;
+  } else if (AttrSpec.Attr == dwarf::DW_AT_call_return_pc) {
+    // Relocate a return PC address within a call site entry.
+    if (Die.getTag() == dwarf::DW_TAG_call_site)
+      Addr += Info.PCOffset;
   }
 
   Die.addValue(DIEAlloc, static_cast<dwarf::Attribute>(AttrSpec.Attr),


        


More information about the lldb-commits mailing list