[DWARF] Emit DW_AT_call_return_pc as an address

[vsk]

Summary

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 originall included in D49887:
the associated test should be sufficient to test DW_AT_call_return_pc
encoding purely on the llvm side.

[dblaikie]

Thanks for looking into this!

Could you measure the size of the object files of, for example, the clang binary before/after this change - and, if possible, on Linux (where relocations are required to fixup these addresses)? I'm concerned this might increase the size significantly.
Could you check if this does the right thing for Split DWARF or DWARFv5? (I suspect it does - if you're using the same API as is used for DW_TAG_label's low_pc, for instance - which I imagine you are) - in terms of adding the address to debug_addr, and using addrx encoding.

I was going to suggest we use a non-standard extension to preserve the subprogram-relative behavior, by using a different DW_FORM (such as data4) to communicate that this is a subprogram-relative value. But that won't be sufficient for PROPELLER, for instance, which fragments subprograms so there's no unique low_pc for it to be relative to. So that leaves us maybe really wanting the addr+offset encoding discussed here: http://lists.llvm.org/pipermail/llvm-dev/2020-January/138029.html

[labath]

Thanks for fixing this. The lldb parts look good to me.

[aprantl]

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Makefile

10

It seems like one of the lib_Two dependencies is redundant? Presumably the one in a.out?

[dblaikie]

I ran some size analysis (& hadn't realized that this special case was only done in the DWARFv5 codepath... that leads me to wonder about size impact in DWARFv4, so maybe I'll need to go back and run some general size analysis cost of these call sites).

The .rela.debug_addr for a clang self host optimized (-O3) build, split DWARF, increased from 62.2MB to 70.5MB, which isn't great - but worse in the context of the further address reduction like the ones I've discussed on llvm-dev recently. "ranges everywhere" gets that 62.2MB down to 4.4MB, so growing it by a further 8MB would be pretty unfortunate.

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

984–991

Side question: How'd this end up like this? Why all these GDB tuning checks? Seems like it'd add another layer of complexity/variety that'll make it harder for us to all be evaluating the same things.

[vsk]

In D72489#1813533, @dblaikie wrote:

Thanks for looking into this!

Could you measure the size of the object files of, for example, the clang binary before/after this change - and, if possible, on Linux (where relocations are required to fixup these addresses)? I'm concerned this might increase the size significantly.

