Skip to content

Commit f012055

Browse files
vedantkvedantk
committed
[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
1 parent d629525 commit f012055

File tree

29 files changed

+338
-68
lines changed

29 files changed

+338
-68
lines changed

‎lldb/include/lldb/Symbol/Function.h

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -281,8 +281,7 @@ class CallEdge {
281281
/// made the call.
282282
lldb::addr_t GetReturnPCAddress(Function &caller, Target &target) const;
283283

284-
/// Like \ref GetReturnPCAddress, but returns an unslid function-local PC
285-
/// offset.
284+
/// Like \ref GetReturnPCAddress, but returns an unresolved file address.
286285
lldb::addr_t GetUnresolvedReturnPCAddress() const { return return_pc; }
287286

288287
/// Get the call site parameters available at this call edge.
@@ -294,9 +293,8 @@ class CallEdge {
294293
CallEdge(lldb::addr_t return_pc, CallSiteParameterArray &&parameters)
295294
: return_pc(return_pc), parameters(std::move(parameters)) {}
296295

297-
/// An invalid address if this is a tail call. Otherwise, the function-local
298-
/// PC offset. Adding this PC offset to the function's base load address
299-
/// gives the return PC for the call.
296+
/// An invalid address if this is a tail call. Otherwise, the return PC for
297+
/// the call. Note that this is a file address which must be resolved.
300298
lldb::addr_t return_pc;
301299

302300
CallSiteParameterArray parameters;

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
LD_EXTRAS := -L. -l$(LIB_PREFIX)One -l$(LIB_PREFIX)Two
2+
C_SOURCES := main.c
3+
CFLAGS_EXTRAS := -g -O2 -glldb
4+
5+
include Makefile.rules
6+
7+
.PHONY:
8+
a.out: lib_One lib_Two
9+
10+
lib_One: lib_Two
11+
12+
lib_%:
13+
$(MAKE) VPATH=$(SRCDIR)/$* -I $(SRCDIR) -f $(SRCDIR)/$*.mk DSYMUTIL=$(DSYMUTIL)
14+
15+
clean::
16+
$(MAKE) -f $(SRCDIR)/One.mk clean
17+
$(MAKE) -f $(SRCDIR)/Two.mk clean

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
DYLIB_NAME := One
2+
DYLIB_C_SOURCES := One.c
3+
DYLIB_ONLY := YES
4+
CFLAGS_EXTRAS := -g -O2 -glldb
5+
LD_EXTRAS := -L. -lTwo
6+
7+
include Makefile.rules

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#include "shared.h"
2+
3+
__attribute__((noinline))
4+
static void helper_in_a() {
5+
tail_called_in_b_from_a();
6+
}
7+
8+
__attribute__((disable_tail_calls))
9+
void tail_called_in_a_from_main() {
10+
helper_in_a();
11+
}

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
"""Test that backtraces can follow cross-DSO tail calls"""
2+
3+
4+
5+
import lldb
6+
from lldbsuite.test.decorators import *
7+
from lldbsuite.test.lldbtest import *
8+
from lldbsuite.test import lldbutil
9+
10+
11+
class TestCrossDSOTailCalls(TestBase):
12+
13+
mydir = TestBase.compute_mydir(__file__)
14+
15+
def setUp(self):
16+
TestBase.setUp(self)
17+
18+
@skipIf(compiler="clang", compiler_version=['<', '8.0'])
19+
@skipIf(dwarf_version=['<', '4'])
20+
@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
21+
def test_cross_dso_tail_calls(self):
22+
self.build()
23+
exe = self.getBuildArtifact("a.out")
24+
target = self.dbg.CreateTarget(exe)
25+
self.assertTrue(target, VALID_TARGET)
26+
27+
# Register our shared libraries for remote targets so they get
28+
# automatically uploaded
29+
environment = self.registerSharedLibrariesWithTarget(
30+
target, ['One', 'Two'])
31+
32+
lldbutil.run_break_set_by_source_regexp(self, '// break here',
33+
extra_options='-f Two.c')
34+
35+
process = target.LaunchSimple(
36+
None, environment, self.get_process_working_directory())
37+
self.assertTrue(process, PROCESS_IS_VALID)
38+
39+
# We should be stopped in the second dylib.
40+
thread = lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint)
41+
42+
# Debug helper:
43+
# self.runCmd("log enable -f /tmp/lldb.log lldb step")
44+
# self.runCmd("bt")
45+
46+
# Check that the backtrace is what we expect:
47+
# frame #0: 0x000000010d5e5f94 libTwo.dylib`tail_called_in_b_from_b at Two.c:7:3 [opt]
48+
# frame #1: 0x000000010d5e5fa0 libTwo.dylib`tail_called_in_b_from_a [opt] [artificial]
49+
# frame #2: 0x000000010d5dcf80 libOne.dylib`helper_in_a [opt] [artificial]
50+
# frame #3: 0x000000010d5dcf79 libOne.dylib`tail_called_in_a_from_main at One.c:10:3 [opt]
51+
# frame #4: 0x000000010d5d3f80 a.out`helper [opt] [artificial]
52+
# frame #5: 0x000000010d5d3f79 a.out`main at main.c:10:3 [opt]
53+
expected_frames = [
54+
("tail_called_in_b_from_b", False),
55+
("tail_called_in_b_from_a", True),
56+
("helper_in_a", True),
57+
("tail_called_in_a_from_main", False),
58+
("helper", True),
59+
("main", False)
60+
]
61+
for idx, (name, is_artificial) in enumerate(expected_frames):
62+
frame = thread.GetFrameAtIndex(idx)
63+
self.assertTrue(name in frame.GetDisplayFunctionName())
64+
self.assertEqual(frame.IsArtificial(), is_artificial)

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
DYLIB_NAME := Two
2+
DYLIB_C_SOURCES := Two.c
3+
DYLIB_ONLY := YES
4+
CFLAGS_EXTRAS := -g -O2 -glldb
5+
6+
include Makefile.rules

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#include "shared.h"
2+
3+
volatile int x;
4+
5+
__attribute__((noinline))
6+
void tail_called_in_b_from_b() {
7+
++x; // break here
8+
}
9+
10+
void tail_called_in_b_from_a() {
11+
tail_called_in_b_from_b();
12+
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#include "shared.h"
2+
3+
__attribute__((noinline))
4+
static void helper() {
5+
tail_called_in_a_from_main();
6+
}
7+
8+
__attribute__((disable_tail_calls))
9+
int main() {
10+
helper();
11+
return 0;
12+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
void tail_called_in_a_from_main();
2+
3+
void tail_called_in_b_from_a();

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
C_SOURCES := main.c One.c Two.c
2+
3+
CFLAGS_EXTRAS := -g -O2 -glldb
4+
include Makefile.rules

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#include "shared.h"
2+
3+
__attribute__((noinline))
4+
static void helper_in_a() {
5+
tail_called_in_b_from_a();
6+
}
7+
8+
__attribute__((disable_tail_calls))
9+
void tail_called_in_a_from_main() {
10+
helper_in_a();
11+
}

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
"""Test that backtraces can follow cross-object tail calls"""
2+
3+
4+
5+
import lldb
6+
from lldbsuite.test.decorators import *
7+
from lldbsuite.test.lldbtest import *
8+
from lldbsuite.test import lldbutil
9+
10+
11+
class TestCrossObjectTailCalls(TestBase):
12+
13+
mydir = TestBase.compute_mydir(__file__)
14+
15+
def setUp(self):
16+
TestBase.setUp(self)
17+
18+
@skipIf(compiler="clang", compiler_version=['<', '8.0'])
19+
@skipIf(dwarf_version=['<', '4'])
20+
@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
21+
def test_cross_object_tail_calls(self):
22+
self.build()
23+
exe = self.getBuildArtifact("a.out")
24+
target = self.dbg.CreateTarget(exe)
25+
self.assertTrue(target, VALID_TARGET)
26+
27+
lldbutil.run_break_set_by_source_regexp(self, '// break here',
28+
extra_options='-f Two.c')
29+
30+
process = target.LaunchSimple(
31+
None, None, self.get_process_working_directory())
32+
self.assertTrue(process, PROCESS_IS_VALID)
33+
34+
# We should be stopped in the second dylib.
35+
thread = lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint)
36+
37+
# Debug helper:
38+
# self.runCmd("log enable -f /tmp/lldb.log lldb step")
39+
# self.runCmd("bt")
40+
41+
# Check that the backtrace is what we expect:
42+
# frame #0: 0x000000010be73f94 a.out`tail_called_in_b_from_b at Two.c:7:3 [opt]
43+
# frame #1: 0x000000010be73fa0 a.out`tail_called_in_b_from_a at Two.c:8:1 [opt] [artificial]
44+
# frame #2: 0x000000010be73f80 a.out`helper_in_a at One.c:11:1 [opt] [artificial]
45+
# frame #3: 0x000000010be73f79 a.out`tail_called_in_a_from_main at One.c:10:3 [opt]
46+
# frame #4: 0x000000010be73f60 a.out`helper at main.c:11:3 [opt] [artificial]
47+
# frame #5: 0x000000010be73f59 a.out`main at main.c:10:3 [opt]
48+
expected_frames = [
49+
("tail_called_in_b_from_b", False),
50+
("tail_called_in_b_from_a", True),
51+
("helper_in_a", True),
52+
("tail_called_in_a_from_main", False),
53+
("helper", True),
54+
("main", False)
55+
]
56+
for idx, (name, is_artificial) in enumerate(expected_frames):
57+
frame = thread.GetFrameAtIndex(idx)
58+
self.assertTrue(name in frame.GetDisplayFunctionName())
59+
self.assertEqual(frame.IsArtificial(), is_artificial)

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#include "shared.h"
2+
3+
volatile int x;
4+
5+
__attribute__((noinline))
6+
void tail_called_in_b_from_b() {
7+
++x; // break here
8+
}
9+
10+
void tail_called_in_b_from_a() {
11+
tail_called_in_b_from_b();
12+
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#include "shared.h"
2+
3+
__attribute__((noinline))
4+
static void helper() {
5+
tail_called_in_a_from_main();
6+
}
7+
8+
__attribute__((disable_tail_calls))
9+
int main() {
10+
helper();
11+
return 0;
12+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
void tail_called_in_a_from_main();
2+
3+
void tail_called_in_b_from_a();

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,13 @@ Function *SymbolFileDWARF::ParseFunction(CompileUnit &comp_unit,
792792
return dwarf_ast->ParseFunctionFromDWARF(comp_unit, die);
793793
}
794794

795+
lldb::addr_t SymbolFileDWARF::FixupAddress(lldb::addr_t file_addr) {
796+
SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
797+
if (debug_map_symfile)
798+
return debug_map_symfile->LinkOSOFileAddress(this, file_addr);
799+
return file_addr;
800+
}
801+
795802
bool SymbolFileDWARF::FixupAddress(Address &addr) {
796803
SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
797804
if (debug_map_symfile) {
@@ -3805,8 +3812,8 @@ CollectCallSiteParameters(ModuleSP module, DWARFDIE call_site_die) {
38053812
}
38063813

38073814
/// Collect call graph edges present in a function DIE.
3808-
static std::vector<std::unique_ptr<lldb_private::CallEdge>>
3809-
CollectCallEdges(ModuleSP module, DWARFDIE function_die) {
3815+
std::vector<std::unique_ptr<lldb_private::CallEdge>>
3816+
SymbolFileDWARF::CollectCallEdges(ModuleSP module, DWARFDIE function_die) {
38103817
// Check if the function has a supported call site-related attribute.
38113818
// TODO: In the future it may be worthwhile to support call_all_source_calls.
38123819
uint64_t has_call_edges =
@@ -3882,6 +3889,11 @@ CollectCallEdges(ModuleSP module, DWARFDIE function_die) {
38823889
continue;
38833890
}
38843891

3892+
// Adjust the return PC. It needs to be fixed up if the main executable
3893+
// contains a debug map (i.e. pointers to object files), because we need a
3894+
// file address relative to the executable's text section.
3895+
return_pc = FixupAddress(return_pc);
3896+
38853897
// Extract call site parameters.
38863898
CallSiteParameterArray parameters =
38873899
CollectCallSiteParameters(module, child);

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,18 @@ class SymbolFileDWARF : public lldb_private::SymbolFile,
417417
bool ClassContainsSelector(const DWARFDIE &class_die,
418418
lldb_private::ConstString selector);
419419

420+
/// Parse call site entries (DW_TAG_call_site), including any nested call site
421+
/// parameters (DW_TAG_call_site_parameter).
422+
std::vector<std::unique_ptr<lldb_private::CallEdge>>
423+
CollectCallEdges(lldb::ModuleSP module, DWARFDIE function_die);
424+
425+
/// If this symbol file is linked to by a debug map (see
426+
/// SymbolFileDWARFDebugMap), and \p file_addr is a file address relative to
427+
/// an object file, adjust \p file_addr so that it is relative to the main
428+
/// binary. Returns the adjusted address, or \p file_addr if no adjustment is
429+
/// needed, on success and LLDB_INVALID_ADDRESS otherwise.
430+
lldb::addr_t FixupAddress(lldb::addr_t file_addr);
431+
420432
bool FixupAddress(lldb_private::Address &addr);
421433

422434
typedef std::set<lldb_private::Type *> TypeSet;

‎lldb/source/Symbol/Function.cpp

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,25 @@ size_t InlineFunctionInfo::MemorySize() const {
123123

124124
lldb::addr_t CallEdge::GetReturnPCAddress(Function &caller,
125125
Target &target) const {
126-
const Address &base = caller.GetAddressRange().GetBaseAddress();
127-
return base.GetLoadAddress(&target) + return_pc;
126+
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
127+
128+
const Address &caller_start_addr = caller.GetAddressRange().GetBaseAddress();
129+
130+
ModuleSP caller_module_sp = caller_start_addr.GetModule();
131+
if (!caller_module_sp) {
132+
LLDB_LOG(log, "GetReturnPCAddress: cannot get Module for caller");
133+
return LLDB_INVALID_ADDRESS;
134+
}
135+
136+
SectionList *section_list = caller_module_sp->GetSectionList();
137+
if (!section_list) {
138+
LLDB_LOG(log, "GetReturnPCAddress: cannot get SectionList for Module");
139+
return LLDB_INVALID_ADDRESS;
140+
}
141+
142+
Address return_pc_addr = Address(return_pc, section_list);
143+
lldb::addr_t ret_addr = return_pc_addr.GetLoadAddress(&target);
144+
return ret_addr;
128145
}
129146

130147
void DirectCallEdge::ParseSymbolFileAndResolve(ModuleList &images) {

‎llvm/include/llvm/CodeGen/DebugHandlerBase.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,6 @@ class DebugHandlerBase : public AsmPrinterHandler {
124124
/// Return Label immediately following the instruction.
125125
MCSymbol *getLabelAfterInsn(const MachineInstr *MI);
126126

127-
/// Return the function-local offset of an instruction. A label for the
128-
/// instruction \p MI should exist (\ref getLabelAfterInsn).
129-
const MCExpr *getFunctionLocalOffsetAfterInsn(const MachineInstr *MI);
130-
131127
/// If this type is derived from a base type then return base type size.
132128
static uint64_t getBaseTypeSize(const DIType *Ty);
133129
};

‎llvm/lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -124,21 +124,6 @@ MCSymbol *DebugHandlerBase::getLabelAfterInsn(const MachineInstr *MI) {
124124
return LabelsAfterInsn.lookup(MI);
125125
}
126126

127-
// Return the function-local offset of an instruction.
128-
const MCExpr *
129-
DebugHandlerBase::getFunctionLocalOffsetAfterInsn(const MachineInstr *MI) {
130-
MCContext &MC = Asm->OutContext;
131-
132-
MCSymbol *Start = Asm->getFunctionBegin();
133-
const auto *StartRef = MCSymbolRefExpr::create(Start, MC);
134-
135-
MCSymbol *AfterInsn = getLabelAfterInsn(MI);
136-
assert(AfterInsn && "Expected label after instruction");
137-
const auto *AfterRef = MCSymbolRefExpr::create(AfterInsn, MC);
138-
139-
return MCBinaryExpr::createSub(AfterRef, StartRef, MC);
140-
}
141-
142127
/// If this type is derived from a base type then return base type size.
143128
uint64_t DebugHandlerBase::getBaseTypeSize(const DIType *Ty) {
144129
assert(Ty);

0 commit comments

Comments
 (0)