Skip to content

Commit be9f5e1

Browse files
authored
Merge pull request #583 from vedantk/cherry
Emit AT_call_return_pc as an address, refactor MD_loop update logic
2 parents 8ab06d5 + ebb7e40 commit be9f5e1

File tree

32 files changed

+391
-115
lines changed

32 files changed

+391
-115
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;
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
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
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+
}
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)
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
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+
}
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+
}
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();
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
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+
}
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)
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+
}
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+
}
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
@@ -807,6 +807,13 @@ Function *SymbolFileDWARF::ParseFunction(CompileUnit &comp_unit,
807807
return dwarf_ast->ParseFunctionFromDWARF(comp_unit, die);
808808
}
809809

810+
lldb::addr_t SymbolFileDWARF::FixupAddress(lldb::addr_t file_addr) {
811+
SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
812+
if (debug_map_symfile)
813+
return debug_map_symfile->LinkOSOFileAddress(this, file_addr);
814+
return file_addr;
815+
}
816+
810817
bool SymbolFileDWARF::FixupAddress(Address &addr) {
811818
SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
812819
if (debug_map_symfile) {
@@ -3820,8 +3827,8 @@ CollectCallSiteParameters(ModuleSP module, DWARFDIE call_site_die) {
38203827
}
38213828

38223829
/// Collect call graph edges present in a function DIE.
3823-
static std::vector<std::unique_ptr<lldb_private::CallEdge>>
3824-
CollectCallEdges(ModuleSP module, DWARFDIE function_die) {
3830+
std::vector<std::unique_ptr<lldb_private::CallEdge>>
3831+
SymbolFileDWARF::CollectCallEdges(ModuleSP module, DWARFDIE function_die) {
38253832
// Check if the function has a supported call site-related attribute.
38263833
// TODO: In the future it may be worthwhile to support call_all_source_calls.
38273834
uint64_t has_call_edges =
@@ -3897,6 +3904,11 @@ CollectCallEdges(ModuleSP module, DWARFDIE function_die) {
38973904
continue;
38983905
}
38993906

3907+
// Adjust the return PC. It needs to be fixed up if the main executable
3908+
// contains a debug map (i.e. pointers to object files), because we need a
3909+
// file address relative to the executable's text section.
3910+
return_pc = FixupAddress(return_pc);
3911+
39003912
// Extract call site parameters.
39013913
CallSiteParameterArray parameters =
39023914
CollectCallSiteParameters(module, child);

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

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

429+
/// Parse call site entries (DW_TAG_call_site), including any nested call site
430+
/// parameters (DW_TAG_call_site_parameter).
431+
std::vector<std::unique_ptr<lldb_private::CallEdge>>
432+
CollectCallEdges(lldb::ModuleSP module, DWARFDIE function_die);
433+
434+
/// If this symbol file is linked to by a debug map (see
435+
/// SymbolFileDWARFDebugMap), and \p file_addr is a file address relative to
436+
/// an object file, adjust \p file_addr so that it is relative to the main
437+
/// binary. Returns the adjusted address, or \p file_addr if no adjustment is
438+
/// needed, on success and LLDB_INVALID_ADDRESS otherwise.
439+
lldb::addr_t FixupAddress(lldb::addr_t file_addr);
440+
429441
bool FixupAddress(lldb_private::Address &addr);
430442

431443
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
@@ -133,8 +133,25 @@ size_t InlineFunctionInfo::MemorySize() const {
133133

134134
lldb::addr_t CallEdge::GetReturnPCAddress(Function &caller,
135135
Target &target) const {
136-
const Address &base = caller.GetAddressRange().GetBaseAddress();
137-
return base.GetLoadAddress(&target) + return_pc;
136+
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
137+
138+
const Address &caller_start_addr = caller.GetAddressRange().GetBaseAddress();
139+
140+
ModuleSP caller_module_sp = caller_start_addr.GetModule();
141+
if (!caller_module_sp) {
142+
LLDB_LOG(log, "GetReturnPCAddress: cannot get Module for caller");
143+
return LLDB_INVALID_ADDRESS;
144+
}
145+
146+
SectionList *section_list = caller_module_sp->GetSectionList();
147+
if (!section_list) {
148+
LLDB_LOG(log, "GetReturnPCAddress: cannot get SectionList for Module");
149+
return LLDB_INVALID_ADDRESS;
150+
}
151+
152+
Address return_pc_addr = Address(return_pc, section_list);
153+
lldb::addr_t ret_addr = return_pc_addr.GetLoadAddress(&target);
154+
return ret_addr;
138155
}
139156

140157
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/include/llvm/IR/DebugInfo.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#ifndef LLVM_IR_DEBUGINFO_H
1717
#define LLVM_IR_DEBUGINFO_H
1818

19+
#include "llvm/ADT/STLExtras.h"
1920
#include "llvm/ADT/SmallPtrSet.h"
2021
#include "llvm/ADT/SmallVector.h"
2122
#include "llvm/ADT/iterator_range.h"
@@ -25,6 +26,7 @@ namespace llvm {
2526

2627
class DbgDeclareInst;
2728
class DbgValueInst;
29+
class Instruction;
2830
class Module;
2931

3032
/// Find subprogram that is enclosing this scope.
@@ -50,6 +52,13 @@ bool stripDebugInfo(Function &F);
5052
/// All debug type metadata nodes are unreachable and garbage collected.
5153
bool stripNonLineTableDebugInfo(Module &M);
5254

55+
/// Update the debug locations contained within the MD_loop metadata attached
56+
/// to the instruction \p I, if one exists. \p Updater is applied to each debug
57+
/// location in the MD_loop metadata: the returned value is included in the
58+
/// updated loop metadata node if it is non-null.
59+
void updateLoopMetadataDebugLocations(
60+
Instruction &I, function_ref<DILocation *(const DILocation &)> Updater);
61+
5362
/// Return Debug Info Metadata Version by checking module flags.
5463
unsigned getDebugMetadataVersionFromModule(const Module &M);
5564

0 commit comments

Comments
 (0)