Hm, there's something I'm missing here. I verified that this change causes one extra X86_64_RELOC_UNSIGNED relocation to be emitted per call site entry in a MachO. For a stage2 build of clang on Darwin (2281 .o's), this patch increases the total file size of the .o's by 0.04% (6,970,706,668 bytes -> 6,973,643,092 bytes, a ~3MB increase). That size increase is much smaller than expected, because clang has ~5.5 million call site entries, and (IIUC) an X86_64_RELOC_UNSIGNED relocation takes 8 bytes. So I'd expect a ~42MB size increase. Does anyone have any insight into this discrepancy? Are relocations compressed, or did the previous subtraction encoding take up space elsewhere?

Could you check if this does the right thing for Split DWARF or DWARFv5? (I suspect it does - if you're using the same API as is used for DW_TAG_label's low_pc, for instance - which I imagine you are) - in terms of adding the address to debug_addr, and using addrx encoding.

I've added a test in test/DebugInfo/X86/debug_addr.ll.

I was going to suggest we use a non-standard extension to preserve the subprogram-relative behavior, by using a different DW_FORM (such as data4) to communicate that this is a subprogram-relative value. But that won't be sufficient for PROPELLER, for instance, which fragments subprograms so there's no unique low_pc for it to be relative to. So that leaves us maybe really wanting the addr+offset encoding discussed here: http://lists.llvm.org/pipermail/llvm-dev/2020-January/138029.html

I don't really understand the addr+offset encoding, I'll follow up on llvm-dev.

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Makefile

10

Ah, yep. If it's all right I'd rather just leave the redundant lib_Two dependency for a.out, to make it really explicit.

[vsk]

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

984–991

+ @djtodoro, I'm not sure why AT_call_return_pc would be needed at a tail call site as the debugger must ignore it. As for emitting DW_AT_low_pc under gdb tuning, I think this might be an artifact from the original GNU implementation.

[djtodoro]

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

984–991

I'm not sure why AT_call_return_pc would be needed at a tail call site as the debugger must ignore it. As for emitting DW_AT_low_pc under gdb tuning, I think this might be an artifact from the original GNU implementation.

Yes, that is the GNU implementation's heritage (I cannot remember why GCC generated the low_pc info in the case of the tail calls), but GNU GDB needs the low_pc (as an address) in order to handle the call_site and call_site_parameters debug info for non-tail calls. To avoiding the pc address info in the case of tail calls makes sense to me, since debuggers should avoid that info.

[djtodoro]

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

990

Why don't we use the getDwarf5OrGNUAttr() for the return_pc/low_pc, since we use the address in both cases now?

[dblaikie]

While the related design discussions continue - the patch itself is good/correct & there's nothing much to be done about the address pool size/relocations increase for now, for GDB at least, which is what I care about.

Perhaps it's best if I split off the design discussions into an llvm-dev thread.

llvm/test/tools/dsymutil/Inputs/call-site-entry.c

22–28

Would this be able to be simplified down to:

__attribute__((optnone)) void f() {
}
int main() {
  f();
}

(the attribute might be simpler than the command line argument to disable optimizations)

Or does the function need to return int to get a call_site?

[vsk]

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

990

I'll plan a follow-up for this, thanks.

llvm/test/tools/dsymutil/Inputs/call-site-entry.c

22–28

That's a nice simplification, I'll fold that in before committing.

lldb/include/lldb/Symbol/Function.h

virtual Function *GetCallee(ModuleList &images,
                               ExecutionContext &exe_ctx) = 0;
 
   /// Get the load PC address of the instruction which executes after the call
   /// returns. Returns LLDB_INVALID_ADDRESS iff this is a tail call. \p caller
   /// is the Function containing this call, and \p target is the Target which
   /// 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.
   llvm::ArrayRef<CallSiteParameter> GetCallSiteParameters() const {
     return parameters;
   }
 
 protected:
   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;
 };
 
 /// A direct call site. Used to represent call sites where the address of the
 /// callee is fixed (e.g. a function call in C in which the call target is not
 /// a function pointer).

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Makefile

+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

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/One.mk

+DYLIB_NAME := One
+DYLIB_C_SOURCES := One.c
+DYLIB_ONLY := YES
+CFLAGS_EXTRAS := -g -O2 -glldb
+LD_EXTRAS := -L. -lTwo
+
+include Makefile.rules

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/One/One.c

+#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();
+}

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/TestCrossDSOTailCalls.py

+"""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)

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Two.mk

+DYLIB_NAME := Two
+DYLIB_C_SOURCES := Two.c
+DYLIB_ONLY := YES
+CFLAGS_EXTRAS := -g -O2 -glldb
+
+include Makefile.rules

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/Two/Two.c

+#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();
+}

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/main.c

+#include "shared.h"
+
+__attribute__((noinline))
+static void helper() {
+  tail_called_in_a_from_main();
+}
+
+__attribute__((disable_tail_calls)) 
+int main() {
+  helper();
+  return 0;
+}

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_dso/shared.h

+void tail_called_in_a_from_main();
+
+void tail_called_in_b_from_a();

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/Makefile

+C_SOURCES := main.c One.c Two.c
+
+CFLAGS_EXTRAS := -g -O2 -glldb
+include Makefile.rules

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/One.c

+#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();
+}

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/TestCrossObjectTailCalls.py

+"""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)

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/Two.c

+#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();
+}

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/main.c

+#include "shared.h"
+
+__attribute__((noinline))
+static void helper() {
+  tail_called_in_a_from_main();
+}
+
+__attribute__((disable_tail_calls)) 
+int main() {
+  helper();
+  return 0;
+}

lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/cross_object/shared.h

+void tail_called_in_a_from_main();
+
+void tail_called_in_b_from_a();

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

