Author: Vedant Kumar Date: 2020-03-24T12:22:12-07:00 New Revision: 0a9b91c390b281e90e51d5839557c5a189dd5401
URL: https://github.com/llvm/llvm-project/commit/0a9b91c390b281e90e51d5839557c5a189dd5401 DIFF: https://github.com/llvm/llvm-project/commit/0a9b91c390b281e90e51d5839557c5a189dd5401.diff LOG: Revert "[lldb/DWARF] Use DW_AT_call_pc to determine artificial frame address" This reverts commit 6905394d153960ded3a7b884a9747ed2d4a6e8d8. The changed test is failing on Debian/x86_64, possibly because lldb is subtracting an offset from the DW_AT_call_pc address used for the artificial frame: http://lab.llvm.org:8011/builders/lldb-x86_64-debian/builds/7171/steps/test/logs/stdio /home/worker/lldb-x86_64-debian/lldb-x86_64-debian/llvm-project/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp:6:17: error: CHECK-NEXT: expected string not found in input // CHECK-NEXT: frame #1: 0x{{[0-9a-f]+}} a.out`func3() at main.cpp:14:3 [opt] [artificial] ^ <stdin>:3:2: note: scanning from here frame #1: 0x0000000000401127 a.out`func3() at main.cpp:13:4 [opt] [artificial] Added: Modified: lldb/include/lldb/Symbol/Function.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Symbol/Function.cpp lldb/source/Target/StackFrameList.cpp lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Symbol/Function.h b/lldb/include/lldb/Symbol/Function.h index 40d316fa78eb..0db9a5116d25 100644 --- a/lldb/include/lldb/Symbol/Function.h +++ b/lldb/include/lldb/Symbol/Function.h @@ -284,33 +284,19 @@ class CallEdge { /// Like \ref GetReturnPCAddress, but returns an unresolved file address. lldb::addr_t GetUnresolvedReturnPCAddress() const { return return_pc; } - /// Get the load PC address of the call instruction (or LLDB_INVALID_ADDRESS). - lldb::addr_t GetCallInstPC(Function &caller, Target &target) const; - /// Get the call site parameters available at this call edge. llvm::ArrayRef<CallSiteParameter> GetCallSiteParameters() const { return parameters; } protected: - CallEdge(lldb::addr_t return_pc, lldb::addr_t call_inst_pc, - CallSiteParameterArray &¶meters) - : return_pc(return_pc), call_inst_pc(call_inst_pc), - parameters(std::move(parameters)) {} - - /// Helper that finds the load address of \p unresolved_pc, a file address - /// which refers to an instruction within \p caller. - static lldb::addr_t GetLoadAddress(lldb::addr_t unresolved_pc, - Function &caller, Target &target); + CallEdge(lldb::addr_t return_pc, CallSiteParameterArray &¶meters) + : return_pc(return_pc), parameters(std::move(parameters)) {} /// 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; - /// The address of the call instruction. Usually an invalid address, unless - /// this is a tail call. - lldb::addr_t call_inst_pc; - CallSiteParameterArray parameters; }; @@ -322,8 +308,8 @@ class DirectCallEdge : public CallEdge { /// Construct a call edge using a symbol name to identify the callee, and a /// return PC within the calling function to identify a specific call site. DirectCallEdge(const char *symbol_name, lldb::addr_t return_pc, - lldb::addr_t call_inst_pc, CallSiteParameterArray &¶meters) - : CallEdge(return_pc, call_inst_pc, std::move(parameters)) { + CallSiteParameterArray &¶meters) + : CallEdge(return_pc, std::move(parameters)) { lazy_callee.symbol_name = symbol_name; } @@ -353,9 +339,8 @@ class IndirectCallEdge : public CallEdge { /// Construct a call edge using a DWARFExpression to identify the callee, and /// a return PC within the calling function to identify a specific call site. IndirectCallEdge(DWARFExpression call_target, lldb::addr_t return_pc, - lldb::addr_t call_inst_pc, CallSiteParameterArray &¶meters) - : CallEdge(return_pc, call_inst_pc, std::move(parameters)), + : CallEdge(return_pc, std::move(parameters)), call_target(std::move(call_target)) {} Function *GetCallee(ModuleList &images, ExecutionContext &exe_ctx) override; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index c98694fca6b5..a703b1e1217d 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -3737,7 +3737,6 @@ SymbolFileDWARF::CollectCallEdges(ModuleSP module, DWARFDIE function_die) { llvm::Optional<DWARFDIE> call_origin; llvm::Optional<DWARFExpression> call_target; addr_t return_pc = LLDB_INVALID_ADDRESS; - addr_t call_inst_pc = LLDB_INVALID_ADDRESS; DWARFAttributes attributes; const size_t num_attributes = child.GetAttributes(attributes); @@ -3766,12 +3765,6 @@ SymbolFileDWARF::CollectCallEdges(ModuleSP module, DWARFDIE function_die) { if (attr == DW_AT_call_return_pc) return_pc = form_value.Address(); - // Extract DW_AT_call_pc (the PC at the call/branch instruction). It - // should only ever be unavailable for non-tail calls, in which case use - // LLDB_INVALID_ADDRESS. - if (attr == DW_AT_call_pc) - call_inst_pc = form_value.Address(); - // Extract DW_AT_call_target (the location of the address of the indirect // call). if (attr == DW_AT_call_target) { @@ -3794,11 +3787,10 @@ SymbolFileDWARF::CollectCallEdges(ModuleSP module, DWARFDIE function_die) { continue; } - // Adjust any PC forms. It needs to be fixed up if the main executable + // 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); - call_inst_pc = FixupAddress(call_inst_pc); // Extract call site parameters. CallSiteParameterArray parameters = @@ -3806,13 +3798,10 @@ SymbolFileDWARF::CollectCallEdges(ModuleSP module, DWARFDIE function_die) { std::unique_ptr<CallEdge> edge; if (call_origin) { - LLDB_LOG(log, - "CollectCallEdges: Found call origin: {0} (retn-PC: {1:x}) " - "(call-PC: {2:x})", - call_origin->GetPubname(), return_pc, call_inst_pc); + LLDB_LOG(log, "CollectCallEdges: Found call origin: {0} (retn-PC: {1:x})", + call_origin->GetPubname(), return_pc); edge = std::make_unique<DirectCallEdge>(call_origin->GetMangledName(), - return_pc, call_inst_pc, - std::move(parameters)); + return_pc, std::move(parameters)); } else { if (log) { StreamString call_target_desc; @@ -3821,8 +3810,8 @@ SymbolFileDWARF::CollectCallEdges(ModuleSP module, DWARFDIE function_die) { LLDB_LOG(log, "CollectCallEdges: Found indirect call target: {0}", call_target_desc.GetString()); } - edge = std::make_unique<IndirectCallEdge>( - *call_target, return_pc, call_inst_pc, std::move(parameters)); + edge = std::make_unique<IndirectCallEdge>(*call_target, return_pc, + std::move(parameters)); } if (log && parameters.size()) { diff --git a/lldb/source/Symbol/Function.cpp b/lldb/source/Symbol/Function.cpp index 0b1f6a8c3a3d..e1d5a720bcc3 100644 --- a/lldb/source/Symbol/Function.cpp +++ b/lldb/source/Symbol/Function.cpp @@ -120,36 +120,27 @@ size_t InlineFunctionInfo::MemorySize() const { /// @name Call site related structures /// @{ -lldb::addr_t CallEdge::GetLoadAddress(lldb::addr_t unresolved_pc, - Function &caller, Target &target) { +lldb::addr_t CallEdge::GetReturnPCAddress(Function &caller, + Target &target) const { 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, "GetLoadAddress: cannot get Module for caller"); + 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, "GetLoadAddress: cannot get SectionList for Module"); + LLDB_LOG(log, "GetReturnPCAddress: cannot get SectionList for Module"); return LLDB_INVALID_ADDRESS; } - Address the_addr = Address(unresolved_pc, section_list); - lldb::addr_t load_addr = the_addr.GetLoadAddress(&target); - return load_addr; -} - -lldb::addr_t CallEdge::GetReturnPCAddress(Function &caller, - Target &target) const { - return GetLoadAddress(return_pc, caller, target); -} - -lldb::addr_t CallEdge::GetCallInstPC(Function &caller, Target &target) const { - return GetLoadAddress(call_inst_pc, caller, target); + 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/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp index 83be7291df0c..e8e72203e204 100644 --- a/lldb/source/Target/StackFrameList.cpp +++ b/lldb/source/Target/StackFrameList.cpp @@ -236,17 +236,13 @@ void StackFrameList::GetOnlyConcreteFramesUpTo(uint32_t end_idx, m_frames.resize(num_frames); } -/// A sequence of calls that comprise some portion of a backtrace. Each frame -/// is represented as a pair of a callee (Function *) and an address within the -/// callee. -using CallSequence = std::vector<std::pair<Function *, addr_t>>; - /// Find the unique path through the call graph from \p begin (with return PC /// \p return_pc) to \p end. On success this path is stored into \p path, and /// on failure \p path is unchanged. static void FindInterveningFrames(Function &begin, Function &end, ExecutionContext &exe_ctx, Target &target, - addr_t return_pc, CallSequence &path, + addr_t return_pc, + std::vector<Function *> &path, ModuleList &images, Log *log) { LLDB_LOG(log, "Finding frames between {0} and {1}, retn-pc={2:x}", begin.GetDisplayName(), end.GetDisplayName(), return_pc); @@ -279,27 +275,24 @@ static void FindInterveningFrames(Function &begin, Function &end, // Fully explore the set of functions reachable from the first edge via tail // calls in order to detect ambiguous executions. struct DFS { - CallSequence active_path = {}; - CallSequence solution_path = {}; + std::vector<Function *> active_path = {}; + std::vector<Function *> solution_path = {}; llvm::SmallPtrSet<Function *, 2> visited_nodes = {}; bool ambiguous = false; Function *end; ModuleList &images; - Target ⌖ ExecutionContext &context; - DFS(Function *end, ModuleList &images, Target &target, - ExecutionContext &context) - : end(end), images(images), target(target), context(context) {} + DFS(Function *end, ModuleList &images, ExecutionContext &context) + : end(end), images(images), context(context) {} - void search(CallEdge &first_edge, Function &first_callee, - CallSequence &path) { - dfs(first_edge, first_callee); + void search(Function &first_callee, std::vector<Function *> &path) { + dfs(first_callee); if (!ambiguous) path = std::move(solution_path); } - void dfs(CallEdge ¤t_edge, Function &callee) { + void dfs(Function &callee) { // Found a path to the target function. if (&callee == end) { if (solution_path.empty()) @@ -319,16 +312,13 @@ static void FindInterveningFrames(Function &begin, Function &end, } // Search the calls made from this callee. - active_path.emplace_back(&callee, LLDB_INVALID_ADDRESS); + active_path.push_back(&callee); for (const auto &edge : callee.GetTailCallingEdges()) { Function *next_callee = edge->GetCallee(images, context); if (!next_callee) continue; - addr_t tail_call_pc = edge->GetCallInstPC(callee, target); - active_path.back().second = tail_call_pc; - - dfs(*edge, *next_callee); + dfs(*next_callee); if (ambiguous) return; } @@ -336,7 +326,7 @@ static void FindInterveningFrames(Function &begin, Function &end, } }; - DFS(&end, images, target, exe_ctx).search(*first_edge, *first_callee, path); + DFS(&end, images, exe_ctx).search(*first_callee, path); } /// Given that \p next_frame will be appended to the frame list, synthesize @@ -389,7 +379,7 @@ void StackFrameList::SynthesizeTailCallFrames(StackFrame &next_frame) { // Try to find the unique sequence of (tail) calls which led from next_frame // to prev_frame. - CallSequence path; + std::vector<Function *> path; addr_t return_pc = next_reg_ctx_sp->GetPC(); Target &target = *target_sp.get(); ModuleList &images = next_frame.CalculateTarget()->GetImages(); @@ -399,13 +389,13 @@ void StackFrameList::SynthesizeTailCallFrames(StackFrame &next_frame) { path, images, log); // Push synthetic tail call frames. - for (auto calleeInfo : llvm::reverse(path)) { - Function *callee = calleeInfo.first; + for (Function *callee : llvm::reverse(path)) { uint32_t frame_idx = m_frames.size(); uint32_t concrete_frame_idx = next_frame.GetConcreteFrameIndex(); addr_t cfa = LLDB_INVALID_ADDRESS; bool cfa_is_valid = false; - addr_t pc = calleeInfo.second; + addr_t pc = + callee->GetAddressRange().GetBaseAddress().GetLoadAddress(&target); constexpr bool behaves_like_zeroth_frame = false; SymbolContext sc; callee->CalculateSymbolContext(&sc); @@ -414,7 +404,7 @@ void StackFrameList::SynthesizeTailCallFrames(StackFrame &next_frame) { cfa_is_valid, pc, StackFrame::Kind::Artificial, behaves_like_zeroth_frame, &sc); m_frames.push_back(synth_frame); - LLDB_LOG(log, "Pushed frame {0} at {1:x}", callee->GetDisplayName(), pc); + LLDB_LOG(log, "Pushed frame {0}", callee->GetDisplayName()); } // If any frames were created, adjust next_frame's index. diff --git a/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp b/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp index 559f8a6d66aa..c9ab74074f90 100644 --- a/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp +++ b/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/main.cpp @@ -3,28 +3,19 @@ volatile int x; void __attribute__((noinline)) sink() { x++; //% self.filecheck("bt", "main.cpp", "-implicit-check-not=artificial") // CHECK: frame #0: 0x{{[0-9a-f]+}} a.out`sink() at main.cpp:[[@LINE-1]]:4 [opt] - // CHECK-NEXT: frame #1: 0x{{[0-9a-f]+}} a.out`func3() at main.cpp:14:3 [opt] [artificial] - // CHECK-NEXT: frame #2: 0x{{[0-9a-f]+}} a.out`func2() {{.*}} [opt] - // CHECK-NEXT: frame #3: 0x{{[0-9a-f]+}} a.out`func1() at main.cpp:23:3 [opt] [artificial] + // CHECK-NEXT: frame #1: 0x{{[0-9a-f]+}} a.out`func3{{.*}} [opt] [artificial] + // CHECK-NEXT: frame #2: 0x{{[0-9a-f]+}} a.out`func2{{.*}} [opt] + // CHECK-NEXT: frame #3: 0x{{[0-9a-f]+}} a.out`func1{{.*}} [opt] [artificial] // CHECK-NEXT: frame #4: 0x{{[0-9a-f]+}} a.out`main{{.*}} [opt] } -void __attribute__((noinline)) func3() { - x++; - sink(); /* tail */ -} +void __attribute__((noinline)) func3() { sink(); /* tail */ } -void __attribute__((disable_tail_calls, noinline)) func2() { - func3(); /* regular */ -} +void __attribute__((disable_tail_calls, noinline)) func2() { func3(); /* regular */ } -void __attribute__((noinline)) func1() { - x++; - func2(); /* tail */ -} +void __attribute__((noinline)) func1() { func2(); /* tail */ } int __attribute__((disable_tail_calls)) main() { - // DEBUG: self.runCmd("log enable lldb step -f /tmp/lldbstep.log") func1(); /* regular */ return 0; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