protected:
 
   virtual UniqueDWARFASTTypeMap &GetUniqueDWARFASTTypeMap();
 
   bool DIEDeclContextsMatch(const DWARFDIE &die1, const DWARFDIE &die2);
 
   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;
 
   void GetTypes(const DWARFDIE &die, dw_offset_t min_die_offset,
                 dw_offset_t max_die_offset, uint32_t type_mask,
                 TypeSet &type_set);
 

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Function *SymbolFileDWARF::ParseFunction(CompileUnit &comp_unit,
   }
   DWARFASTParser *dwarf_ast = type_system_or_err->GetDWARFParser();
   if (!dwarf_ast)
     return nullptr;
 
   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) {
     return debug_map_symfile->LinkOSOAddress(addr);
   }
   // This is a normal DWARF file, no address fixups need to happen
   return true;
 }
if (LocationInCallee && LocationInCaller) {
       CallSiteParameter param = {*LocationInCallee, *LocationInCaller};
       parameters.push_back(param);
     }
   }
   return parameters;
 }
 
 /// 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 =
       function_die.GetAttributeValueAsUnsigned(DW_AT_call_all_calls, 0);
   if (!has_call_edges)
     return {};
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
for (size_t i = 0; i < num_attributes; ++i) {
             child.GetCU());
       }
     }
     if (!call_origin && !call_target) {
       LLDB_LOG(log, "CollectCallEdges: call site without any call target");
       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);
 
     std::unique_ptr<CallEdge> edge;
     if (call_origin) {
       LLDB_LOG(log, "CollectCallEdges: Found call origin: {0} (retn-PC: {1:x})",
                call_origin->GetPubname(), return_pc);

lldb/source/Symbol/Function.cpp

size_t InlineFunctionInfo::MemorySize() const {
   return FunctionInfo::MemorySize() + m_mangled.MemorySize();
 }
 
 /// @name Call site related structures
 /// @{
 
 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) {
   if (resolved)
     return;
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
   LLDB_LOG(log, "DirectCallEdge: Lazily parsing the call graph for {0}",

llvm/include/llvm/CodeGen/DebugHandlerBase.h

public:
   void endFunction(const MachineFunction *MF) override;
 
   /// Return Label preceding the instruction.
   MCSymbol *getLabelBeforeInsn(const MachineInstr *MI);
 
   /// 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);
 };
 
 }
 
 #endif

llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp

MCSymbol *DebugHandlerBase::getLabelBeforeInsn(const MachineInstr *MI) {
   return Label;
 }
 
 // Return Label immediately following the instruction.
 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);
   const DIDerivedType *DDTy = dyn_cast<DIDerivedType>(Ty);
   if (!DDTy)
     return Ty->getSizeInBits();
 
   unsigned Tag = DDTy->getTag();

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h

public:
   dwarf::Attribute getDwarf5OrGNUAttr(dwarf::Attribute Attr) const;
 
   /// This takes a DWARF 5 location atom and either returns it or a GNU analog.
   dwarf::LocationAtom getDwarf5OrGNULocationAtom(dwarf::LocationAtom Loc) const;
 
   /// 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
   ///       call site parameters by the DW_AT_location attribute.
   void constructCallSiteParmEntryDIEs(DIE &CallSiteDIE,
                                       SmallVector<DbgCallSiteParam, 4> &Params);
 
   /// Construct import_module DIE.
public:
 
   /// Add a Dwarf loclistptr attribute data and value.
   void addLocationList(DIE &Die, dwarf::Attribute Attribute, unsigned Index);
   void applyVariableAttributes(const DbgVariable &Var, DIE &VariableDie);
 
   /// 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);
 
   void applyLabelAttributes(const DbgLabel &Label, DIE &LabelDie);
 
   /// getRanges - Get the list of ranges for this unit.
   const SmallVectorImpl<RangeSpan> &getRanges() const { return CURanges; }
   SmallVector<RangeSpan, 2> takeRanges() { return std::move(CURanges); }

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

DwarfCompileUnit::getDwarf5OrGNULocationAtom(dwarf::LocationAtom Loc) const {
   switch (Loc) {
   case dwarf::DW_OP_entry_value:
     return dwarf::DW_OP_GNU_entry_value;
   default:
     llvm_unreachable("DWARF5 location atom with no GNU analog");
   }
 }
 
-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);
 
   if (CallReg) {
     // Indirect call.
     addAddress(CallSiteDIE, getDwarf5OrGNUAttr(dwarf::DW_AT_call_target),
                MachineLocation(CallReg));
   } else {
     DIE *CalleeDIE = getDIE(CalleeSP);
     assert(CalleeDIE && "Could not find DIE for call site entry origin");
     addDIEEntry(CallSiteDIE, getDwarf5OrGNUAttr(dwarf::DW_AT_call_origin),
                 *CalleeDIE);
   }
 
   if (IsTail)
     // Attach DW_AT_call_tail_call to tail calls for standards compliance.
     addFlag(CallSiteDIE, getDwarf5OrGNUAttr(dwarf::DW_AT_call_tail_call));
 
   // Attach the return PC to allow the debugger to disambiguate call paths
   // from one function to another.
   if (DD->getDwarfVersion() == 4 && DD->tuneForGDB()) {
     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;
 }
 
 void DwarfCompileUnit::constructCallSiteParmEntryDIEs(
     DIE &CallSiteDIE, SmallVector<DbgCallSiteParam, 4> &Params) {
   for (const auto &Param : Params) {
     unsigned Register = Param.getRegister();

 }
 
 /// Add a Dwarf expression attribute data and value.
 void DwarfCompileUnit::addExpr(DIELoc &Die, dwarf::Form Form,
                                const MCExpr *Expr) {
   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();
   auto *Context = SPDecl ? SPDecl->getScope() : SP->getScope();
   applySubprogramAttributes(SP, SPDie, includeMinimalInlineScopes());
   addGlobalName(SP->getName(), SPDie, Context);
 }
 

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

void DwarfDebug::constructCallSiteEntryDIEs(const DISubprogram &SP,
   // Use DW_AT_call_all_calls to express that call site entries are present
   // for both tail and non-tail calls. Don't use DW_AT_call_all_source_calls
   // because one of its requirements is not met: call site entries for
   // optimized-out calls are elided.
   CU.addFlag(ScopeDIE, CU.getDwarf5OrGNUAttr(dwarf::DW_AT_call_all_calls));
 
   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) {
     for (const MachineInstr &MI : MBB.instrs()) {
       // Bundles with call in them will pass the isCall() test below but do not
       // have callee operand information so skip them here. Iterator will
       // eventually reach the call MI.
       if (MI.isBundle())
for (const MachineInstr &MI : MBB.instrs()) {
       bool IsTail = TII->isTailCall(MI);
 
       // If MI is in a bundle, the label was created after the bundle since
       // EmitFunctionBody iterates over top-level MIs. Get that top-level MI
       // to search for that label below.
       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()
                                        : StringRef(MF.getSubtarget()
                                                        .getRegisterInfo()
                                                        ->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 &&
           (tuneForGDB() || tuneForLLDB())) {
         ParamSet Params;
         // Try to interpret values of call site parameters.
         collectCallSiteParameters(&MI, Params);
         CU.constructCallSiteParmEntryDIEs(CallSiteDIE, Params);

llvm/test/tools/dsymutil/Inputs/call-site-entry.c

+/*
+ * 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.
+ */
+
+int zero() {
+  return 0;
+}
+
+int main() {
+  return zero();
+}

llvm/test/tools/dsymutil/call-site-entry-linking.test

+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)

llvm/tools/dsymutil/DwarfLinker.cpp

if (Die.getTag() == dwarf::DW_TAG_compile_unit) {
         Addr = HighPc;
       else
         return 0;
     } else
       // If we have a high_pc recorded for the input DIE, use
       // 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),
                static_cast<dwarf::Form>(AttrSpec.Form), DIEInteger(Addr));
   return Unit.getOrigUnit().getAddressByteSize();
 }
 
 unsigned DwarfLinker::DIECloner::cloneScalarAttribute(