[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk updated this revision to Diff 166534.
vsk added a comment.
As discussed offline, print out a note when stepping out of a frame with
artificial ancestors explaining that they were skipped while stepping out. See
the added test: functionalities/tail_call_frames/thread_step_out_message.
https://reviews.llvm.org/D50478
Files:
lldb/include/lldb/API/SBFrame.h
lldb/include/lldb/Core/FormatEntity.h
lldb/include/lldb/Symbol/Block.h
lldb/include/lldb/Symbol/Function.h
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Target/StackFrame.h
lldb/include/lldb/Target/StackFrameList.h
lldb/include/lldb/Target/ThreadPlanStepOut.h
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/TestAmbiguousTailCallSeq1.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/TestAmbiguousTailCallSeq2.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_message/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_message/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
lldb/scripts/interface/SBFrame.i
lldb/source/API/SBFrame.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/FormatEntity.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/source/Symbol/Block.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Target/StackFrame.cpp
lldb/source/Target/StackFrameList.cpp
lldb/source/Target/ThreadPlanStepOut.cpp
Index: lldb/source/Target/ThreadPlanStepOut.cpp
===
--- lldb/source/Target/ThreadPlanStepOut.cpp
+++ lldb/source/Target/ThreadPlanStepOut.cpp
@@ -48,26 +48,44 @@
m_return_addr(LLDB_INVALID_ADDRESS), m_stop_others(stop_others),
m_immediate_step_from_function(nullptr),
m_calculate_return_value(gather_return_value) {
+ Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
SetFlagsToDefault();
SetupAvoidNoDebug(step_out_avoids_code_without_debug_info);
m_step_from_insn = m_thread.GetRegisterContext()->GetPC(0);
- StackFrameSP return_frame_sp(m_thread.GetStackFrameAtIndex(frame_idx + 1));
+ uint32_t return_frame_index = frame_idx + 1;
+ StackFrameSP return_frame_sp(
+ m_thread.GetStackFrameAtIndex(return_frame_index));
StackFrameSP immediate_return_from_sp(
m_thread.GetStackFrameAtIndex(frame_idx));
if (!return_frame_sp || !immediate_return_from_sp)
return; // we can't do anything here. ValidatePlan() wil
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk planned changes to this revision. vsk added a comment. While testing this out, I found an issue with CallEdge::GetReturnPCAddress. Getting the load address there adds an unnecessary slide to the PC. I'll try to have that worked out next week. https://reviews.llvm.org/D50478 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52516: [lldbinline] Set directory attribute on test-specific classes
vsk created this revision. vsk added reviewers: labath, jingham, JDevlieghere. Herald added subscribers: eraman, aprantl. Set the "mydir" attribute of an inline test on the test-specific class, instead of on the base InlineTest class. This makes it possible to run dotest.py on a directory containing inline tests. This wasn't really possible prior to this patch, because what would happen is that one test would just run over and over again, even though the test infrastructure would claim that different tests were being run. Example: The test infrastructure claimed that all of these different tests were passing, which couldn't be true -- $ ./bin/lldb-dotest /Users/vsk/src/tailcall/lldb/test/testcases/functionalities/tail_call_frames/ -G dwarf -t 2>&1 | grep PASS PASS: LLDB (/Users/vsk/src/builds/tailcall-RA/bin/clang-8-x86_64) :: test_dwarf (lldbsuite.test.lldbtest.TestDisambiguateTailCallSeq) PASS: LLDB (/Users/vsk/src/builds/tailcall-RA/bin/clang-8-x86_64) :: test_dwarf (lldbsuite.test.lldbtest.TestArtificialFrameStepOutMessage) PASS: LLDB (/Users/vsk/src/builds/tailcall-RA/bin/clang-8-x86_64) :: test_dwarf (lldbsuite.test.lldbtest.TestAmbiguousTailCallSeq1) PASS: LLDB (/Users/vsk/src/builds/tailcall-RA/bin/clang-8-x86_64) :: test_dwarf (lldbsuite.test.lldbtest.TestDisambiguatePathsToCommonSink) PASS: LLDB (/Users/vsk/src/builds/tailcall-RA/bin/clang-8-x86_64) :: test_dwarf (lldbsuite.test.lldbtest.TestDisambiguateCallSite) PASS: LLDB (/Users/vsk/src/builds/tailcall-RA/bin/clang-8-x86_64) :: test_dwarf (lldbsuite.test.lldbtest.TestUnambiguousTailCalls) PASS: LLDB (/Users/vsk/src/builds/tailcall-RA/bin/clang-8-x86_64) :: test_dwarf (lldbsuite.test.lldbtest.TestAmbiguousTailCallSeq2) RESULT: PASSED (7 passes, 0 failures, 0 errors, 24 skipped, 0 expected failures, 0 unexpected successes) ... because it wasn't even looking at some of these tests: $ ./bin/lldb-dotest /Users/vsk/src/tailcall/lldb/test/testcases/functionalities/tail_call_frames/ -G dwarf -t 2>&1 | grep "Change dir" Change dir to: /Users/vsk/src/tailcall/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2 Change dir to: /Users/vsk/src/tailcall/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2 Change dir to: /Users/vsk/src/tailcall/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2 Change dir to: /Users/vsk/src/tailcall/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support Change dir to: /Users/vsk/src/tailcall/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2 Change dir to: /Users/vsk/src/tailcall/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return Change dir to: /Users/vsk/src/tailcall/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2 Change dir to: /Users/vsk/src/tailcall/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2 Change dir to: /Users/vsk/src/tailcall/lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2 E.g it was only building one of them: $ ls lldb-test-build.noindex/functionalities/tail_call_frames/ ambiguous_tail_call_seq2 https://reviews.llvm.org/D52516 Files: lldb/packages/Python/lldbsuite/test/lldbinline.py Index: lldb/packages/Python/lldbsuite/test/lldbinline.py === --- lldb/packages/Python/lldbsuite/test/lldbinline.py +++ lldb/packages/Python/lldbsuite/test/lldbinline.py @@ -195,7 +195,6 @@ # Derive the test name from the current file name file_basename = os.path.basename(__file) -InlineTest.mydir = TestBase.compute_mydir(__file) test_name, _ = os.path.splitext(file_basename) @@ -209,4 +208,5 @@ # Keep track of the original test filename so we report it # correctly in test results. test_class.test_filename = __file +test_class.mydir = TestBase.compute_mydir(__file) return test_class Index: lldb/packages/Python/lldbsuite/test/lldbinline.py === --- lldb/packages/Python/lldbsuite/test/lldbinline.py +++ lldb/packages/Python/lldbsuite/test/lldbinline.py @@ -195,7 +195,6 @@ # Derive the test name from the current file name file_basename = os.path.basename(__file) -InlineTest.mydir = TestBase.compute_mydir(__file) test_name, _ = os.path.splitext(file_basename) @@ -209,4 +208,5 @@ # Keep track of the original test filename so we report it # correctly in test results. test_class.test_filename = __file +test_class.mydir = TestBase.compute_mydir(__file) return test_class ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk updated this revision to Diff 167038. vsk added a comment. The bug I thought I saw in CallEdge::GetReturnPCAddress turned out to be an issue I introduced in dsymutil. It's not correct for dsymutil to relocate the return PC value in TAG_call_site entries, because the correct section slide offset can't be applied until we have a running process (and things like dylibs are mapped in). Other changes: - Add a test which requires both inline frame and tail call frame support. - Require a call edge’s return PC attribute to match the register context’s PC exactly. Otherwise, we report incorrect frames. - Improve logging and add more tests. https://reviews.llvm.org/D50478 Files: lldb/include/lldb/API/SBFrame.h lldb/include/lldb/Core/FormatEntity.h lldb/include/lldb/Symbol/Block.h lldb/include/lldb/Symbol/Function.h lldb/include/lldb/Symbol/SymbolFile.h lldb/include/lldb/Target/StackFrame.h lldb/include/lldb/Target/StackFrameList.h lldb/include/lldb/Target/ThreadPlanStepOut.h lldb/packages/Python/lldbsuite/test/decorators.py lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/Makefile lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/TestAmbiguousTailCallSeq1.py lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/main.cpp lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/Makefile lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/TestAmbiguousTailCallSeq2.py lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/main.cpp lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/Makefile lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/main.cpp lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/Makefile lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/Makefile lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/main.cpp lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/inlining_and_tail_calls/Makefile lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/inlining_and_tail_calls/main.cpp lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/Makefile lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/main.cpp lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_message/Makefile lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_message/main.cpp lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/Makefile lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/main.cpp lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/Makefile lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/main.cpp lldb/scripts/interface/SBFrame.i lldb/source/API/SBFrame.cpp lldb/source/Core/Debugger.cpp lldb/source/Core/FormatEntity.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h lldb/source/Symbol/Block.cpp lldb/source/Symbol/Function.cpp lldb/source/Target/StackFrame.cpp lldb/source/Target/StackFrameList.cpp lldb/source/Target/ThreadPlanS
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk updated this revision to Diff 167148.
vsk marked an inline comment as done.
vsk added a comment.
- Use std::make_shared(...), instead of StackFrameSP(new ...).
https://reviews.llvm.org/D50478
Files:
lldb/include/lldb/API/SBFrame.h
lldb/include/lldb/Core/FormatEntity.h
lldb/include/lldb/Symbol/Block.h
lldb/include/lldb/Symbol/Function.h
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Target/StackFrame.h
lldb/include/lldb/Target/StackFrameList.h
lldb/include/lldb/Target/ThreadPlanStepOut.h
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/TestAmbiguousTailCallSeq1.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/TestAmbiguousTailCallSeq2.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/inlining_and_tail_calls/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/inlining_and_tail_calls/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_message/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_message/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
lldb/scripts/interface/SBFrame.i
lldb/source/API/SBFrame.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/FormatEntity.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/source/Symbol/Block.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Target/StackFrame.cpp
lldb/source/Target/StackFrameList.cpp
lldb/source/Target/ThreadPlanStepOut.cpp
Index: lldb/source/Target/ThreadPlanStepOut.cpp
===
--- lldb/source/Target/ThreadPlanStepOut.cpp
+++ lldb/source/Target/ThreadPlanStepOut.cpp
@@ -48,26 +48,44 @@
m_return_addr(LLDB_INVALID_ADDRESS), m_stop_others(stop_others),
m_immediate_step_from_function(nullptr),
m_calculate_return_value(gather_return_value) {
+ Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk added inline comments. Comment at: lldb/source/Target/StackFrameList.cpp:331 +dfs(next_callee); +if (ambiguous) + return; aprantl wrote: > On what path can this happen? Aren't all paths that set `ambiguous=true` > returning immediately? If a recursive call to dfs() sets `ambiguous = true` and returns early, we also want its caller to return early instead of visiting any additional edges. This is a small optimization: the code is correct without the early return. https://reviews.llvm.org/D50478 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap
vsk added inline comments.
Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:431
const size_t num_ranges =
-die->GetAttributeAddressRanges(dwarf, this, ranges, false);
+die->GetAttributeAddressRanges(dwarf, this, ranges, check_hi_lo_pc);
if (num_ranges > 0) {
sgraenitz wrote:
> clayborg wrote:
> > sgraenitz wrote:
> > > @JDevlieghere Thanks for your feedback!
> > >
> > > @everyone:
> > > This parameter is, actually, one of the key questions in this review: If
> > > there is no DW_AT_range attribute for the compile unit, can we safely
> > > fall back to DW_AT_low/high_pc?
> > I have seen DWARF where the DW_AT_ranges is incorrect and I have seen cases
> > where the low pc is just set to zero and the high pc is set to the max
> > address. So you might have many overlapping ranges between different CUs in
> > the latter case. We might need to be more vigilant with high/low pc values
> > though and that was the reason that we previously ignored high/low pc
> > values. If the stuff is missing, it should index the DWARF and generate the
> > address ranges table manually right? What was the reasoning behind this
> > change?
> I had a closer look at the individual fallbacks and you are right. After
> fixing `SymbolFileDWARF::ParseCompileUnit()` we have the correct
> `CompileUnit` pointers in `DWARFUnit::m_user_data` for all CUs. Thus, the
> below `die->BuildAddressRangeTable(dwarf, this, debug_aranges);` now
> correctly constructs the ranges! Great, with this we can keep
> `check_hi_lo_pc=false`.
>
> I didn't notice this while testing, because of the following issue that kept
> my tests failing: 2 of my LTO object's CUs have no code remaining after
> optimization, so we step into the next fallback assuming 'a line tables only
> situation' and eventually call `AddOSOARanges()`, which adds to
> `debug_aranges` all entries of the CU's FileRangeMap with a zero offset.
>
> Thus, my `debug_aranges` had 28000 entries too much and most requests would
> continue to return `0` as before. For now I commented out the call below.
>
> **What do you think is a good way to avoid this for empty CUs?
> IIUC they should have no DW_AT_low/high_pc right? :-D**
+1 to @clayborg's suggestion here that we either use AT_ranges, or build up the
ranges by parsing the low/high pc's in subprograms. ISTM that it'd be
acceptable to implement the fallback path as a follow-up, provided that doing
so isn't a regression.
Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:489
+ } //else
+//debug_map_sym_file->AddOSOARanges(dwarf, debug_aranges);
}
Could you leave an in-source comment explaining why this is commented out?
https://reviews.llvm.org/D52375
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap
vsk added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp:1531 + if (m_compile_unit_infos.size() > 1) +return 0; + sgraenitz wrote: > sgraenitz wrote: > > Skipping AddOSOARanges() here. > > Could you leave an in-source comment explaining why this is commented out? > > @vsk AddOSOARanges() was commented out only temporarily. I reverted it in the > latest patch and added these lines with a comment. Is that what you were > asking for? Ah sure sure, sorry, I missed the broader context of the change. https://reviews.llvm.org/D52375 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk added a comment. Friendly ping. https://reviews.llvm.org/D50478 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52689: [LLDB] - Add support for DW_FORM_implicit_const.
vsk added a comment. Could you describe how the test exercises DW_FORM_implicit_const support? It's not immediately clear to me. https://reviews.llvm.org/D52689 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk updated this revision to Diff 168155.
vsk marked an inline comment as done.
vsk added a comment.
- Address feedback from Adrian.
https://reviews.llvm.org/D50478
Files:
lldb/include/lldb/API/SBFrame.h
lldb/include/lldb/Core/FormatEntity.h
lldb/include/lldb/Symbol/Block.h
lldb/include/lldb/Symbol/Function.h
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Target/StackFrame.h
lldb/include/lldb/Target/StackFrameList.h
lldb/include/lldb/Target/ThreadPlanStepOut.h
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/TestAmbiguousTailCallSeq1.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/TestAmbiguousTailCallSeq2.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/inlining_and_tail_calls/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/inlining_and_tail_calls/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_message/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_message/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
lldb/scripts/interface/SBFrame.i
lldb/source/API/SBFrame.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/FormatEntity.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/source/Symbol/Block.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Target/StackFrame.cpp
lldb/source/Target/StackFrameList.cpp
lldb/source/Target/ThreadPlanStepOut.cpp
Index: lldb/source/Target/ThreadPlanStepOut.cpp
===
--- lldb/source/Target/ThreadPlanStepOut.cpp
+++ lldb/source/Target/ThreadPlanStepOut.cpp
@@ -48,26 +48,44 @@
m_return_addr(LLDB_INVALID_ADDRESS), m_stop_others(stop_others),
m_immediate_step_from_function(nullptr),
m_calculate_return_value(gather_return_value) {
+ Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
SetFlagsToDefault();
SetupAv
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk added inline comments. Comment at: lldb/include/lldb/Symbol/Function.h:304 +public: + CallEdge(const char *mangled_name, lldb::addr_t return_pc); + aprantl wrote: > Does this also work for C functions? If yes, would `symbol_name` be a more > accurate description? > > Is this pointer globally unique in the program, or can two mangled names > appear in a legal C program that don't refer to the same function? Yes, C functions are handled. I'll use "symbol_name", hopefully that makes it clearer. Comment at: lldb/include/lldb/Symbol/Function.h:304 +public: + CallEdge(const char *mangled_name, lldb::addr_t return_pc); + vsk wrote: > aprantl wrote: > > Does this also work for C functions? If yes, would `symbol_name` be a more > > accurate description? > > > > Is this pointer globally unique in the program, or can two mangled names > > appear in a legal C program that don't refer to the same function? > Yes, C functions are handled. I'll use "symbol_name", hopefully that makes it > clearer. Great question. No, a symbol name is not necessarily globally unique. Prior to C11, the one-definition-rule doesn't apply. Even with the ODR, a private symbol in a shared library may have the same name as a strong definition in the main executable. I haven't tried to disambiguate ODR conflicts in this patch. What happens when a conflict occurs is: `FindFunctionSymbols` prioritizes the symbol in the main executable, and if the call edge's return PC doesn't exactly match what's in the register state we decline to create any artificial frames. I.e. in the presence of ODR conflicts, we only present artificial frames when we're 100% sure they are accurate. Handling ODR conflicts is a 'to do', though. I'll make an explicit note of that here. Comment at: lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/TestAmbiguousTailCallSeq2.py:5 +lldbinline.MakeInlineTest(__file__, globals(), +[decorators.skipUnlessHasCallSiteInfo]) aprantl wrote: > This test should be restricted to only run with clang as the compiler or the > -glldb flag won't work. > > I see. It is implicit in in skipUnlessHasCallSiteInfo — perhaps we should > just generally add -glldb to CFLAGS? It's not a bug deal. Yes, skipUnlessHasCallSiteInfo requires clang for now. I don't think we can add -glldb to a higher-level CFLAGS variable without breaking compiling with gcc? https://reviews.llvm.org/D50478 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk added inline comments. Comment at: lldb/include/lldb/Symbol/Function.h:304 +public: + CallEdge(const char *mangled_name, lldb::addr_t return_pc); + aprantl wrote: > vsk wrote: > > vsk wrote: > > > aprantl wrote: > > > > Does this also work for C functions? If yes, would `symbol_name` be a > > > > more accurate description? > > > > > > > > Is this pointer globally unique in the program, or can two mangled > > > > names appear in a legal C program that don't refer to the same function? > > > Yes, C functions are handled. I'll use "symbol_name", hopefully that > > > makes it clearer. > > Great question. No, a symbol name is not necessarily globally unique. Prior > > to C11, the one-definition-rule doesn't apply. Even with the ODR, a private > > symbol in a shared library may have the same name as a strong definition in > > the main executable. > > > > I haven't tried to disambiguate ODR conflicts in this patch. What happens > > when a conflict occurs is: `FindFunctionSymbols` prioritizes the symbol in > > the main executable, and if the call edge's return PC doesn't exactly match > > what's in the register state we decline to create any artificial frames. > > I.e. in the presence of ODR conflicts, we only present artificial frames > > when we're 100% sure they are accurate. > > > > Handling ODR conflicts is a 'to do', though. I'll make an explicit note of > > that here. > Thanks! It might be interesting to grep through an XNU dsym to see just how > common conflicts are in typical C code. Of a total of ~27,000 function names in xnu, 140 names were duplicates. In every case I spot-checked, a subprogram with a duplicate AT_name had a unique AT_linkage_name. So, well under 0.5% in that project. https://reviews.llvm.org/D50478 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51874: Fix buildbot regression by rL339929: NameError: global name 'test_directory' is not defined
vsk added inline comments. Comment at: packages/Python/lldbsuite/test/dosep.py:1693 for core in cores: dst = core.replace(test_directory, "")[1:] dst = dst.replace(os.path.sep, "-") Instead of redefining test_directory, please use 'test_subdir' in the call to core.replace. That leaves the logic to find the right path in configuration.get_absolute_path_to_root_test_dir. Repository: rLLDB LLDB https://reviews.llvm.org/D51874 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52678: DWARFExpression: Resolve file addresses in the linked module
vsk added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/target_var/globals.ll:1 +source_filename = "globals.c" +target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" Should we check in bitcode instead? That might make it easier to avoid breakage when the textual IR format changes. Repository: rLLDB LLDB https://reviews.llvm.org/D52678 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52678: DWARFExpression: Resolve file addresses in the linked module
vsk added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/target_var/globals.ll:1 +source_filename = "globals.c" +target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" davide wrote: > vsk wrote: > > Should we check in bitcode instead? That might make it easier to avoid > > breakage when the textual IR format changes. > But also will be more painful to regenerate in case, e.g. add a verifier > check where this breaks. > I think it's a tradeoff anyway, I and Adrian discussed this briefly and > agreed this is a decent middle ground (on one extreme, one might check ASM > directly, but that seems even harder to handle). > > This is honestly based on my experience of having to regenerate broken > bitcode files, happy to be convinced otherwise if you have arguments :) (per an off-list discussion) I'm slightly biased towards preventing build breaks, but this sounds reasonable. A third option might be to check in both the bitcode (which is compiled) and the IR (which serves as a reference for regenerating the bitcode); that could address all/most of the concerns. Repository: rLLDB LLDB https://reviews.llvm.org/D52678 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50155: Delete MacOSXFrameBackchain unwind logic (NFC)
vsk added a comment. Ping? https://reviews.llvm.org/D50155 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk added a comment. In https://reviews.llvm.org/D50478#1262710, @stella.stamenova wrote: > Unfortunately, the bots are broken because of the FileCheck issue, so I can't > confirm with them, but I see a number of these tests fail in our local > testing. Some fail on both Windows and Linux and some just fail on Linux. > Here are the failing tests: > > Linux: > lldb-Suite :: > functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py > lldb-Suite :: > functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py > lldb-Suite :: > functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py > lldb-Suite :: > functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py > lldb-Suite :: > functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py > lldb-Suite :: > functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py > lldb-Suite :: > functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py > > Windows: > lldb-Suite :: > functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py > lldb-Suite :: > functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py > > > Let me know what you need to investigate. Strange, I didn't get any bot failure notifications in the days after this landed. Could you share the output from the failing tests? Repository: rLLDB LLDB https://reviews.llvm.org/D50478 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error
vsk created this revision.
vsk added reviewers: stella.stamenova, zturner.
This allows bots which haven't updated to pass in --filecheck to dotest.py to
run more tests. FileCheck-dependent tests will continue to fail.
https://reviews.llvm.org/D53175
Files:
lldb/packages/Python/lldbsuite/test/dotest.py
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -311,8 +311,8 @@
# The CMake build passes in a path to a working FileCheck binary.
configuration.filecheck = os.path.abspath(args.filecheck)
else:
-logging.error('No valid FileCheck executable; aborting...')
-sys.exit(-1)
+logging.warning('No valid FileCheck executable; some tests may
fail...')
+logging.warning('(Pass in a --filecheck argument to dotest.py)')
if args.channels:
lldbtest_config.channels = args.channels
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -311,8 +311,8 @@
# The CMake build passes in a path to a working FileCheck binary.
configuration.filecheck = os.path.abspath(args.filecheck)
else:
-logging.error('No valid FileCheck executable; aborting...')
-sys.exit(-1)
+logging.warning('No valid FileCheck executable; some tests may fail...')
+logging.warning('(Pass in a --filecheck argument to dotest.py)')
if args.channels:
lldbtest_config.channels = args.channels
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53175: [dotest] Make a missing FileCheck binary a warning, not an error
vsk updated this revision to Diff 169471.
vsk added a comment.
- Address comments from @stella.stamenova
https://reviews.llvm.org/D53175
Files:
lldb/packages/Python/lldbsuite/test/configuration.py
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/lldbtest.py
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2237,6 +2237,8 @@
# Run FileCheck.
filecheck_bin = configuration.get_filecheck_path()
+if not filecheck_bin:
+self.assertTrue(False, "No valid FileCheck executable specified")
filecheck_args = [filecheck_bin, check_file_abs]
if filecheck_options:
filecheck_args.append(filecheck_options)
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -308,11 +308,15 @@
'xcrun -find -toolchain default dsymutil')
if args.filecheck:
-# The CMake build passes in a path to a working FileCheck binary.
+# The lldb-dotest script produced by the CMake build passes in a path
+# to a working FileCheck binary. So does one specific Xcode project
+# target. However, when invoking dotest.py directly, a valid
--filecheck
+# option needs to be given.
configuration.filecheck = os.path.abspath(args.filecheck)
-else:
-logging.error('No valid FileCheck executable; aborting...')
-sys.exit(-1)
+
+if not configuration.get_filecheck_path():
+logging.warning('No valid FileCheck executable; some tests may
fail...')
+logging.warning('(Double-check the --filecheck argument to dotest.py)')
if args.channels:
lldbtest_config.channels = args.channels
Index: lldb/packages/Python/lldbsuite/test/configuration.py
===
--- lldb/packages/Python/lldbsuite/test/configuration.py
+++ lldb/packages/Python/lldbsuite/test/configuration.py
@@ -188,5 +188,5 @@
"""
Get the path to the FileCheck testing tool.
"""
-assert os.path.lexists(filecheck)
-return filecheck
+if os.path.lexists(filecheck):
+return filecheck
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2237,6 +2237,8 @@
# Run FileCheck.
filecheck_bin = configuration.get_filecheck_path()
+if not filecheck_bin:
+self.assertTrue(False, "No valid FileCheck executable specified")
filecheck_args = [filecheck_bin, check_file_abs]
if filecheck_options:
filecheck_args.append(filecheck_options)
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -308,11 +308,15 @@
'xcrun -find -toolchain default dsymutil')
if args.filecheck:
-# The CMake build passes in a path to a working FileCheck binary.
+# The lldb-dotest script produced by the CMake build passes in a path
+# to a working FileCheck binary. So does one specific Xcode project
+# target. However, when invoking dotest.py directly, a valid --filecheck
+# option needs to be given.
configuration.filecheck = os.path.abspath(args.filecheck)
-else:
-logging.error('No valid FileCheck executable; aborting...')
-sys.exit(-1)
+
+if not configuration.get_filecheck_path():
+logging.warning('No valid FileCheck executable; some tests may fail...')
+logging.warning('(Double-check the --filecheck argument to dotest.py)')
if args.channels:
lldbtest_config.channels = args.channels
Index: lldb/packages/Python/lldbsuite/test/configuration.py
===
--- lldb/packages/Python/lldbsuite/test/configuration.py
+++ lldb/packages/Python/lldbsuite/test/configuration.py
@@ -188,5 +188,5 @@
"""
Get the path to the FileCheck testing tool.
"""
-assert os.path.lexists(filecheck)
-return filecheck
+if os.path.lexists(filecheck):
+return filecheck
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53415: [lldbsuite, windows] Disable two tail call frames tests that fail on Windows
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm. Repository: rLLDB LLDB https://reviews.llvm.org/D53415 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53731: [NativePDB] Add the ability to display global variables
vsk added a comment.
In https://reviews.llvm.org/D53731#1276732, @zturner wrote:
> In https://reviews.llvm.org/D53731#1276660, @jingham wrote:
>
> > Could you also use Vedant's new FileCheck dotest test class? That should
> > allow you to write the tests exactly as you are, but use the dotest
> > mechanism to build and run the example.
>
>
> Adding Vedant here. My understanding is that the work he did there is like
> the inverse of what I'm doing. It allows you to use FileCheck from inside of
> dotest tests, but I was trying to bypass dotest here. I don't necessarily
> think "dotest for all things" should be an explicit goal (i actually think
> long term we should move away from it, but that's for another day). Aside
> from that though, I don't think this particular test needs any of the
> functionality that dotest offers. It offers building in multiple
> configurations, but we can get that from this test by just specifying
> mulitple run lines (I'm testing this out locally and it seems like I can get
> it to work). Eventually, using some kind of configuration / builder type
> system like I brainstormed in the review thread I linked previously, I think
> we could have all the advantages of dotest's builder even with this style of
> test.
>
> FWIW, I was also under the impression that Vedant's solution with FileCheck
> in dotest was only intended as sort of an immediate solution to a problem,
> but not necessary the long term desired end-state. (I could be wrong about
> this though).
The tests as-written seem to exercise the new functionality. I think it'd be
fine to use the FileCheck-in-{dotest,lldbinline} support if you need to set a
breakpoint, run a command, and then validate its output (though it looks like
you don't)
https://reviews.llvm.org/D53731
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54385: Remove comments after header includes.
vsk accepted this revision as: vsk. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm. Repository: rLLDB LLDB https://reviews.llvm.org/D54385 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D54731: [lit] Enable the use of custom user-defined lit commands
vsk added a subscriber: filcab. vsk added a comment. For compiling/linking, I think we can get by using lit substitutions to fill in platform-specific options? iOS testing for Swift is done this way (both on-device and simulator), as is testing for the profiling runtime. Dan and @filcab are more active in the area of sanitizer runtime testing, so they may have more informed opinions to share about how well that model works. I’m not sure what we’d use custom lit commands for beyond compiling tests. https://reviews.llvm.org/D54731 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D55574: Remove else statements after returns
vsk added a comment. + 1 to this. If there's a tidy plugin for misleading indention, that might address some of Adrian's concerns. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55574/new/ https://reviews.llvm.org/D55574 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D55761: lldb-test ir-memory-map: Use IntervalMap::contains
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, lgtm! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55761/new/ https://reviews.llvm.org/D55761 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)
vsk added a comment.
Looks like a nice/reasonable cleanup, thanks!
Based on the coverage report
(https://teemperor.de/lldb-coverage/coverage/Users/vsk/src/llvm.org-lldbsan/llvm/tools/lldb/source/Breakpoint/BreakpointList.cpp.html#L217)
and benchmarks (https://teemperor.de/lldb-bench/static.html)
GetBreakpointAtIndex doesn't looks that hot, so I'm not sure it's worth
changing / accidentally regressing.
Comment at: source/Breakpoint/BreakpointList.cpp:196
const BreakpointSP BreakpointList::GetBreakpointAtIndex(size_t i) const {
std::lock_guard guard(m_mutex);
Just call the non-const overload and const_cast?
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56425/new/
https://reviews.llvm.org/D56425
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47508: [lldb-test] Add a testing harness for the JIT's IRMemoryMap
vsk created this revision.
vsk added reviewers: jingham, davide, labath, zturner.
This teaches lldb-test how to launch a process, set up an IRMemoryMap,
and issue memory allocations in the target process through the map. This
makes it possible to test IRMemoryMap in a targeted way.
The main motivation for testing IRMemoryMap is to uncover bugs. I've
noticed two so far. The first bug is that IRMemoryMap::Malloc performs
an adjustment on the pointer returned from Process::AllocateMemory (for
alignment purposes) which ultimately allows overlapping memory regions
to be created. The second bug is that after most of the address space on
the host side is exhausted, Malloc may return the same address multiple
times. These bugs (and hopefully more!) can be uncovered and tested for
with targeted lldb-test commands.
At an even higher level, the motivation for addressing bugs in
IRMemoryMap::Malloc is that they can lead to strange user-visible
failures (e.g, variables assume the wrong value during expression
evaluation, or the debugger crashes). See my third comment on this
swift-lldb PR for an example:
https://github.com/apple/swift-lldb/pull/652
I hope lldb-test is the right place to test IRMemoryMap. Setting up a
gtest-style unit test proved too cumbersome (you need to recreate or
mock way too much debugger state), as did writing end-to-end tests (it's
hard to write a test that actually hits a buggy path).
With lldb-test, it's easy to read/generate the test input and parse the
test output. I'll attach a simple "fuzz" tester which generates failing test
cases with ease.
https://reviews.llvm.org/D47508
Files:
lit/Expr/TestIRMemoryMap.test
source/Target/Process.cpp
tools/lldb-test/lldb-test.cpp
Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -15,20 +15,25 @@
#include "lldb/Core/Debugger.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/Section.h"
+#include "lldb/Expression/IRMemoryMap.h"
#include "lldb/Initialization/SystemLifetimeManager.h"
#include "lldb/Interpreter/CommandInterpreter.h"
#include "lldb/Interpreter/CommandReturnObject.h"
#include "lldb/Symbol/ClangASTContext.h"
#include "lldb/Symbol/ClangASTImporter.h"
#include "lldb/Symbol/SymbolVendor.h"
#include "lldb/Symbol/TypeList.h"
#include "lldb/Symbol/VariableList.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
#include "lldb/Utility/CleanUp.h"
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/StreamString.h"
+#include "llvm/ADT/IntervalMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MathExtras.h"
#include "llvm/Support/ManagedStatic.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/PrettyStackTrace.h"
@@ -41,11 +46,18 @@
using namespace llvm;
namespace opts {
+
+TargetSP createTarget(Debugger &Dbg, const std::string &Filename);
+std::unique_ptr openFile(const std::string &Filename);
+
static cl::SubCommand BreakpointSubcommand("breakpoints",
"Test breakpoint resolution");
cl::SubCommand ModuleSubcommand("module-sections",
"Display LLDB Module Information");
cl::SubCommand SymbolsSubcommand("symbols", "Dump symbols for an object file");
+cl::SubCommand IRMemoryMapSubcommand("ir-memory-map", "Test IRMemoryMap");
+cl::opt Log("log", cl::desc("Path to a log file"), cl::init(""),
+ cl::sub(IRMemoryMapSubcommand));
namespace breakpoint {
static cl::opt Target(cl::Positional, cl::desc(""),
@@ -135,8 +147,47 @@
static int dumpSymbols(Debugger &Dbg);
}
+
+namespace irmemorymap {
+static cl::opt Target(cl::Positional, cl::desc(""),
+ cl::Required,
+ cl::sub(IRMemoryMapSubcommand));
+static cl::opt CommandFile(cl::Positional,
+cl::desc(""),
+cl::init("-"),
+cl::sub(IRMemoryMapSubcommand));
+using AddrIntervalMap =
+ IntervalMap>;
+bool evalMalloc(IRMemoryMap &IRMemMap, StringRef Line,
+AddrIntervalMap &AllocatedIntervals);
+int evaluateMemoryMapCommands(Debugger &Dbg);
+} // namespace irmemorymap
+
} // namespace opts
+TargetSP opts::createTarget(Debugger &Dbg, const std::string &Filename) {
+ TargetSP Target;
+ Status ST =
+ Dbg.GetTargetList().CreateTarget(Dbg, Filename, /*triple*/ "",
+ /*get_dependent_modules*/ false,
+ /*platform_options*/ nullptr, Target);
+ if (ST.Fail()) {
+errs() << formatv("Failed to create target '{0}: {1}\n", Filename, ST);
+exit(1);
+ }
+ return Target;
+}
+
+std::unique_ptr opts::openFile(const std::string &Filename) {
+ auto MB = MemoryBuffer::getFileOrSTDIN(Fi
[Lldb-commits] [PATCH] D47508: [lldb-test] Add a testing harness for the JIT's IRMemoryMap
vsk added inline comments.
Comment at: tools/lldb-test/lldb-test.cpp:503
+ uint8_t Alignment;
+ int Matches = sscanf(Line.data(), "malloc %lu %hhu", &Size, &Alignment);
+ if (Matches != 2)
labath wrote:
> is `Line` null-terminated here? Also a size_t arg should have a `%zu`
> modifier, but I am not sure if all msvc versions support that. It might be
> best to make the type uint64_t and then use SCNu64.
Yes, `Line` is null-terminated because `MemoryBuffer::getFileOrSTDIN` defaults
to adding a null terminator.
Comment at: tools/lldb-test/lldb-test.cpp:503
+ uint8_t Alignment;
+ int Matches = sscanf(Line.data(), "malloc %lu %hhu", &Size, &Alignment);
+ if (Matches != 2)
vsk wrote:
> labath wrote:
> > is `Line` null-terminated here? Also a size_t arg should have a `%zu`
> > modifier, but I am not sure if all msvc versions support that. It might be
> > best to make the type uint64_t and then use SCNu64.
> Yes, `Line` is null-terminated because `MemoryBuffer::getFileOrSTDIN`
> defaults to adding a null terminator.
LLVM currently requires MSVC >= 2015 Update 3 (see:
https://reviews.llvm.org/D47073), which supports %zu (see:
https://blogs.msdn.microsoft.com/vcblog/2014/06/03/visual-studio-14-ctp/#div-comment-77743).
I'll just use %zu.
Comment at: tools/lldb-test/lldb-test.cpp:536-542
+ bool Overlaps = AllocatedIntervals.lookup(Addr, false);
+ if (Size && !Overlaps)
+Overlaps = AllocatedIntervals.lookup(Addr + Size - 1, false);
+ if (Overlaps) {
+outs() << "Malloc error: overlapping allocation detected\n";
+exit(1);
+ }
labath wrote:
> It looks like this won't detect the case when a larger interval is placed on
> top of a smaller one (e.g. `0x1000-0x4000` and `0x2000-0x3000`).
Thanks for pointing this out. I wasn't sure how to do this efficiently. Taking
another look at things, it looks like IntervalMap surfaces iterators which can
be used to scan a range quickly. I'll try that out.
https://reviews.llvm.org/D47508
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47508: [lldb-test] Add a testing harness for the JIT's IRMemoryMap
vsk updated this revision to Diff 149159.
vsk edited the summary of this revision.
vsk added a comment.
- Use %zu, and improve detection of overlapping allocations.
https://reviews.llvm.org/D47508
Files:
lit/Expr/TestIRMemoryMap.test
source/Target/Process.cpp
tools/lldb-test/lldb-test.cpp
Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -15,37 +15,53 @@
#include "lldb/Core/Debugger.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/Section.h"
+#include "lldb/Expression/IRMemoryMap.h"
#include "lldb/Initialization/SystemLifetimeManager.h"
#include "lldb/Interpreter/CommandInterpreter.h"
#include "lldb/Interpreter/CommandReturnObject.h"
#include "lldb/Symbol/ClangASTContext.h"
#include "lldb/Symbol/ClangASTImporter.h"
#include "lldb/Symbol/SymbolVendor.h"
#include "lldb/Symbol/TypeList.h"
#include "lldb/Symbol/VariableList.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
#include "lldb/Utility/CleanUp.h"
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/StreamString.h"
+#include "llvm/ADT/IntervalMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MathExtras.h"
#include "llvm/Support/ManagedStatic.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/PrettyStackTrace.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/WithColor.h"
+#include
#include
using namespace lldb;
using namespace lldb_private;
using namespace llvm;
namespace opts {
+
static cl::SubCommand BreakpointSubcommand("breakpoints",
"Test breakpoint resolution");
cl::SubCommand ModuleSubcommand("module-sections",
"Display LLDB Module Information");
cl::SubCommand SymbolsSubcommand("symbols", "Dump symbols for an object file");
+cl::SubCommand IRMemoryMapSubcommand("ir-memory-map", "Test IRMemoryMap");
+cl::opt Log("log", cl::desc("Path to a log file"), cl::init(""),
+ cl::sub(IRMemoryMapSubcommand));
+
+/// Create a target using the file pointed to by \p Filename, or abort.
+TargetSP createTarget(Debugger &Dbg, const std::string &Filename);
+
+/// Read \p Filename into a null-terminated buffer, or abort.
+std::unique_ptr openFile(const std::string &Filename);
namespace breakpoint {
static cl::opt Target(cl::Positional, cl::desc(""),
@@ -135,8 +151,47 @@
static int dumpSymbols(Debugger &Dbg);
}
+
+namespace irmemorymap {
+static cl::opt Target(cl::Positional, cl::desc(""),
+ cl::Required,
+ cl::sub(IRMemoryMapSubcommand));
+static cl::opt CommandFile(cl::Positional,
+cl::desc(""),
+cl::init("-"),
+cl::sub(IRMemoryMapSubcommand));
+using AddrIntervalMap =
+ IntervalMap>;
+bool evalMalloc(IRMemoryMap &IRMemMap, StringRef Line,
+AddrIntervalMap &AllocatedIntervals);
+int evaluateMemoryMapCommands(Debugger &Dbg);
+} // namespace irmemorymap
+
} // namespace opts
+TargetSP opts::createTarget(Debugger &Dbg, const std::string &Filename) {
+ TargetSP Target;
+ Status ST =
+ Dbg.GetTargetList().CreateTarget(Dbg, Filename, /*triple*/ "",
+ /*get_dependent_modules*/ false,
+ /*platform_options*/ nullptr, Target);
+ if (ST.Fail()) {
+errs() << formatv("Failed to create target '{0}: {1}\n", Filename, ST);
+exit(1);
+ }
+ return Target;
+}
+
+std::unique_ptr opts::openFile(const std::string &Filename) {
+ auto MB = MemoryBuffer::getFileOrSTDIN(Filename);
+ if (!MB) {
+errs() << formatv("Could not open file '{0}: {1}\n", Filename,
+ MB.getError().message());
+exit(1);
+ }
+ return std::move(*MB);
+}
+
void opts::breakpoint::dumpState(const BreakpointList &List, LinePrinter &P) {
P.formatLine("{0} breakpoint{1}", List.GetSize(), plural(List.GetSize()));
if (List.GetSize() > 0)
@@ -177,7 +232,7 @@
switch (Cmd[0]) {
case '%':
if (Cmd.consume_front("%p") && (Cmd.empty() || !isalnum(Cmd[0]))) {
-OS << sys::path::parent_path(CommandFile);
+OS << sys::path::parent_path(breakpoint::CommandFile);
break;
}
// fall through
@@ -192,26 +247,11 @@
}
int opts::breakpoint::evaluateBreakpoints(Debugger &Dbg) {
- TargetSP Target;
- Status ST =
- Dbg.GetTargetList().CreateTarget(Dbg, breakpoint::Target, /*triple*/ "",
- /*get_dependent_modules*/ false,
- /*platform_options*/ nullptr, Target);
- if (ST.Fail()) {
-errs() << formatv("Failed to create target '{0}: {1}\n", breakpoint::Target,
- ST);
-
[Lldb-commits] [PATCH] D47508: [lldb-test] Add a testing harness for the JIT's IRMemoryMap
vsk updated this revision to Diff 149173.
vsk edited the summary of this revision.
vsk added a comment.
- Really fix the allocation overlap test. The previous version of this patch
would not detect overlaps in which the end of the new allocation is contained
within an existing allocation.
> The idea that came to me while looking at this is testing this gdb-client
> style. This would allow you to mock the server responses to allocation and
> e.g. test handling of allocation failures. However, the problem is these
> tests sit on top of SBAPI and there seems to be no way to issue "raw"
> allocation requests through that (although maybe there is a case to be made
> for SBProcess.AllocateMemory as a generally useful API).
>
> However, if this does the job you want, then I'm fine with that too.
Testing at this level looks to be sufficient to uncover the bugs I'm concerned
about, so I'd prefer not to extend the SB API if possible.
https://reviews.llvm.org/D47508
Files:
lit/Expr/TestIRMemoryMap.test
source/Target/Process.cpp
tools/lldb-test/lldb-test.cpp
Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -15,37 +15,53 @@
#include "lldb/Core/Debugger.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/Section.h"
+#include "lldb/Expression/IRMemoryMap.h"
#include "lldb/Initialization/SystemLifetimeManager.h"
#include "lldb/Interpreter/CommandInterpreter.h"
#include "lldb/Interpreter/CommandReturnObject.h"
#include "lldb/Symbol/ClangASTContext.h"
#include "lldb/Symbol/ClangASTImporter.h"
#include "lldb/Symbol/SymbolVendor.h"
#include "lldb/Symbol/TypeList.h"
#include "lldb/Symbol/VariableList.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
#include "lldb/Utility/CleanUp.h"
#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/StreamString.h"
+#include "llvm/ADT/IntervalMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MathExtras.h"
#include "llvm/Support/ManagedStatic.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/PrettyStackTrace.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/WithColor.h"
+#include
#include
using namespace lldb;
using namespace lldb_private;
using namespace llvm;
namespace opts {
+
static cl::SubCommand BreakpointSubcommand("breakpoints",
"Test breakpoint resolution");
cl::SubCommand ModuleSubcommand("module-sections",
"Display LLDB Module Information");
cl::SubCommand SymbolsSubcommand("symbols", "Dump symbols for an object file");
+cl::SubCommand IRMemoryMapSubcommand("ir-memory-map", "Test IRMemoryMap");
+cl::opt Log("log", cl::desc("Path to a log file"), cl::init(""),
+ cl::sub(IRMemoryMapSubcommand));
+
+/// Create a target using the file pointed to by \p Filename, or abort.
+TargetSP createTarget(Debugger &Dbg, const std::string &Filename);
+
+/// Read \p Filename into a null-terminated buffer, or abort.
+std::unique_ptr openFile(const std::string &Filename);
namespace breakpoint {
static cl::opt Target(cl::Positional, cl::desc(""),
@@ -135,8 +151,49 @@
static int dumpSymbols(Debugger &Dbg);
}
+
+namespace irmemorymap {
+static cl::opt Target(cl::Positional, cl::desc(""),
+ cl::Required,
+ cl::sub(IRMemoryMapSubcommand));
+static cl::opt CommandFile(cl::Positional,
+cl::desc(""),
+cl::init("-"),
+cl::sub(IRMemoryMapSubcommand));
+using AllocationT = std::pair;
+bool areAllocationsOverlapping(const AllocationT &L, const AllocationT &R);
+using AddrIntervalMap =
+ IntervalMap>;
+bool evalMalloc(IRMemoryMap &IRMemMap, StringRef Line,
+AddrIntervalMap &AllocatedIntervals);
+int evaluateMemoryMapCommands(Debugger &Dbg);
+} // namespace irmemorymap
+
} // namespace opts
+TargetSP opts::createTarget(Debugger &Dbg, const std::string &Filename) {
+ TargetSP Target;
+ Status ST =
+ Dbg.GetTargetList().CreateTarget(Dbg, Filename, /*triple*/ "",
+ /*get_dependent_modules*/ false,
+ /*platform_options*/ nullptr, Target);
+ if (ST.Fail()) {
+errs() << formatv("Failed to create target '{0}: {1}\n", Filename, ST);
+exit(1);
+ }
+ return Target;
+}
+
+std::unique_ptr opts::openFile(const std::string &Filename) {
+ auto MB = MemoryBuffer::getFileOrSTDIN(Filename);
+ if (!MB) {
+errs() << formatv("Could not open file '{0}: {1}\n", Filename,
+ MB.getError().message());
+exit(1);
+ }
+ return std::move(*MB);
+}
+
void opts::breakpoint::dumpState(const BreakpointList &List, LinePrinter &P)
[Lldb-commits] [PATCH] D47551: [IRMemoryMap] Fix the alignment adjustment in Malloc
vsk created this revision.
vsk added reviewers: labath, zturner, jingham, aprantl.
vsk edited the summary of this revision.
This prevents Malloc from allocating the same chunk of memory twice, as
a byproduct of an alignment adjustment which gave the client access to
unallocated memory.
Prior to this patch, the newly-added test failed with:
$ lldb-test ir-memory-map ... ir-memory-map-overlap1.test
Command: malloc(size=8, alignment=16)
Malloc: address = 0x1000cd000
Command: malloc(size=16, alignment=8)
Malloc: address = 0x1000cd010
Command: malloc(size=64, alignment=32)
Malloc: address = 0x1000cd020
Command: malloc(size=1, alignment=8)
Malloc: address = 0x1000cd060
Command: malloc(size=64, alignment=32)
Malloc: address = 0x1000cd080
Command: malloc(size=64, alignment=8)
Malloc: address = 0x1000cd0b0
Malloc error: overlapping allocation detected, previous allocation at
[0x1000cd080, 0x1000cd0c0)
I don't see anything controversial here (in fact Jim lgtm'd part of this patch
off-list), but as this is unfamiliar territory for me I think it'd help to have
a proper review. Depends on https://reviews.llvm.org/D47508.
https://reviews.llvm.org/D47551
Files:
lit/Expr/Inputs/ir-memory-map-basic.test
lit/Expr/Inputs/ir-memory-map-overlap1.test
lit/Expr/TestIRMemoryMap.test
source/Expression/IRMemoryMap.cpp
Index: source/Expression/IRMemoryMap.cpp
===
--- source/Expression/IRMemoryMap.cpp
+++ source/Expression/IRMemoryMap.cpp
@@ -304,12 +304,25 @@
size_t alignment_mask = alignment - 1;
size_t allocation_size;
- if (size == 0)
+ if (size == 0) {
+// FIXME: Malloc(0) should either return an invalid address or assert, in
+// order to cut down on unnecessary allocations.
allocation_size = alignment;
- else
-allocation_size = (size & alignment_mask)
- ? ((size + alignment) & (~alignment_mask))
- : size;
+ } else {
+// Round up the requested size to an aligned value, if needed.
+if (size & alignment_mask)
+ allocation_size = ((size + alignment) & (~alignment_mask));
+else
+ allocation_size = size;
+
+// The process page cache does not see the requested alignment. We can't
+// assume its result will be any more than 1-byte aligned. To work around
+// this, request `alignment` additional bytes.
+//
+// FIXME: Pass the requested alignment into the process page cache to
+// reduce internal fragmentation.
+allocation_size += alignment;
+ }
switch (policy) {
default:
Index: lit/Expr/TestIRMemoryMap.test
===
--- lit/Expr/TestIRMemoryMap.test
+++ lit/Expr/TestIRMemoryMap.test
@@ -1,28 +1,3 @@
# RUN: %cxx %p/Inputs/call-function.cpp -g -o %t
-# RUN: lldb-test ir-memory-map %t %s
-
-malloc 0 1
-malloc 1 1
-
-malloc 2 1
-malloc 2 2
-malloc 2 4
-
-malloc 3 1
-malloc 3 2
-malloc 3 4
-
-malloc 128 1
-malloc 128 2
-malloc 128 4
-malloc 128 128
-
-malloc 2048 1
-malloc 2048 2
-malloc 2048 4
-
-malloc 3968 1
-malloc 3968 2
-malloc 3968 4
-
-malloc 0 1
+# RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-basic.test
+# RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-overlap1.test
Index: lit/Expr/Inputs/ir-memory-map-overlap1.test
===
--- /dev/null
+++ lit/Expr/Inputs/ir-memory-map-overlap1.test
@@ -0,0 +1,10 @@
+malloc 8 16
+malloc 16 8
+malloc 64 32
+malloc 1 8
+malloc 64 32
+malloc 64 8
+malloc 1024 32
+malloc 1 16
+malloc 8 16
+malloc 1024 16
\ No newline at end of file
Index: lit/Expr/Inputs/ir-memory-map-basic.test
===
--- /dev/null
+++ lit/Expr/Inputs/ir-memory-map-basic.test
@@ -0,0 +1,25 @@
+malloc 0 1
+malloc 1 1
+
+malloc 2 1
+malloc 2 2
+malloc 2 4
+
+malloc 3 1
+malloc 3 2
+malloc 3 4
+
+malloc 128 1
+malloc 128 2
+malloc 128 4
+malloc 128 128
+
+malloc 2048 1
+malloc 2048 2
+malloc 2048 4
+
+malloc 3968 1
+malloc 3968 2
+malloc 3968 4
+
+malloc 0 1
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47551: [IRMemoryMap] Fix the alignment adjustment in Malloc
vsk updated this revision to Diff 149198.
vsk added a reviewer: lhames.
vsk added a comment.
- Don't insert extra padding bytes when `alignment` = 1.
- + Lang
https://reviews.llvm.org/D47551
Files:
lit/Expr/Inputs/ir-memory-map-basic.test
lit/Expr/Inputs/ir-memory-map-overlap1.test
lit/Expr/TestIRMemoryMap.test
source/Expression/IRMemoryMap.cpp
Index: source/Expression/IRMemoryMap.cpp
===
--- source/Expression/IRMemoryMap.cpp
+++ source/Expression/IRMemoryMap.cpp
@@ -304,12 +304,26 @@
size_t alignment_mask = alignment - 1;
size_t allocation_size;
- if (size == 0)
+ if (size == 0) {
+// FIXME: Malloc(0) should either return an invalid address or assert, in
+// order to cut down on unnecessary allocations.
allocation_size = alignment;
- else
-allocation_size = (size & alignment_mask)
- ? ((size + alignment) & (~alignment_mask))
- : size;
+ } else {
+// Round up the requested size to an aligned value, if needed.
+if (size & alignment_mask)
+ allocation_size = ((size + alignment) & (~alignment_mask));
+else
+ allocation_size = size;
+
+// The process page cache does not see the requested alignment. We can't
+// assume its result will be any more than 1-byte aligned. To work around
+// this, request `alignment` additional bytes.
+//
+// FIXME: Pass the requested alignment into the process page cache to
+// reduce internal fragmentation.
+if (alignment > 1)
+ allocation_size += alignment;
+ }
switch (policy) {
default:
Index: lit/Expr/TestIRMemoryMap.test
===
--- lit/Expr/TestIRMemoryMap.test
+++ lit/Expr/TestIRMemoryMap.test
@@ -1,28 +1,3 @@
# RUN: %cxx %p/Inputs/call-function.cpp -g -o %t
-# RUN: lldb-test ir-memory-map %t %s
-
-malloc 0 1
-malloc 1 1
-
-malloc 2 1
-malloc 2 2
-malloc 2 4
-
-malloc 3 1
-malloc 3 2
-malloc 3 4
-
-malloc 128 1
-malloc 128 2
-malloc 128 4
-malloc 128 128
-
-malloc 2048 1
-malloc 2048 2
-malloc 2048 4
-
-malloc 3968 1
-malloc 3968 2
-malloc 3968 4
-
-malloc 0 1
+# RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-basic.test
+# RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-overlap1.test
Index: lit/Expr/Inputs/ir-memory-map-overlap1.test
===
--- /dev/null
+++ lit/Expr/Inputs/ir-memory-map-overlap1.test
@@ -0,0 +1,10 @@
+malloc 8 16
+malloc 16 8
+malloc 64 32
+malloc 1 8
+malloc 64 32
+malloc 64 8
+malloc 1024 32
+malloc 1 16
+malloc 8 16
+malloc 1024 16
\ No newline at end of file
Index: lit/Expr/Inputs/ir-memory-map-basic.test
===
--- /dev/null
+++ lit/Expr/Inputs/ir-memory-map-basic.test
@@ -0,0 +1,25 @@
+malloc 0 1
+malloc 1 1
+
+malloc 2 1
+malloc 2 2
+malloc 2 4
+
+malloc 3 1
+malloc 3 2
+malloc 3 4
+
+malloc 128 1
+malloc 128 2
+malloc 128 4
+malloc 128 128
+
+malloc 2048 1
+malloc 2048 2
+malloc 2048 4
+
+malloc 3968 1
+malloc 3968 2
+malloc 3968 4
+
+malloc 0 1
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47551: [IRMemoryMap] Fix the alignment adjustment in Malloc
vsk added a comment. In https://reviews.llvm.org/D47551#1117086, @lhames wrote: > LGTM. > > I haven't looked at process memory management. How hard would your FIXME be > to implement? After looking at this more carefully, I think the FIXME makes a bad prescription. It's based on the assumption that each platform-specific allocator makes use of an efficient API to make aligned allocations. This doesn't generally seem to be true, because the platform-specific allocators rely on mmap(). A better alternative might be to add a new API, `AllocatedMemoryCache::AllocateAlignedMemory(size, alignment)`. You could implement an alignment-aware allocator here, say, by splitting up free memory into a list of buckets (one list per alignment). Next, you could surface the API from `Process`, provided there's a fallback strategy when the allocated memory cache is disabled. One caveat to all of this is that it might not be worth doing unless fragmentation-related overhead is identified as a bottleneck. I'll remove the FIXME. https://reviews.llvm.org/D47551 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47551: [IRMemoryMap] Fix the alignment adjustment in Malloc
vsk updated this revision to Diff 149355.
vsk marked 2 inline comments as done.
vsk added a comment.
- Address Pavel's feedback, remove a questionable FIXME.
https://reviews.llvm.org/D47551
Files:
lit/Expr/Inputs/ir-memory-map-basic.test
lit/Expr/Inputs/ir-memory-map-overlap1.test
lit/Expr/TestIRMemoryMap.test
source/Expression/IRMemoryMap.cpp
Index: source/Expression/IRMemoryMap.cpp
===
--- source/Expression/IRMemoryMap.cpp
+++ source/Expression/IRMemoryMap.cpp
@@ -301,15 +301,21 @@
lldb::addr_t allocation_address = LLDB_INVALID_ADDRESS;
lldb::addr_t aligned_address = LLDB_INVALID_ADDRESS;
- size_t alignment_mask = alignment - 1;
size_t allocation_size;
- if (size == 0)
+ if (size == 0) {
+// FIXME: Malloc(0) should either return an invalid address or assert, in
+// order to cut down on unnecessary allocations.
allocation_size = alignment;
- else
-allocation_size = (size & alignment_mask)
- ? ((size + alignment) & (~alignment_mask))
- : size;
+ } else {
+// Round up the requested size to an aligned value.
+allocation_size = llvm::alignTo(size, alignment);
+
+// The process page cache does not see the requested alignment. We can't
+// assume its result will be any more than 1-byte aligned. To work around
+// this, request `alignment - 1` additional bytes.
+allocation_size += alignment - 1;
+ }
switch (policy) {
default:
Index: lit/Expr/TestIRMemoryMap.test
===
--- lit/Expr/TestIRMemoryMap.test
+++ lit/Expr/TestIRMemoryMap.test
@@ -1,28 +1,3 @@
# RUN: %cxx %p/Inputs/call-function.cpp -g -o %t
-# RUN: lldb-test ir-memory-map %t %s
-
-malloc 0 1
-malloc 1 1
-
-malloc 2 1
-malloc 2 2
-malloc 2 4
-
-malloc 3 1
-malloc 3 2
-malloc 3 4
-
-malloc 128 1
-malloc 128 2
-malloc 128 4
-malloc 128 128
-
-malloc 2048 1
-malloc 2048 2
-malloc 2048 4
-
-malloc 3968 1
-malloc 3968 2
-malloc 3968 4
-
-malloc 0 1
+# RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-basic.test
+# RUN: lldb-test ir-memory-map %t %S/Inputs/ir-memory-map-overlap1.test
Index: lit/Expr/Inputs/ir-memory-map-overlap1.test
===
--- /dev/null
+++ lit/Expr/Inputs/ir-memory-map-overlap1.test
@@ -0,0 +1,10 @@
+malloc 8 16
+malloc 16 8
+malloc 64 32
+malloc 1 8
+malloc 64 32
+malloc 64 8
+malloc 1024 32
+malloc 1 16
+malloc 8 16
+malloc 1024 16
\ No newline at end of file
Index: lit/Expr/Inputs/ir-memory-map-basic.test
===
--- /dev/null
+++ lit/Expr/Inputs/ir-memory-map-basic.test
@@ -0,0 +1,25 @@
+malloc 0 1
+malloc 1 1
+
+malloc 2 1
+malloc 2 2
+malloc 2 4
+
+malloc 3 1
+malloc 3 2
+malloc 3 4
+
+malloc 128 1
+malloc 128 2
+malloc 128 4
+malloc 128 128
+
+malloc 2048 1
+malloc 2048 2
+malloc 2048 4
+
+malloc 3968 1
+malloc 3968 2
+malloc 3968 4
+
+malloc 0 1
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47646: [IRMemoryMap] Use labels in the "malloc" and "free" lldb-test commands
vsk created this revision.
vsk added a reviewer: labath.
Change the syntax of the malloc and free commands in lldb-test's
ir-memory-map subcommand to:
::= = malloc
::= free
This should make it easier to read and extend tests in the future, e.g
to test IRMemoryMap::WriteMemory or double-free behavior.
https://reviews.llvm.org/D47646
Files:
lit/Expr/Inputs/ir-memory-map-basic
lit/Expr/Inputs/ir-memory-map-mix-malloc-free
lit/Expr/Inputs/ir-memory-map-overlap1
tools/lldb-test/lldb-test.cpp
Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -167,13 +167,25 @@
cl::init(false), cl::sub(IRMemoryMapSubcommand));
using AllocationT = std::pair;
-bool areAllocationsOverlapping(const AllocationT &L, const AllocationT &R);
using AddrIntervalMap =
IntervalMap>;
-bool evalMalloc(IRMemoryMap &IRMemMap, StringRef Line,
-AddrIntervalMap &AllocatedIntervals);
-bool evalFree(IRMemoryMap &IRMemMap, StringRef Line,
- AddrIntervalMap &AllocatedIntervals);
+
+struct IRMemoryMapTestState {
+ TargetSP Target;
+ IRMemoryMap Map;
+
+ AddrIntervalMap::Allocator IntervalMapAllocator;
+ AddrIntervalMap Allocations;
+
+ StringMap Label2AddrMap;
+
+ IRMemoryMapTestState(TargetSP Target)
+ : Target(Target), Map(Target), Allocations(IntervalMapAllocator) {}
+};
+
+bool areAllocationsOverlapping(const AllocationT &L, const AllocationT &R);
+bool evalMalloc(StringRef Line, IRMemoryMapTestState &State);
+bool evalFree(StringRef Line, IRMemoryMapTestState &State);
int evaluateMemoryMapCommands(Debugger &Dbg);
} // namespace irmemorymap
@@ -514,17 +526,23 @@
return R.first < L.second && L.first < R.second;
}
-bool opts::irmemorymap::evalMalloc(IRMemoryMap &IRMemMap, StringRef Line,
- AddrIntervalMap &AllocatedIntervals) {
- // ::= malloc
+bool opts::irmemorymap::evalMalloc(StringRef Line,
+ IRMemoryMapTestState &State) {
+ // ::= = malloc
+ StringRef Label;
+ std::tie(Label, Line) = Line.split('=');
+ if (Line.empty())
+return false;
+ Label = Label.trim();
+ Line = Line.trim();
size_t Size;
uint8_t Alignment;
int Matches = sscanf(Line.data(), "malloc %zu %hhu", &Size, &Alignment);
if (Matches != 2)
return false;
- outs() << formatv("Command: malloc(size={0}, alignment={1})\n", Size,
-Alignment);
+ outs() << formatv("Command: {0} = malloc(size={1}, alignment={2})\n", Label,
+Size, Alignment);
if (!isPowerOf2_32(Alignment)) {
outs() << "Malloc error: alignment is not a power of 2\n";
exit(1);
@@ -539,7 +557,7 @@
const bool ZeroMemory = false;
Status ST;
addr_t Addr =
- IRMemMap.Malloc(Size, Alignment, Permissions, AP, ZeroMemory, ST);
+ State.Map.Malloc(Size, Alignment, Permissions, AP, ZeroMemory, ST);
if (ST.Fail()) {
outs() << formatv("Malloc error: {0}\n", ST);
return true;
@@ -557,10 +575,10 @@
// Check that the allocation does not overlap another allocation. Do so by
// testing each allocation which may cover the interval [Addr, EndOfRegion).
addr_t EndOfRegion = Addr + Size;
- auto Probe = AllocatedIntervals.begin();
+ auto Probe = State.Allocations.begin();
Probe.advanceTo(Addr); //< First interval s.t stop >= Addr.
AllocationT NewAllocation = {Addr, EndOfRegion};
- while (Probe != AllocatedIntervals.end() && Probe.start() < EndOfRegion) {
+ while (Probe != State.Allocations.end() && Probe.start() < EndOfRegion) {
AllocationT ProbeAllocation = {Probe.start(), Probe.stop()};
if (areAllocationsOverlapping(ProbeAllocation, NewAllocation)) {
outs() << "Malloc error: overlapping allocation detected"
@@ -575,41 +593,42 @@
// to inhibit interval coalescing.
static unsigned AllocationID = 0;
if (Size)
-AllocatedIntervals.insert(Addr, EndOfRegion, AllocationID++);
+State.Allocations.insert(Addr, EndOfRegion, AllocationID++);
+
+ // Store the label -> address mapping.
+ State.Label2AddrMap[Label] = Addr;
return true;
}
-bool opts::irmemorymap::evalFree(IRMemoryMap &IRMemMap, StringRef Line,
- AddrIntervalMap &AllocatedIntervals) {
- // ::= free
- size_t AllocIndex;
- int Matches = sscanf(Line.data(), "free %zu", &AllocIndex);
- if (Matches != 1)
+bool opts::irmemorymap::evalFree(StringRef Line, IRMemoryMapTestState &State) {
+ // ::= free
+ if (!Line.startswith("free"))
return false;
+ StringRef Label = Line.substr(Line.find(' ')).trim();
- outs() << formatv("Command: free(allocation-index={0})\n", AllocIndex);
-
- // Find and free the AllocIndex-th allocation.
- auto Probe = AllocatedIntervals.begin();
- for (size_t I = 0; I < AllocIndex && Probe != AllocatedIntervals.end(); ++I)
-++Probe;
-
- if (Probe == AllocatedIntervals.end
[Lldb-commits] [PATCH] D47646: [IRMemoryMap] Use labels in the "malloc" and "free" lldb-test commands
vsk added a comment. In https://reviews.llvm.org/D47646#1119396, @stella.stamenova wrote: > While you are at it, can you make sure this works on Windows? The current > version of the test that is checked in fails. Sorry about that. Could you point me to the error message? https://reviews.llvm.org/D47646 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47646: [IRMemoryMap] Use labels in the "malloc" and "free" lldb-test commands
vsk added a comment. In https://reviews.llvm.org/D47646#1119479, @stella.stamenova wrote: > In https://reviews.llvm.org/D47646#1119471, @vsk wrote: > > > In https://reviews.llvm.org/D47646#1119396, @stella.stamenova wrote: > > > > > While you are at it, can you make sure this works on Windows? The current > > > version of the test that is checked in fails. > > > > > > Sorry about that. Could you point me to the error message? > > > "Cannot use process to test IRMemoryMap" Hm, I'm afraid this just tells us that the debugger either failed to create a process, or that the process is not alive / incapable of JIT'ing. As I don't have a Windows machine on hand, I can't dig into this further. Would you mind taking a look? I can simply disable these tests on Windows for now. https://reviews.llvm.org/D47646 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47646: [IRMemoryMap] Use labels in the "malloc" and "free" lldb-test commands
vsk added a comment. In https://reviews.llvm.org/D47646#1119487, @vsk wrote: > In https://reviews.llvm.org/D47646#1119479, @stella.stamenova wrote: > > > In https://reviews.llvm.org/D47646#1119471, @vsk wrote: > > > > > In https://reviews.llvm.org/D47646#1119396, @stella.stamenova wrote: > > > > > > > While you are at it, can you make sure this works on Windows? The > > > > current version of the test that is checked in fails. > > > > > > > > > Sorry about that. Could you point me to the error message? > > > > > > "Cannot use process to test IRMemoryMap" > > > Hm, I'm afraid this just tells us that the debugger either failed to create a > process, or that the process is not alive / incapable of JIT'ing. As I don't > have a Windows machine on hand, I can't dig into this further. Would you mind > taking a look? I can simply disable these tests on Windows for now. One additional thing to try: you can run the lldb-test command with `-log path/to/log/file` appended. The resulting logging output may help narrow down the process creation issue. https://reviews.llvm.org/D47646 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47646: [IRMemoryMap] Use labels in the "malloc" and "free" lldb-test commands
vsk added a comment. In https://reviews.llvm.org/D47646#1119512, @stella.stamenova wrote: > I can look into the failure - but can you XFAIL the test rather than skipping > it and log a bug, so that we can track the failure rather than potentially > assuming down the line that the test is not meant for windows? Sure, I've xfailed the test in r333787 and filed llvm.org/PR37656. https://reviews.llvm.org/D47646 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48450: [DataFormatter] Add CFDictionary data formatter
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rL LLVM https://reviews.llvm.org/D48450 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48303: Don't take the address of an xvalue when printing an expr result
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, LGTM. This looks pretty cut and dry. The evaluator shouldn't try to take the address of an rvalue. https://reviews.llvm.org/D48303 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50087: Add doxygen comments to the StackFrameList API (NFC)
vsk created this revision.
vsk added reviewers: labath, jasonmolenda, tberghammer.
Clarify how StackFrameList works by documenting its methods. Also,
delete some dead code and insert some TODOs.
https://reviews.llvm.org/D50087
Files:
lldb/include/lldb/Target/StackFrameList.h
lldb/source/Target/StackFrameList.cpp
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -436,11 +436,7 @@
if (can_create)
GetFramesUpTo(UINT32_MAX);
- uint32_t inlined_depth = GetCurrentInlinedDepth();
- if (inlined_depth == UINT32_MAX)
-return m_frames.size();
- else
-return m_frames.size() - inlined_depth;
+ return GetVisibleStackFrameIndex(m_frames.size());
}
void StackFrameList::Dump(Stream *s) {
@@ -620,7 +616,6 @@
return m_selected_frame_idx;
}
-// Mark a stack frame as the current frame using the frame index
bool StackFrameList::SetSelectedFrameByIndex(uint32_t idx) {
std::lock_guard guard(m_mutex);
StackFrameSP frame_sp(GetFrameAtIndex(idx));
@@ -652,19 +647,6 @@
m_concrete_frames_fetched = 0;
}
-void StackFrameList::InvalidateFrames(uint32_t start_idx) {
- std::lock_guard guard(m_mutex);
- if (m_show_inlined_frames) {
-Clear();
- } else {
-const size_t num_frames = m_frames.size();
-while (start_idx < num_frames) {
- m_frames[start_idx].reset();
- ++start_idx;
-}
- }
-}
-
void StackFrameList::Merge(std::unique_ptr &curr_ap,
lldb::StackFrameListSP &prev_sp) {
std::unique_lock current_lock, previous_lock;
Index: lldb/include/lldb/Target/StackFrameList.h
===
--- lldb/include/lldb/Target/StackFrameList.h
+++ lldb/include/lldb/Target/StackFrameList.h
@@ -10,14 +10,10 @@
#ifndef liblldb_StackFrameList_h_
#define liblldb_StackFrameList_h_
-// C Includes
-// C++ Includes
#include
#include
#include
-// Other libraries and framework includes
-// Project includes
#include "lldb/Target/StackFrame.h"
namespace lldb_private {
@@ -32,39 +28,57 @@
~StackFrameList();
+ /// Get the number of visible frames. Frames may be created if \p can_create
+ /// is true. Synthetic (inline) frames expanded from the concrete frame #0
+ /// (aka invisible frames) are not included in this count.
uint32_t GetNumFrames(bool can_create = true);
+ /// Get the frame at index \p idx. Invisible frames cannot be indexed.
lldb::StackFrameSP GetFrameAtIndex(uint32_t idx);
+ /// Get the first concrete frame with index greater than or equal to \p idx.
+ /// Unlike \ref GetFrameAtIndex, this cannot return a synthetic frame.
lldb::StackFrameSP GetFrameWithConcreteFrameIndex(uint32_t unwind_idx);
+ /// Retrieve the stack frame with the given ID \p stack_id.
lldb::StackFrameSP GetFrameWithStackID(const StackID &stack_id);
- // Mark a stack frame as the current frame
+ /// Mark a stack frame as the currently selected frame and return its index.
uint32_t SetSelectedFrame(lldb_private::StackFrame *frame);
+ /// Get the currently selected frame index.
uint32_t GetSelectedFrameIndex() const;
- // Mark a stack frame as the current frame using the frame index
+ /// Mark a stack frame as the currently selected frame using the frame index
+ /// \p idx. Like \ref GetFrameAtIndex, invisible frames cannot be selected.
bool SetSelectedFrameByIndex(uint32_t idx);
+ /// If the current inline depth is valid, subtract it from \p idx. Otherwise
+ /// simply return \p idx.
uint32_t GetVisibleStackFrameIndex(uint32_t idx) {
if (m_current_inlined_depth < UINT32_MAX)
return idx - m_current_inlined_depth;
else
return idx;
}
+ /// Calculate and set the current inline depth. This may be used to update
+ /// the StackFrameList's set of inline frames when execution stops, e.g when
+ /// a breakpoint is hit.
void CalculateCurrentInlinedDepth();
+ /// If the currently selected frame comes from the currently selected thread,
+ /// point the default file and line of the thread's target to the location
+ /// specified by the frame.
void SetDefaultFileAndLineToSelectedFrame();
+ /// Clear the cache of frames.
void Clear();
- void InvalidateFrames(uint32_t start_idx);
-
void Dump(Stream *s);
+ /// If \p stack_frame_ptr is contained in this StackFrameList, return its
+ /// wrapping shared pointer.
lldb::StackFrameSP
GetStackFrameSPForStackFramePtr(StackFrame *stack_frame_ptr);
@@ -101,14 +115,44 @@
typedef collection::iterator iterator;
typedef collection::const_iterator const_iterator;
+ /// The thread this frame list describes.
Thread &m_thread;
+
+ /// The old stack frame list.
+ // TODO: The old stack frame list is used to fill in missing frame info
+ // heuristically when it's otherwise unavailable (say, because the unwinder
[Lldb-commits] [PATCH] D50087: Add doxygen comments to the StackFrameList API (NFC)
vsk added a comment.
In https://reviews.llvm.org/D50087#1183982, @labath wrote:
> I am not too familiar with this code, but the descriptions seem to make sense.
>
> However, since you have kind of opened up the `Optional` discussion, I'll use
> this opportunity to give my take on it:
>
> I've wanted to use `Optional` in this way in the past, but the thing
> that has always stopped me is that this essentially doubles the amount of
> storage required for the value (you only need one bit for the `IsValid`
> field, but due to alignment constraints, this will usually consume the same
> amount of space as the underlying int type). This may not matter for the
> StackFrameList class, but it does matter for classes which have a lot of
> instances floating around. I suspect this is also why LLVM does not generally
> use this idiom (the last example where I ran into this is the VersionTuple
> class, which hand-rolls a packed optional by stealing a bit from the int
> type, and then converts this to Optional for the public accessors).
>
> For this reason, I think it would be useful to have a specialized class
> (`OptionalInt`?), which has the same interface as `Optional`, but does not
> increase the storage size. It could be implemented the same way as we
> hand-roll the invalid values, only now the invalid values would be a part of
> the type. So, you could do something like:
>
> using tid_t = OptionalInt;
>
> tid_t my_tid; // initialized to "invalid";
> my_tid = get_thread_id();
> if (my_tid) // no need to remember what is the correct "invalid" value here
> do_stuff(*my_tid);
> else
> printf("no thread");
>
>
> I am sure this is more than what you wanted to hear in this patch, but I
> wanted to make sure consider the tradeoffs before we start putting Optionals
> everywhere.
No, I think that's a good point. I held off on introducing llvm::Optional here
because of similar reservations. I think it'd make sense to introduce an
OptionalInt facility, or possibly something more general that supports non-POD
types & more exotic invalid values. Definitely something to revisit.
https://reviews.llvm.org/D50087
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50155: Delete MacOSXFrameBackchain unwind logic (NFC)
vsk created this revision.
vsk added reviewers: aprantl, jasonmolenda.
Herald added subscribers: chrib, krytarowski, mgorny, srhines.
This code looks like a good reference for building a new unwinder, but
is currently unused, so there's no need to keep it.
https://reviews.llvm.org/D50155
Files:
lldb/lldb.xcodeproj/project.pbxproj
lldb/source/Plugins/Process/Utility/CMakeLists.txt
lldb/source/Plugins/Process/Utility/RegisterContextMacOSXFrameBackchain.cpp
lldb/source/Plugins/Process/Utility/RegisterContextMacOSXFrameBackchain.h
lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp
lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.h
lldb/source/Target/Thread.cpp
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -13,7 +13,6 @@
// Project includes
#include "lldb/Target/Thread.h"
#include "Plugins/Process/Utility/UnwindLLDB.h"
-#include "Plugins/Process/Utility/UnwindMacOSXFrameBackchain.h"
#include "lldb/Breakpoint/BreakpointLocation.h"
#include "lldb/Core/Debugger.h"
#include "lldb/Core/FormatEntity.h"
@@ -2055,10 +2054,7 @@
case llvm::Triple::hexagon:
m_unwinder_ap.reset(new UnwindLLDB(*this));
break;
-
default:
- if (target_arch.GetTriple().getVendor() == llvm::Triple::Apple)
-m_unwinder_ap.reset(new UnwindMacOSXFrameBackchain(*this));
break;
}
}
Index: lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.h
===
--- lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.h
+++ /dev/null
@@ -1,60 +0,0 @@
-//===-- UnwindMacOSXFrameBackchain.h *- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#ifndef lldb_UnwindMacOSXFrameBackchain_h_
-#define lldb_UnwindMacOSXFrameBackchain_h_
-
-// C Includes
-// C++ Includes
-#include
-
-// Other libraries and framework includes
-// Project includes
-#include "lldb/Target/Unwind.h"
-#include "lldb/lldb-private.h"
-
-class UnwindMacOSXFrameBackchain : public lldb_private::Unwind {
-public:
- UnwindMacOSXFrameBackchain(lldb_private::Thread &thread);
-
- ~UnwindMacOSXFrameBackchain() override = default;
-
-protected:
- void DoClear() override { m_cursors.clear(); }
-
- uint32_t DoGetFrameCount() override;
-
- bool DoGetFrameInfoAtIndex(uint32_t frame_idx, lldb::addr_t &cfa,
- lldb::addr_t &pc) override;
-
- lldb::RegisterContextSP
- DoCreateRegisterContextForFrame(lldb_private::StackFrame *frame) override;
-
- friend class RegisterContextMacOSXFrameBackchain;
-
- struct Cursor {
-lldb::addr_t pc; // Program counter
-lldb::addr_t fp; // Frame pointer for us with backchain
- };
-
-private:
- std::vector m_cursors;
-
- size_t GetStackFrameData_i386(const lldb_private::ExecutionContext &exe_ctx);
-
- size_t
- GetStackFrameData_x86_64(const lldb_private::ExecutionContext &exe_ctx);
-
- //--
- // For UnwindMacOSXFrameBackchain only
- //--
- DISALLOW_COPY_AND_ASSIGN(UnwindMacOSXFrameBackchain);
-};
-
-#endif // lldb_UnwindMacOSXFrameBackchain_h_
Index: lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp
===
--- lldb/source/Plugins/Process/Utility/UnwindMacOSXFrameBackchain.cpp
+++ /dev/null
@@ -1,246 +0,0 @@
-//===-- UnwindMacOSXFrameBackchain.cpp --*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "lldb/Symbol/Function.h"
-#include "lldb/Symbol/ObjectFile.h"
-#include "lldb/Symbol/Symbol.h"
-#include "lldb/Target/ExecutionContext.h"
-#include "lldb/Target/Process.h"
-#include "lldb/Target/Target.h"
-#include "lldb/Target/Thread.h"
-#include "lldb/Utility/ArchSpec.h"
-
-#include "RegisterContextMacOSXFrameBackchain.h"
-
-using namespace lldb;
-using namespace lldb_private;
-
-UnwindMacOSXFrameBackchain::UnwindMacOSXFrameBackchain(Thread &thread)
-: Unwind(thread), m_cursors() {}
-
-uint32_t UnwindMacOSXFrameBackchain::DoGetFrameCount() {
- if (m_cursors.empty()) {
-ExecutionContext exe_ctx(m_thread.shared_from_this());
-Target *target = exe_ctx.GetTargetPtr();
-if (target) {
- const ArchSpec &target_arch = target->GetArchitecture();
- // Frame zero should always be supplied
[Lldb-commits] [PATCH] D50149: Fix out-of-bounds read in Stream::PutCStringAsRawHex8
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, LGTM. https://reviews.llvm.org/D50149 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50225: Use a DenseMap for looking up functions by UID in CompileUnit::FindFunctionByUID
vsk added a comment.
Thanks for doing this :)!
Comment at: source/Symbol/CompileUnit.cpp:111
// TODO: order these by address
m_functions.push_back(funcSP);
}
Is m_functions used to do anything crucial?
I see a use in Dump, but it seems like you could replace that use by copying
the function map into a vector and sorting it. I see another use in
GetFunctionAtIndex, but that API can be deleted entirely because its only user
is lldb-test (via Module::ParseAllDebugSymbols). You'd just need to sink this
block of code into CompileUnit:
```
for (size_t func_idx = 0;
(sc.function = sc.comp_unit->GetFunctionAtIndex(func_idx).get()) !=
nullptr;
++func_idx) {
symbols->ParseFunctionBlocks(sc);
// Parse the variables for this function and all its blocks
symbols->ParseVariablesForContext(sc);
}
```
Repository:
rLLDB LLDB
https://reviews.llvm.org/D50225
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50271: [IRMemoryMap] Shrink Allocation by sizeof(addr_t) (NFC)
vsk created this revision.
vsk added reviewers: teemperor, lhames.
Profiling data show that Allocation::operator= is hot, see:
https://teemperor.de/lldb-bench/data/arithmetic.svg
Reorder a few fields within Allocation to avoid implicit structure
padding and shrink the structure. This should make copies cheaper and
reduce lldb's RSS.
There should be more low-hanging fruit here for performance
optimization: we don't need std::map, we can make Allocation move-only,
etc.
https://reviews.llvm.org/D50271
Files:
lldb/include/lldb/Expression/IRMemoryMap.h
lldb/source/Expression/IRMemoryMap.cpp
Index: lldb/source/Expression/IRMemoryMap.cpp
===
--- lldb/source/Expression/IRMemoryMap.cpp
+++ lldb/source/Expression/IRMemoryMap.cpp
@@ -272,8 +272,8 @@
uint32_t permissions, uint8_t alignment,
AllocationPolicy policy)
: m_process_alloc(process_alloc), m_process_start(process_start),
- m_size(size), m_permissions(permissions), m_alignment(alignment),
- m_policy(policy), m_leak(false) {
+ m_size(size), m_policy(policy), m_leak(false),
m_permissions(permissions),
+ m_alignment(alignment) {
switch (policy) {
default:
assert(0 && "We cannot reach this!");
Index: lldb/include/lldb/Expression/IRMemoryMap.h
===
--- lldb/include/lldb/Expression/IRMemoryMap.h
+++ lldb/include/lldb/Expression/IRMemoryMap.h
@@ -39,7 +39,7 @@
IRMemoryMap(lldb::TargetSP target_sp);
~IRMemoryMap();
- enum AllocationPolicy {
+ enum AllocationPolicy : uint8_t {
eAllocationPolicyInvalid =
0, ///< It is an error for an allocation to have this policy.
eAllocationPolicyHostOnly, ///< This allocation was created in the host and
@@ -91,32 +91,36 @@
private:
struct Allocation {
lldb::addr_t
-m_process_alloc; ///< The (unaligned) base for the remote allocation
+m_process_alloc; ///< The (unaligned) base for the remote allocation.
lldb::addr_t
-m_process_start; ///< The base address of the allocation in the process
-size_t m_size; ///< The size of the requested allocation
-uint32_t m_permissions; ///< The access permissions on the memory in the
-///process. In the host, the memory is always
-///read/write.
-uint8_t m_alignment;///< The alignment of the requested allocation
+m_process_start; ///< The base address of the allocation in the
process.
+size_t m_size; ///< The size of the requested allocation.
DataBufferHeap m_data;
-///< Flags
+/// Flags. Keep these grouped together to avoid structure padding.
AllocationPolicy m_policy;
bool m_leak;
+uint8_t m_permissions; ///< The access permissions on the memory in the
+ /// process. In the host, the memory is always
+ /// read/write.
+uint8_t m_alignment; ///< The alignment of the requested allocation.
public:
Allocation(lldb::addr_t process_alloc, lldb::addr_t process_start,
size_t size, uint32_t permissions, uint8_t alignment,
AllocationPolicy m_policy);
Allocation()
: m_process_alloc(LLDB_INVALID_ADDRESS),
- m_process_start(LLDB_INVALID_ADDRESS), m_size(0), m_permissions(0),
- m_alignment(0), m_data(), m_policy(eAllocationPolicyInvalid),
- m_leak(false) {}
+ m_process_start(LLDB_INVALID_ADDRESS), m_size(0), m_data(),
+ m_policy(eAllocationPolicyInvalid), m_leak(false), m_permissions(0),
+ m_alignment(0) {}
};
+ static_assert(sizeof(Allocation) <=
+(4 * sizeof(lldb::addr_t)) + sizeof(DataBufferHeap),
+"IRMemoryMap::Allocation is larger than expected");
+
lldb::ProcessWP m_process_wp;
lldb::TargetWP m_target_wp;
typedef std::map AllocationMap;
Index: lldb/source/Expression/IRMemoryMap.cpp
===
--- lldb/source/Expression/IRMemoryMap.cpp
+++ lldb/source/Expression/IRMemoryMap.cpp
@@ -272,8 +272,8 @@
uint32_t permissions, uint8_t alignment,
AllocationPolicy policy)
: m_process_alloc(process_alloc), m_process_start(process_start),
- m_size(size), m_permissions(permissions), m_alignment(alignment),
- m_policy(policy), m_leak(false) {
+ m_size(size), m_policy(policy), m_leak(false), m_permissions(permissions),
+ m_alignment(alignment) {
switch (policy) {
default:
assert(0 && "We cannot reach this!");
Index: lldb/include/lldb/Expression/IRMemoryMap.h
===
--- lldb/include/lldb/Expression/IRMemoryMap.h
+++ lldb/include/lldb/Expressio
[Lldb-commits] [PATCH] D50271: [IRMemoryMap] Shrink Allocation make it move-only (NFC)
vsk updated this revision to Diff 159339.
vsk retitled this revision from "[IRMemoryMap] Shrink Allocation by
sizeof(addr_t) (NFC)" to "[IRMemoryMap] Shrink Allocation make it move-only
(NFC)".
vsk edited the summary of this revision.
vsk added a comment.
- Make Allocation move-only. This should prevent a std::vector copy in ::Malloc.
https://reviews.llvm.org/D50271
Files:
lldb/include/lldb/Expression/IRMemoryMap.h
lldb/source/Expression/IRMemoryMap.cpp
Index: lldb/source/Expression/IRMemoryMap.cpp
===
--- lldb/source/Expression/IRMemoryMap.cpp
+++ lldb/source/Expression/IRMemoryMap.cpp
@@ -272,8 +272,8 @@
uint32_t permissions, uint8_t alignment,
AllocationPolicy policy)
: m_process_alloc(process_alloc), m_process_start(process_start),
- m_size(size), m_permissions(permissions), m_alignment(alignment),
- m_policy(policy), m_leak(false) {
+ m_size(size), m_policy(policy), m_leak(false),
m_permissions(permissions),
+ m_alignment(alignment) {
switch (policy) {
default:
assert(0 && "We cannot reach this!");
@@ -393,9 +393,10 @@
lldb::addr_t mask = alignment - 1;
aligned_address = (allocation_address + mask) & (~mask);
- m_allocations[aligned_address] =
- Allocation(allocation_address, aligned_address, allocation_size,
- permissions, alignment, policy);
+ m_allocations.emplace(
+ std::piecewise_construct, std::forward_as_tuple(aligned_address),
+ std::forward_as_tuple(allocation_address, aligned_address,
+allocation_size, permissions, alignment, policy));
if (zero_memory) {
Status write_error;
Index: lldb/include/lldb/Expression/IRMemoryMap.h
===
--- lldb/include/lldb/Expression/IRMemoryMap.h
+++ lldb/include/lldb/Expression/IRMemoryMap.h
@@ -39,7 +39,7 @@
IRMemoryMap(lldb::TargetSP target_sp);
~IRMemoryMap();
- enum AllocationPolicy {
+ enum AllocationPolicy : uint8_t {
eAllocationPolicyInvalid =
0, ///< It is an error for an allocation to have this policy.
eAllocationPolicyHostOnly, ///< This allocation was created in the host and
@@ -91,32 +91,32 @@
private:
struct Allocation {
lldb::addr_t
-m_process_alloc; ///< The (unaligned) base for the remote allocation
+m_process_alloc; ///< The (unaligned) base for the remote allocation.
lldb::addr_t
-m_process_start; ///< The base address of the allocation in the process
-size_t m_size; ///< The size of the requested allocation
-uint32_t m_permissions; ///< The access permissions on the memory in the
-///process. In the host, the memory is always
-///read/write.
-uint8_t m_alignment;///< The alignment of the requested allocation
+m_process_start; ///< The base address of the allocation in the
process.
+size_t m_size; ///< The size of the requested allocation.
DataBufferHeap m_data;
-///< Flags
+/// Flags. Keep these grouped together to avoid structure padding.
AllocationPolicy m_policy;
bool m_leak;
+uint8_t m_permissions; ///< The access permissions on the memory in the
+ /// process. In the host, the memory is always
+ /// read/write.
+uint8_t m_alignment; ///< The alignment of the requested allocation.
public:
Allocation(lldb::addr_t process_alloc, lldb::addr_t process_start,
size_t size, uint32_t permissions, uint8_t alignment,
AllocationPolicy m_policy);
-Allocation()
-: m_process_alloc(LLDB_INVALID_ADDRESS),
- m_process_start(LLDB_INVALID_ADDRESS), m_size(0), m_permissions(0),
- m_alignment(0), m_data(), m_policy(eAllocationPolicyInvalid),
- m_leak(false) {}
+DISALLOW_COPY_AND_ASSIGN(Allocation);
};
+ static_assert(sizeof(Allocation) <=
+(4 * sizeof(lldb::addr_t)) + sizeof(DataBufferHeap),
+"IRMemoryMap::Allocation is larger than expected");
+
lldb::ProcessWP m_process_wp;
lldb::TargetWP m_target_wp;
typedef std::map AllocationMap;
Index: lldb/source/Expression/IRMemoryMap.cpp
===
--- lldb/source/Expression/IRMemoryMap.cpp
+++ lldb/source/Expression/IRMemoryMap.cpp
@@ -272,8 +272,8 @@
uint32_t permissions, uint8_t alignment,
AllocationPolicy policy)
: m_process_alloc(process_alloc), m_process_start(process_start),
- m_size(size), m_permissions(permissions), m_alignment(alignment),
- m_policy(policy), m_leak(false) {
+ m_size(size), m_policy(policy), m_leak(false), m_permissions(permissions),
+
[Lldb-commits] [PATCH] D50478: WIP: Basic tail call frame support
vsk created this revision.
Herald added a subscriber: JDevlieghere.
This is a prototype of tail call frame support for lldb based on
compiler support from https://reviews.llvm.org/D49887. It isn't ready for
review. I'm sharing the
proof-of-concept to give a heads-up about this coming down the pipeline,
and/or to answer any early questions.
Demo (see the included tail5.cc test case):
+ /Users/vsk/src/builds/llvm-project-tailcall-RA/bin//clang++ tail5.cc -o
tail5.cc.out -g -mllvm -callsite-debuginfo-experimental=true -O2
+ xcrun lldb -o 'b sink' -o r -o bt -o quit tail5.cc.out
(lldb) target create "tail5.cc.out"
Current executable set to 'tail5.cc.out' (x86_64).
(lldb) b sink
Breakpoint 1: where = tail5.cc.out`sink() + 4 at tail5.cc:2, address =
0x00010f64
(lldb) r
tail5.cc.out was compiled with optimization - stepping may behave oddly;
variables may not be available.
Process 21540 stopped
- thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
frame #0: 0x00010f64 tail5.cc.out`sink() at tail5.cc:2 [opt] 1
volatile int x;
-> 2void __attribute__((noinline)) sink() { x++; }
3void __attribute__((noinline)) D() { sink(); /* tail */ }
4void __attribute__((disable_tail_calls, noinline)) C() { D(); /* regular
*/ }
5void __attribute__((noinline)) B() { C(); /* tail */ }
6int __attribute__((disable_tail_calls)) main() { B(); /* regular */ }
Target 0: (tail5.cc.out) stopped.
Process 21540 launched:
'/Users/vsk/src/llvm-project-tailcall/lldb/test/testcases/functionalities/tail_call_frames/tail5.cc.out'
(x86_64)
(lldb) bt
- thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
- frame #0: 0x00010f64 tail5.cc.out`sink() at tail5.cc:2 [opt] frame
#1: 0x00010f89 tail5.cc.out`C() at tail5.cc:4 [opt] frame #2:
0x00010fa9 tail5.cc.out`main at tail5.cc:6 [opt] frame #3:
0x7fff57147015 libdyld.dylib`start + 1 frame #4: 0x7fff57147015
libdyld.dylib`start + 1
(lldb) quit
+ /Users/vsk/src/builds/llvm-project-tailcall-RA/bin//lldb -o 'b sink' -o r -o
'log enable -f tail5.cc.log lldb step' -o bt -o quit tail5.cc.out
(lldb) target create "tail5.cc.out"
Current executable set to 'tail5.cc.out' (x86_64).
(lldb) b sink
Breakpoint 1: where = tail5.cc.out`sink() + 4 at tail5.cc:2, address =
0x00010f64
(lldb) r
tail5.cc.out was compiled with optimization - stepping may behave oddly;
variables may not be available.
Process 21544 stopped
- thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
frame #0: 0x00010f64 tail5.cc.out`sink() at tail5.cc:2 [opt] 1
volatile int x;
-> 2void __attribute__((noinline)) sink() { x++; }
3void __attribute__((noinline)) D() { sink(); /* tail */ }
4void __attribute__((disable_tail_calls, noinline)) C() { D(); /* regular
*/ }
5void __attribute__((noinline)) B() { C(); /* tail */ }
6int __attribute__((disable_tail_calls)) main() { B(); /* regular */ }
Process 21544 launched:
'/Users/vsk/src/llvm-project-tailcall/lldb/test/testcases/functionalities/tail_call_frames/tail5.cc.out'
(x86_64)
(lldb) log enable -f tail5.cc.log lldb step
(lldb) bt
- thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
- frame #0: 0x00010f64 tail5.cc.out`sink() at tail5.cc:2 [opt] frame
#1: 0x0010 tail5.cc.out`D() - 4294971232 [opt] frame #2:
0x00010f89 tail5.cc.out`C() at tail5.cc:4 [opt] frame #3:
0x0030 tail5.cc.out`B() - 4294971232 [opt] frame #4:
0x00010fa9 tail5.cc.out`main at tail5.cc:6 [opt] frame #5:
0x7fff57147015 libdyld.dylib`start + 1 frame #6: 0x7fff57147015
libdyld.dylib`start + 1
https://reviews.llvm.org/D50478
Files:
lldb/include/lldb/Symbol/Block.h
lldb/include/lldb/Symbol/Function.h
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Target/StackFrame.h
lldb/include/lldb/Target/StackFrameList.h
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/tail.cc
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/tail2.cc
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/tail3.cc
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/tail4.cc
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/tail5.cc
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/test.sh
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Symbol/Block.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Target/StackFrameList.cpp
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -27,6 +27,7 @@
#include "lldb/Target/Thread.h"
#include "lldb/Target/Unwind.h"
#include "lldb/Utility/Log.h"
+#include "llvm/A
[Lldb-commits] [PATCH] D50225: Use a DenseMap for looking up functions by UID in CompileUnit::FindFunctionByUID
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks, LGTM! https://reviews.llvm.org/D50225 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50478: WIP: Basic tail call frame support
vsk updated this revision to Diff 160224.
vsk added a comment.
Herald added a subscriber: mgrang.
Rebase, and update the patch to use DW_AT_call_return_pc information.
https://reviews.llvm.org/D50478
Files:
lldb/include/lldb/Symbol/Block.h
lldb/include/lldb/Symbol/Function.h
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Target/StackFrame.h
lldb/include/lldb/Target/StackFrameList.h
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/TODO
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/tail.cc
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/tail2.cc
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/tail3.cc
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/tail4.cc
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/tail5.cc
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/test.sh
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Symbol/Block.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Target/StackFrameList.cpp
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -27,6 +27,7 @@
#include "lldb/Target/Thread.h"
#include "lldb/Target/Unwind.h"
#include "lldb/Utility/Log.h"
+#include "llvm/ADT/SmallPtrSet.h"
//#define DEBUG_STACK_FRAMES 1
@@ -240,6 +241,134 @@
m_frames.resize(num_frames);
}
+/// Find the unique path through the call graph from \p begin (at PC offset
+/// \p call_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,
+ addr_t call_pc, std::vector &path,
+ ModuleList &images, Log *log) {
+ LLDB_LOG(log, "Finding frames between {0} and {1}, call-pc={2:x}",
+ begin.GetDisplayName(), end.GetDisplayName(), call_pc);
+
+ // Find a non-tail calling edge with the first return PC greater than the
+ // call PC.
+ auto first_level_edges = begin.GetCallEdges();
+ auto first_edge_it =
+ std::lower_bound(first_level_edges.begin(), first_level_edges.end(),
+ call_pc, [=](const CallEdge &edge, addr_t target_pc) {
+ return edge.GetReturnPCAddress() < target_pc;
+ });
+ if (first_edge_it == first_level_edges.end())
+return;
+ CallEdge &first_edge = const_cast(*first_edge_it);
+ if (first_edge.GetReturnPCAddress() == LLDB_INVALID_ADDRESS)
+return;
+
+ // The first callee may not be resolved, or there may be nothing to fill in.
+ Function *first_callee = first_edge.GetCallee(images);
+ if (!first_callee || first_callee == &end)
+return;
+
+ // Run DFS on the tail-calling edges out of the first callee to find \p end.
+ struct DFS {
+std::vector active_path = {};
+llvm::SmallPtrSet visited_nodes = {};
+bool ambiguous = false;
+Function *end;
+ModuleList &images;
+
+DFS(Function *end, ModuleList &images) : end(end), images(images) {}
+
+void search(Function *first_callee, std::vector &path) {
+ if (dfs(first_callee) && !ambiguous)
+path = std::move(active_path);
+}
+
+bool dfs(Function *callee) {
+ if (callee == end)
+return true;
+
+ // Terminate the search when tail recursion is detected.
+ if (!visited_nodes.insert(callee).second) {
+ambiguous = true;
+return false;
+ }
+
+ active_path.push_back(callee);
+ for (CallEdge &edge : callee->GetTailCallingEdges()) {
+Function *next_callee = edge.GetCallee(images);
+if (!next_callee)
+ continue;
+
+if (dfs(next_callee) && !ambiguous)
+ return true;
+
+if (ambiguous)
+ return false;
+ }
+ active_path.pop_back();
+ return false;
+}
+ };
+
+ DFS(&end, images).search(first_callee, path);
+}
+
+/// Given that \p next_frame will be appended to the frame list, synthesize
+/// tail call frames between the current end of the list and \p next_frame.
+/// If any frames are added, adjust the frame index of \p next_frame.
+void StackFrameList::SynthesizeTailCallFrames(StackFrame &next_frame) {
+ lldb::RegisterContextSP next_reg_ctx_sp = next_frame.GetRegisterContext();
+ if (!next_reg_ctx_sp)
+return;
+
+ Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
+
+ assert(!m_frames.empty() && "Cannot synthesize frames in an empty stack");
+ StackFrame &prev_frame = *m_frames.back().get();
+
+ // Find the functions prev_frame and next_frame are stopped in. The function
+ // objects are needed to search the lazy call graph for intervening frames.
+ Function *prev_func =
+ prev_frame.G
[Lldb-commits] [PATCH] D50620: Added test for Core/Range class.
vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.
Thanks, looks good with nitpicks.
Comment at: unittests/Core/RangeTest.cpp:139
+ RangeT r;
+ // FIXME: This is probably not intended.
+ EXPECT_TRUE(r.ContainsEndInclusive(0));
Yeah, this looks wrong.
Comment at: unittests/Core/RangeTest.cpp:177
+ EXPECT_TRUE(r.Contains(RangeT(3, 0)));
+ EXPECT_TRUE(r.Contains(RangeT(4, 0)));
+ EXPECT_FALSE(r.Contains(RangeT(8, 0)));
Yep, the first two results are at least inconsistent with the third, and look
wrong to me.
Comment at: unittests/Core/RangeTest.cpp:250
+
+// For less than, we should
+TEST(RangeTest, LessThan) {
Drop this comment?
Comment at: unittests/Core/RangeTest.cpp:253
+ RangeT r(10, 20);
+
+ // Equal range.
Might help to have a helper lambda here, like `expectOrderedLessThan = [](r1,
r2) { expect_true(r1 < r2); expect_false(r2 < r1); }`.
https://reviews.llvm.org/D50620
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk updated this revision to Diff 160666.
vsk retitled this revision from "WIP: Basic tail call frame support" to "Add
support for artificial tail call frames".
vsk edited the summary of this revision.
vsk added reviewers: aprantl, probinson, JDevlieghere, jingham, friss, zturner.
vsk removed subscribers: JDevlieghere, jingham, probinson, aprantl.
vsk added a dependency: D49887: [DebugInfo] Add support for DWARF5 call
site-related attributes.
https://reviews.llvm.org/D50478
Files:
lldb/include/lldb/Symbol/Block.h
lldb/include/lldb/Symbol/Function.h
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Target/StackFrame.h
lldb/include/lldb/Target/StackFrameList.h
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/TestAmbiguousTailCallSeq1.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/TestAmbiguousTailCallSeq2.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/source/Symbol/Block.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Target/StackFrameList.cpp
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -27,6 +27,7 @@
#include "lldb/Target/Thread.h"
#include "lldb/Target/Unwind.h"
#include "lldb/Utility/Log.h"
+#include "llvm/ADT/SmallPtrSet.h"
//#define DEBUG_STACK_FRAMES 1
@@ -240,6 +241,152 @@
m_frames.resize(num_frames);
}
+/// Find the unique path through the call graph from \p begin (at PC offset
+/// \p call_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,
+ Target &target, addr_t call_pc,
+ std::vector &path,
+ ModuleList &images, Log *log) {
+ LLDB_LOG(log, "Finding frames between {0} and {1}, call-pc={2:x}",
+ begin.GetDisplayName(), end.GetDisplayName(), call_pc);
+
+ // Find a non-tail calling edge with the first return PC greater than the
+ // call PC.
+ auto first_level_edges = begin.GetCallEdges();
+ auto first_edge_it = std::lower_bound(
+ first_level_edges.begin(), first_level_edges.end(), call_pc,
+ [&](const CallEdge &edge, addr_t target_pc) {
+return edge.GetReturnPCAddress(begin, target) < target_pc;
+ });
+ if (first_edge_it == first_level_edges.end())
+return;
+ CallEdge &first_edge = const_cast(*first_edge_it);
+ if (first_edge.GetReturnPCAddress(begin, target) == LLDB_INVALID_ADDRESS)
+return;
+
+ // The first callee may not be resolved, or there may be nothing to fill in.
+ Function *first_callee = first_edge.GetCallee(images);
+ if (!first_callee || first_callee == &end)
+return;
+
+ // Run DFS on the tail-calling edges out of the first callee to find \p end.
+ // Fully explore the set of functions reachable from the first edge via tail
+ // calls in order to detect ambiguous executions.
+ struct DFS {
+std::vector active_path = {};
+std::vector solution_path = {};
+llvm::SmallPtrSet visited_nodes = {};
+bool ambiguous = false;
+Function *end;
+ModuleList
[Lldb-commits] [PATCH] D50751: WIP: Expose FileCheck-style testing within lldb inline tests
vsk created this revision.
This patch isn't quite ready for review. It's a simple proof-of-concept which
shows what it would take to make FileCheck available within lldb inline tests.
I'll kick off a discussion about this on lldb-dev.
https://reviews.llvm.org/D50751
Files:
lldb/packages/Python/lldbsuite/test/lldbtest.py
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -48,6 +48,8 @@
import signal
from subprocess import *
import sys
+import commands
+import tempfile
import time
import traceback
import types
@@ -2143,6 +2145,53 @@
return match_object
+def filecheck(
+self,
+command,
+check_file,
+filecheck_options = '',
+trace = False):
+# Run the command.
+self.runCmd(
+command,
+msg="FileCheck'ing result of `{0}`".format(command))
+
+# Get the error text if there was an error, and the regular text if
not.
+output = self.res.GetOutput() if self.res.Succeeded() \
+else self.res.GetError()
+
+# Write the output to a temporary file.
+input_file = tempfile.NamedTemporaryFile()
+input_file.write(output)
+input_file.flush()
+
+self.assertTrue(len(output) > 0, "No output to FileCheck")
+
+# Assemble the absolute path to the check file.
+check_file_abs = os.path.join(os.path.dirname(self.test_filename),
+check_file)
+
+# Run FileCheck. Source the check lines from the inline test.
+filecheck_bin = os.path.join(os.path.dirname(configuration.compiler),
+'FileCheck')
+filecheck_cmd = "{0} {1} -input-file {2} {3}".format(filecheck_bin,
+check_file_abs, input_file.name, filecheck_options)
+
+if not is_exe(filecheck_bin):
+with recording(self, trace) as sbuf:
+print("warning: FileCheck not found, skipping command:",
+filecheck_cmd, file=sbuf)
+return
+
+(cmd_status, cmd_output) = commands.getstatusoutput(filecheck_cmd)
+
+with recording(self, trace) as sbuf:
+print("filecheck:", filecheck_cmd, file=sbuf)
+
+self.assertTrue(cmd_status == 0,
+"FileCheck failed (code={0}):\n\n{1}".format(cmd_status,
+cmd_output))
+
def expect(
self,
str,
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -48,6 +48,8 @@
import signal
from subprocess import *
import sys
+import commands
+import tempfile
import time
import traceback
import types
@@ -2143,6 +2145,53 @@
return match_object
+def filecheck(
+self,
+command,
+check_file,
+filecheck_options = '',
+trace = False):
+# Run the command.
+self.runCmd(
+command,
+msg="FileCheck'ing result of `{0}`".format(command))
+
+# Get the error text if there was an error, and the regular text if not.
+output = self.res.GetOutput() if self.res.Succeeded() \
+else self.res.GetError()
+
+# Write the output to a temporary file.
+input_file = tempfile.NamedTemporaryFile()
+input_file.write(output)
+input_file.flush()
+
+self.assertTrue(len(output) > 0, "No output to FileCheck")
+
+# Assemble the absolute path to the check file.
+check_file_abs = os.path.join(os.path.dirname(self.test_filename),
+check_file)
+
+# Run FileCheck. Source the check lines from the inline test.
+filecheck_bin = os.path.join(os.path.dirname(configuration.compiler),
+'FileCheck')
+filecheck_cmd = "{0} {1} -input-file {2} {3}".format(filecheck_bin,
+check_file_abs, input_file.name, filecheck_options)
+
+if not is_exe(filecheck_bin):
+with recording(self, trace) as sbuf:
+print("warning: FileCheck not found, skipping command:",
+filecheck_cmd, file=sbuf)
+return
+
+(cmd_status, cmd_output) = commands.getstatusoutput(filecheck_cmd)
+
+with recording(self, trace) as sbuf:
+print("filecheck:", filecheck_cmd, file=sbuf)
+
+self.assertTrue(cmd_status == 0,
+"FileCheck failed (code={0}):\n\n{1}".format(cmd_status,
+cmd_output))
+
def expect(
self,
str,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http:/
[Lldb-commits] [PATCH] D50864: Add libc++ data formatter for std::function
vsk added inline comments.
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:46
+ // Member __f_ has type __base*, the contents of which will either directly
+ // hold a pointer to the callable object or vtable entry which will hold the
+ // type information need to discover the lambda being called.
'or' -> 'or hold a'
'will hold' -> 'points to'
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:47
+ // hold a pointer to the callable object or vtable entry which will hold the
+ // type information need to discover the lambda being called.
+ ValueObjectSP member__f_(
'need' -> 'needed'
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:49
+ ValueObjectSP member__f_(
+ valobj_sp->GetChildMemberWithName(ConstString("__f_"), true));
+ lldb::addr_t member__f_pointer_value = member__f_->GetValueAsUnsigned(0);
Could you explicitly pass in a DynamicValueType into GetChildMemberWithName
instead of true? You should be able to find one from
valobj->GetManager()->GetUseDynamic().
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:55
+
+ // We may need to increment pointers into __f_ so we need to know the size
+ uint32_t address_size = process->GetAddressByteSize();
The address size is a generically useful thing to have around, there's no need
to comment about defining this.
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:58
+
+ if (process != nullptr) {
+Status status;
Please use an early exit here, and everywhere else in the function where it's
possible to do so. If the process has really disappeared, there's no need to
print anything. In general there's no need to present an incomplete result from
this formatter.
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:71
+lldb::addr_t address_after_vtable = member__f_pointer_value + address_size;
+lldb::addr_t possible_function_address =
+process->ReadPointerFromMemory(address_after_vtable, status);
When you say "possible", what do you mean? Could you add a comment?
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:72
+lldb::addr_t possible_function_address =
+process->ReadPointerFromMemory(address_after_vtable, status);
+
Do you need to check `status.Fail()` here?
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:76
+
+if (!target.GetSectionLoadList().IsEmpty()) {
+ Address vtable_addr_resolved;
Before doing anything related to SectionLoadList, it would help to have a short
comment explaining the intent of the code. Looking through it, it seems like
it'd be even better if split out into a helper function.
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:80
+ Symbol *symbol;
+
+ if (target.GetSectionLoadList().ResolveLoadAddress(
Instead of attempting to parse symbol names, which is inherently brittle and
complicated, why not get the base address of the callable and look up it's
location via the symbol table? You'd lose some precision in being able to
disambiguate lambdas vs. regular functions, but that's a great tradeoff to
make, because the code would be simpler and more reliable.
https://reviews.llvm.org/D50864
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50997: Automatically set path to sanitizer runtime when running tests on macOS.
vsk added a comment. Thanks so much for doing this! Comment at: lit/Suite/lit.cfg:28 + resource_dir = subprocess.check_output(config.cmake_cxx_compiler + + ' -print-resource-dir', shell=True) + runtime = os.path.join(resource_dir[:-1], This might be easier to read as either `import commands; commands.getoutput(...)` or `check_output(..., shell=True).strip()`. Then it's possible to drop the [:-1], which is nice, because it's not clear that that strips a trailing newline. Comment at: lit/Suite/lit.cfg:31 + 'lib/darwin/libclang_rt.asan_osx_dynamic.dylib') + config.environment['ASAN_OPTIONS'] = 'detect_stack_use-after_return=1' + config.environment['DYLD_INSERT_LIBRARIES'] = runtime The dash should be an underscore, and lldb hits false positives when the container overflow check is enabled. How about: `ASAN_OPTIONS=detect_stack_use_after_return=1:container_overflow=0`? https://reviews.llvm.org/D50997 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50997: Automatically set path to sanitizer runtime when running tests on macOS.
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. (LGTM with the second comment addressed.) https://reviews.llvm.org/D50997 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50481: Fix broken builtin functions in the expression command
vsk accepted this revision as: vsk. vsk added a comment. This revision is now accepted and ready to land. Thanks, LGTM! Repository: rLLDB LLDB https://reviews.llvm.org/D50481 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51185: Reuse the SelectorTable from Clang's Preprocessor
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. LGTM. This is NFC, it seems. There's a FIXME in Preprocessor.h about the lifetime of SelectorTable eventually not being tied to Preprocessor, but this is correct for now. Repository: rLLDB LLDB https://reviews.llvm.org/D51185 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk added a comment. Ping. https://reviews.llvm.org/D50478 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk added inline comments.
Comment at: lldb/include/lldb/Symbol/Function.h:331
+ /// \ref resolved.
+ union {
+const char *mangled_name;
aprantl wrote:
> `llvm::PointerUnion` ?
It's not possible to use PointerUnion here because `const char *` has 1-byte
alignment. There's no space in the pointer to store a discriminator bit.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3757
+ // For now, assume that all entries are nested directly under the subprogram
+ // (this is the kind of DWARF LLVM produces) and parse them eagerly.
+ std::vector call_edges;
sgraenitz wrote:
> Does any of the tests fail in case LLVM DWARFs change? Or is it very unlikely?
Yes, the tests in this patch would fail if TAG_call_site entries were nested
within the nearest enclosing lexical block (as specified by DWARF 5). We would
have a heads-up about such a breaking change. That said, it's unlikely to catch
us by surprise, because we control the LLVM DWARF producer (see
https://reviews.llvm.org/D49887).
https://reviews.llvm.org/D50478
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk updated this revision to Diff 162416.
vsk marked 4 inline comments as done.
vsk added a comment.
Thanks for the feedback!
https://reviews.llvm.org/D50478
Files:
lldb/include/lldb/Symbol/Block.h
lldb/include/lldb/Symbol/Function.h
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Target/StackFrame.h
lldb/include/lldb/Target/StackFrameList.h
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/TestAmbiguousTailCallSeq1.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/TestAmbiguousTailCallSeq2.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/source/Symbol/Block.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Target/StackFrameList.cpp
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -27,6 +27,7 @@
#include "lldb/Target/Thread.h"
#include "lldb/Target/Unwind.h"
#include "lldb/Utility/Log.h"
+#include "llvm/ADT/SmallPtrSet.h"
//#define DEBUG_STACK_FRAMES 1
@@ -240,6 +241,152 @@
m_frames.resize(num_frames);
}
+/// Find the unique path through the call graph from \p begin (at PC offset
+/// \p call_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,
+ Target &target, addr_t call_pc,
+ std::vector &path,
+ ModuleList &images, Log *log) {
+ LLDB_LOG(log, "Finding frames between {0} and {1}, call-pc={2:x}",
+ begin.GetDisplayName(), end.GetDisplayName(), call_pc);
+
+ // Find a non-tail calling edge with the first return PC greater than the
+ // call PC.
+ auto first_level_edges = begin.GetCallEdges();
+ auto first_edge_it = std::lower_bound(
+ first_level_edges.begin(), first_level_edges.end(), call_pc,
+ [&](const CallEdge &edge, addr_t target_pc) {
+return edge.GetReturnPCAddress(begin, target) < target_pc;
+ });
+ if (first_edge_it == first_level_edges.end())
+return;
+ CallEdge &first_edge = const_cast(*first_edge_it);
+ if (first_edge.GetReturnPCAddress(begin, target) == LLDB_INVALID_ADDRESS)
+return;
+
+ // The first callee may not be resolved, or there may be nothing to fill in.
+ Function *first_callee = first_edge.GetCallee(images);
+ if (!first_callee || first_callee == &end)
+return;
+
+ // Run DFS on the tail-calling edges out of the first callee to find \p end.
+ // Fully explore the set of functions reachable from the first edge via tail
+ // calls in order to detect ambiguous executions.
+ struct DFS {
+std::vector active_path = {};
+std::vector solution_path = {};
+llvm::SmallPtrSet visited_nodes = {};
+bool ambiguous = false;
+Function *end;
+ModuleList &images;
+
+DFS(Function *end, ModuleList &images) : end(end), images(images) {}
+
+void search(Function *first_callee, std::vector &path) {
+ dfs(first_callee);
+ if (!ambiguous)
+path = std::move(solution_path);
+}
+
+void dfs(Function *callee) {
+ // Found a path to the target
[Lldb-commits] [PATCH] D50751: Allow use of self.filecheck in LLDB tests (c.f self.expect)
vsk updated this revision to Diff 162491.
vsk retitled this revision from "WIP: Expose FileCheck-style testing within
lldb inline tests" to "Allow use of self.filecheck in LLDB tests (c.f
self.expect)".
vsk edited the summary of this revision.
vsk added reviewers: teemperor, aprantl, zturner.
vsk added a project: LLDB.
vsk removed a subscriber: zturner.
https://reviews.llvm.org/D50751
Files:
lldb/packages/Python/lldbsuite/test/configuration.py
lldb/packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/typedef_array/main.cpp
lldb/packages/Python/lldbsuite/test/lldbtest.py
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -48,6 +48,8 @@
import signal
from subprocess import *
import sys
+import commands
+import tempfile
import time
import traceback
import types
@@ -2145,6 +2147,48 @@
return match_object
+def filecheck(
+self,
+command,
+check_file,
+filecheck_options = '',
+trace = False):
+# Run the command.
+self.runCmd(
+command,
+msg="FileCheck'ing result of `{0}`".format(command))
+
+# Get the error text if there was an error, and the regular text if not.
+output = self.res.GetOutput() if self.res.Succeeded() \
+else self.res.GetError()
+
+# Write the output to a temporary file.
+input_file = tempfile.NamedTemporaryFile()
+input_file.write(output)
+input_file.flush()
+
+# Assemble the absolute path to the check file. As a convenience for
+# LLDB inline tests, assume that the check file is a relative path to
+# a file within the inline test directory.
+if hasattr(self, 'test_filename'):
+check_file_abs = os.path.join(os.path.dirname(self.test_filename),
+check_file)
+else:
+check_file_abs = os.path.abspath(check_file)
+
+# Run FileCheck.
+filecheck_bin = configuration.get_filecheck_path()
+filecheck_cmd = "{0} {1} -input-file {2} {3}".format(filecheck_bin,
+check_file_abs, input_file.name, filecheck_options)
+with recording(self, trace) as sbuf:
+print("filecheck:", filecheck_cmd, file=sbuf)
+
+(cmd_status, cmd_output) = commands.getstatusoutput(filecheck_cmd)
+
+self.assertTrue(cmd_status == 0,
+"FileCheck failed (code={0}):\n\n{1}\n\n{2}".format(cmd_status,
+cmd_output, output))
+
def expect(
self,
str,
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/typedef_array/main.cpp
===
--- lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/typedef_array/main.cpp
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/typedef_array/main.cpp
@@ -9,6 +9,11 @@
typedef int Foo;
int main() {
+// CHECK: (Foo [3]) array = {
+// CHECK-NEXT: (Foo) [0] = 1
+// CHECK-NEXT: (Foo) [1] = 2
+// CHECK-NEXT: (Foo) [2] = 3
+// CHECK-NEXT: }
Foo array[3] = {1,2,3};
-return 0; //% self.expect("frame variable array --show-types --", substrs = ['(Foo [3]) array = {','(Foo) [0] = 1','(Foo) [1] = 2','(Foo) [2] = 3'])
+return 0; //% self.filecheck("frame variable array --show-types --", 'main.cpp')
}
Index: lldb/packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
===
--- lldb/packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
+++ lldb/packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
@@ -57,14 +57,21 @@
self.runCmd("frame variable foo1.b --show-types")
self.runCmd("frame variable foo1.b.b_ref --show-types")
-self.expect(
-"expression --show-types -- *(new foo(47))",
-substrs=[
-'(int) a = 47',
-'(bar) b = {',
-'(int) i = 94',
-'(baz) b = {',
-'(int) k = 99'])
+self.filecheck("expression --show-types -- *(new foo(47))", __file__,
+'-check-prefix=EXPR-TYPES-NEW-FOO')
+# EXPR-TYPES-NEW-FOO: (foo) ${{.*}} = {
+# EXPR-TYPES-NEW-FOO-NEXT: (int) a = 47
+# EXPR-TYPES-NEW-FOO-NEXT: (int *) a_ptr = 0x
+# EXPR-TYPES-NEW-FOO-NEXT: (bar) b = {
+# EXPR-TYPES-NEW-FOO-NEXT: (int) i = 94
+# EXPR-TYPES-NEW-FOO-NEXT: (int *) i_ptr = 0x
+# EXPR-TYPES-NEW-FOO-NEXT: (baz) b = {
+# EXPR-TYPES-NEW-FOO-NEXT: (int) h =
[Lldb-commits] [PATCH] D51319: Use a RAII guard to lock/unlock DisassemblerLLVMC [NFC]
vsk added inline comments.
Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h:81
+ struct Guard {
+DisassemblerLLVMC *m_instance;
+Guard(DisassemblerLLVMC *instance, InstructionLLVMC *inst,
This is nice. Do you think it might be even safer to have the guard own the
disassembler shared_ptr instance? Users could then access the disassembler via
the guard's operator->, and there's less chance of the shared_ptr escaping /
being abused. We could even have GetDisassembler() return a guard instance.
Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h:85
+: m_instance(instance) {
+ m_instance->Lock(inst, exe_ctx);
+}
It doesn't make sense to me that DisassemblerLLVMC's "Lock" method conflates
locking and initialization. If you decide to have a guard own the disassembler
instance, it'd make sense to split the initialization step out.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D51319
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51319: Use a RAII guard to control access to DisassemblerLLVMC.
vsk accepted this revision.
vsk added a comment.
Looks great, thanks!
Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp:176
bool got_op = false;
-std::shared_ptr disasm_sp(GetDisassembler());
-if (disasm_sp) {
- const ArchSpec &arch = disasm_sp->GetArchitecture();
+DisassemblerScope disasm(*this);
+if (disasm) {
Were all callers into Decode locking the disasm instance prior to this patch?
If not, this is a sweet fix.
https://reviews.llvm.org/D51319
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant
vsk added inline comments.
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py:70
+self.expect("frame variable v_no_value",
+substrs=['v_no_value = No Value'])
Could you add a test which inspects a reference to a variant (to cover the "((
)?&)?" bit you're matching)?
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp:29
+std::variant v3;
+std::variant v_no_value;
+
Does a std::variant containing a std::variant work?
https://reviews.llvm.org/D51520
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51930: Add a basic test for 'memory region'
vsk added a comment. I think it'd be useful to test the driver output specifically. The kind of testing lldb-test facilitates might not be a good fit here (too low-level). Repository: rLLDB LLDB https://reviews.llvm.org/D51930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51930: Add a basic test for 'memory region'
vsk added a comment. This LGTM. Davide? Repository: rLLDB LLDB https://reviews.llvm.org/D51930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant
vsk added a comment. Please clang-format your diffs. https://reviews.llvm.org/D51520 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50751: Allow use of self.filecheck in LLDB tests (c.f self.expect)
vsk updated this revision to Diff 165613.
vsk marked 6 inline comments as done.
vsk added a comment.
Herald added a subscriber: mgorny.
Sorry for the delay, I was busy with other work. I think I've addressed the
review feedback. PTAL.
https://reviews.llvm.org/D50751
Files:
lldb/CMakeLists.txt
lldb/lldb.xcodeproj/project.pbxproj
lldb/packages/Python/lldbsuite/test/configuration.py
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/dotest_args.py
lldb/packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/typedef_array/main.cpp
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/test/CMakeLists.txt
Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -49,6 +49,7 @@
list(APPEND LLDB_TEST_COMMON_ARGS
--executable ${LLDB_TEST_EXECUTABLE}
--dsymutil ${LLDB_TEST_DSYMUTIL}
+ --filecheck ${LLDB_TEST_FILECHECK}
-C ${LLDB_TEST_C_COMPILER}
)
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -48,6 +48,7 @@
import signal
from subprocess import *
import sys
+import tempfile
import time
import traceback
import types
@@ -2214,6 +2215,54 @@
compare_string, msg=COMPLETION_MSG(
str_input, p, match_strings), exe=False, patterns=[p])
+def filecheck(
+self,
+command,
+check_file,
+filecheck_options = ''):
+# Run the command.
+self.runCmd(
+command,
+msg="FileCheck'ing result of `{0}`".format(command))
+
+# Get the error text if there was an error, and the regular text if not.
+output = self.res.GetOutput() if self.res.Succeeded() \
+else self.res.GetError()
+
+# Assemble the absolute path to the check file. As a convenience for
+# LLDB inline tests, assume that the check file is a relative path to
+# a file within the inline test directory.
+if check_file.endswith('.pyc'):
+check_file = check_file[:-1]
+if hasattr(self, 'test_filename'):
+check_file_abs = os.path.join(os.path.dirname(self.test_filename),
+check_file)
+else:
+check_file_abs = os.path.abspath(check_file)
+
+# Run FileCheck.
+filecheck_bin = configuration.get_filecheck_path()
+filecheck_args = [filecheck_bin, check_file_abs]
+if filecheck_options:
+filecheck_args.append(filecheck_options)
+subproc = Popen(filecheck_args, stdin=PIPE, stdout=PIPE, stderr=PIPE)
+cmd_stdout, cmd_stderr = subproc.communicate(input=output)
+cmd_status = subproc.returncode
+
+if cmd_status != 0:
+filecheck_cmd = " ".join(filecheck_args)
+self.assertTrue(cmd_status == 0, """
+--- FileCheck failed (code={0}) ---
+{1}
+
+FileCheck input:
+{2}
+
+FileCheck output:
+{3}
+{4}
+""".format(cmd_status, filecheck_cmd, output, cmd_stdout, cmd_stderr))
+
def expect(
self,
str,
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/typedef_array/main.cpp
===
--- lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/typedef_array/main.cpp
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/typedef_array/main.cpp
@@ -9,6 +9,11 @@
typedef int Foo;
int main() {
+// CHECK: (Foo [3]) array = {
+// CHECK-NEXT: (Foo) [0] = 1
+// CHECK-NEXT: (Foo) [1] = 2
+// CHECK-NEXT: (Foo) [2] = 3
+// CHECK-NEXT: }
Foo array[3] = {1,2,3};
-return 0; //% self.expect("frame variable array --show-types --", substrs = ['(Foo [3]) array = {','(Foo) [0] = 1','(Foo) [1] = 2','(Foo) [2] = 3'])
+return 0; //% self.filecheck("frame variable array --show-types --", 'main.cpp')
}
Index: lldb/packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
===
--- lldb/packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
+++ lldb/packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
@@ -57,14 +57,21 @@
self.runCmd("frame variable foo1.b --show-types")
self.runCmd("frame variable foo1.b.b_ref --show-types")
-self.expect(
-"expression --show-types -- *(new foo(47))",
-substrs=[
-'(int) a = 47',
-'(bar) b = {',
-'(int) i = 94',
-'(baz) b = {',
-'(int) k = 99'])
+
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk updated this revision to Diff 166378.
vsk added a comment.
I've added SB API support (SBFrame::IsArtificial), a SB API test, fleshed out
the remaining tests, and rebased. PTAL, thanks!
https://reviews.llvm.org/D50478
Files:
lldb/include/lldb/API/SBFrame.h
lldb/include/lldb/Core/FormatEntity.h
lldb/include/lldb/Symbol/Block.h
lldb/include/lldb/Symbol/Function.h
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Target/StackFrame.h
lldb/include/lldb/Target/StackFrameList.h
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/TestAmbiguousTailCallSeq1.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/TestAmbiguousTailCallSeq2.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
lldb/scripts/interface/SBFrame.i
lldb/source/API/SBFrame.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/FormatEntity.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/source/Symbol/Block.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Target/StackFrame.cpp
lldb/source/Target/StackFrameList.cpp
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -27,6 +27,7 @@
#include "lldb/Target/Thread.h"
#include "lldb/Target/Unwind.h"
#include "lldb/Utility/Log.h"
+#include "llvm/ADT/SmallPtrSet.h"
//#define DEBUG_STACK_FRAMES 1
@@ -240,6 +241,162 @@
m_frames.resize(num_frames);
}
+/// Find the unique path through the call graph from \p begin (at PC offset
+/// \p call_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,
+ Target &target, addr_t call_pc,
+ std::vector &path,
+ ModuleList &images, Log *log) {
+ LLDB_LOG(log, "Finding frames between {0} and {1}, call-pc={2:x}",
+ begin.GetDisplayName(), end.GetDisplayName(), call_pc);
+
+ // Find a non-tail calling edge with the first return PC greater than the
+ // call PC.
+ auto first_level_edges = begin.GetCallEdges();
+ auto first_edge_it = std::lower_bound(
+ first_level_edges.begin(), first_level_edges.end(), call_pc,
+ [&](const CallEdge &edge, addr_t target_pc) {
+return edge.GetReturnPCAddress(begin, target) < target_pc;
+ });
+ if (first_edge_it == first_level_edges.end())
+return;
+ CallEdge &first_edge = const_cast(*first_edge_it);
+ if (first_edge.GetReturnPCAddress(begin, target) == LLDB_INVALID_ADDRESS)
+return;
+
+ // The first callee may not be resolved, or there may be nothing to fill in.
+ Function *first_callee = first_edge.GetCallee(images);
+ if (!first_callee || first_callee == &end)
+return;
+
+ // Run DFS on the tail-calling edges out of the first callee to find \p end.
+ // Fully explore the set of functions
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk planned changes to this revision. vsk added a comment. In https://reviews.llvm.org/D50478#1241264, @jingham wrote: > Can you add a test that makes sure that when you stop in a frame that has > artificial frames above it, and then you do "finish", or "step out" past the > end of frame 0, the presence of the artificial frame doesn't confuse us? I > am pretty sure that will just work, but it would be good to make sure that it > actually does. > > Really "step out past the end of the frame" isn't really necessary, since > that doesn't pay any attention to the frames above it, it just runs the code > till the frame ID changes. But "finish" does look at the parent frame, so it > might get confused. Great suggestion. The 'finish' command doesn't appear to understand artificial frames, and allows program execution to continue past the point where a tail call is made. I'll try and teach it to "look past" artificial frames. https://reviews.llvm.org/D50478 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames
vsk updated this revision to Diff 166394.
vsk added a comment.
Teach SBThread::StepOut and SBThread::ReturnFromFrame to behave as-if
artificial frames were not present.
This preserves the current behavior of "finish" and "thread return". The
alternatives -- stepping out into an artificial frame or issuing errors --
don't appear to be better. The newly-added thread_step_out_or_return test
covers this.
https://reviews.llvm.org/D50478
Files:
lldb/include/lldb/API/SBFrame.h
lldb/include/lldb/Core/FormatEntity.h
lldb/include/lldb/Symbol/Block.h
lldb/include/lldb/Symbol/Function.h
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Target/StackFrame.h
lldb/include/lldb/Target/StackFrameList.h
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/TestAmbiguousTailCallSeq1.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq1/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/TestAmbiguousTailCallSeq2.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/ambiguous_tail_call_seq2/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_call_site/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/disambiguate_tail_call_seq/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/sbapi_support/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/TestSteppingOutWithArtificialFrames.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/thread_step_out_or_return/main.cpp
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
lldb/packages/Python/lldbsuite/test/functionalities/tail_call_frames/unambiguous_sequence/main.cpp
lldb/scripts/interface/SBFrame.i
lldb/source/API/SBFrame.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/FormatEntity.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/source/Symbol/Block.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Target/StackFrame.cpp
lldb/source/Target/StackFrameList.cpp
lldb/source/Target/ThreadPlanStepOut.cpp
Index: lldb/source/Target/ThreadPlanStepOut.cpp
===
--- lldb/source/Target/ThreadPlanStepOut.cpp
+++ lldb/source/Target/ThreadPlanStepOut.cpp
@@ -48,26 +48,42 @@
m_return_addr(LLDB_INVALID_ADDRESS), m_stop_others(stop_others),
m_immediate_step_from_function(nullptr),
m_calculate_return_value(gather_return_value) {
+ Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
SetFlagsToDefault();
SetupAvoidNoDebug(step_out_avoids_code_without_debug_info);
m_step_from_insn = m_thread.GetRegisterContext()->GetPC(0);
- StackFrameSP return_frame_sp(m_thread.GetStackFrameAtIndex(frame_idx + 1));
+ uint32_t return_frame_index = frame_idx + 1;
+ StackFrameSP return_frame_sp(
+ m_thread.GetStackFrameAtIndex(return_frame_index));
StackFrameSP immediate_return_from_sp(
m_thread.GetStackFrameAtIndex(frame_idx));
if (!return_frame_sp || !immediate_return_from_sp)
return; // we can't do anything here. ValidatePlan() will return false.
+ // While stepping out, behave as-if artificial frames are not present.
+ while (return_frame_sp->IsArtificial()) {
+++return_frame_index;
+return_frame_sp = m_thread.GetStackFrameAtIndex(return_frame_index);
+
+// We never expect to see a
[Lldb-commits] [PATCH] D40745: Add a clang-ast subcommand to lldb-test
vsk added a comment. The general approach sgtm, and the patch itself looks reasonable. https://reviews.llvm.org/D40745 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40757: Disable warnings related to anonymous types
vsk created this revision.
Herald added a subscriber: mgorny.
Various part of lldb make use of anonymous structs and unions. In every case
I've seen the usage is idiomatic, and doesn't deserve a warning.
For example, logic in the NSDictionary and NSSet plugins use anonymous structs
in a manner consistent with the relevant Apple frameworks.
https://reviews.llvm.org/D40757
Files:
cmake/modules/LLDBConfig.cmake
Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -231,6 +231,18 @@
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-vla-extension")
endif ()
+check_cxx_compiler_flag("-Wno-gnu-anonymous-struct"
+CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+if (CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-gnu-anonymous-struct")
+endif ()
+
+check_cxx_compiler_flag("-Wno-nested-anon-types"
+CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+if (CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-nested-anon-types")
+endif ()
+
# Disable MSVC warnings
if( MSVC )
add_definitions(
Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -231,6 +231,18 @@
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-vla-extension")
endif ()
+check_cxx_compiler_flag("-Wno-gnu-anonymous-struct"
+CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+if (CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-gnu-anonymous-struct")
+endif ()
+
+check_cxx_compiler_flag("-Wno-nested-anon-types"
+CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+if (CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-nested-anon-types")
+endif ()
+
# Disable MSVC warnings
if( MSVC )
add_definitions(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40757: Disable warnings related to anonymous types
vsk added a comment.
In https://reviews.llvm.org/D40757#943483, @labath wrote:
> If the "excuse" for not following llvm here is that these structs mirror
> apple headers, should we restrict these warnings only to the relevant files
> (e.g. everything in `source/Plugins/Language/ObjC`)?
>
> I don't particularly care about whether we do, but I wanted to throw the idea
> out there, as this is the reason why I haven't done something similar yet
> (although the warnings are quite annoying).
I'd like to make this a narrower change as well, but unfortunately, I haven't
found a way to add in extra CXX flags through the add_library or
llvm_add_library utilities. Somebody tried to do this once in AddLLDB.cmake
(see "set(CMAKE_CXX_FLAGS ...)"), but it's busted, and does not do anything,
AFAICT.
In this case, I think it should be safe to disable these warnings project-wide,
because they're not often (ever?) immediate indicators of correctness issues /
undefined behavior. Given all of that, @labath are you OK with the patch as-is?
Comment at: cmake/modules/LLDBConfig.cmake:241
+check_cxx_compiler_flag("-Wno-nested-anon-types"
+CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+if (CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
I should point out that there's a typo here -- I've fixed this so it's spelled
CXX_SUPPORTS_NO_NESTED_ANON_TYPES locally.
https://reviews.llvm.org/D40757
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40812: Remove no-op null checks, NFC
vsk created this revision.
Null-checking functions which aren't marked 'weak_import' is a no-op
(the compiler rewrites the check to 'true'), regardless of whether a
library providing its definition is weak-linked.
Remove the no-op checks to clean up the code and silence
-Wpointer-to-bool.
https://reviews.llvm.org/D40812
Files:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1022,10 +1022,7 @@
std::string avail_name;
#if defined(HAVE_LIBCOMPRESSION)
- // libcompression is weak linked so test if compression_decode_buffer() is
- // available
- if (compression_decode_buffer != NULL &&
- avail_type == CompressionType::None) {
+ if (avail_type == CompressionType::None) {
for (auto compression : supported_compressions) {
if (compression == "lzfse") {
avail_type = CompressionType::LZFSE;
@@ -1037,10 +1034,7 @@
#endif
#if defined(HAVE_LIBCOMPRESSION)
- // libcompression is weak linked so test if compression_decode_buffer() is
- // available
- if (compression_decode_buffer != NULL &&
- avail_type == CompressionType::None) {
+ if (avail_type == CompressionType::None) {
for (auto compression : supported_compressions) {
if (compression == "zlib-deflate") {
avail_type = CompressionType::ZlibDeflate;
@@ -1064,10 +1058,7 @@
#endif
#if defined(HAVE_LIBCOMPRESSION)
- // libcompression is weak linked so test if compression_decode_buffer() is
- // available
- if (compression_decode_buffer != NULL &&
- avail_type == CompressionType::None) {
+ if (avail_type == CompressionType::None) {
for (auto compression : supported_compressions) {
if (compression == "lz4") {
avail_type = CompressionType::LZ4;
@@ -1079,10 +1070,7 @@
#endif
#if defined(HAVE_LIBCOMPRESSION)
- // libcompression is weak linked so test if compression_decode_buffer() is
- // available
- if (compression_decode_buffer != NULL &&
- avail_type == CompressionType::None) {
+ if (avail_type == CompressionType::None) {
for (auto compression : supported_compressions) {
if (compression == "lzma") {
avail_type = CompressionType::LZMA;
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1022,10 +1022,7 @@
std::string avail_name;
#if defined(HAVE_LIBCOMPRESSION)
- // libcompression is weak linked so test if compression_decode_buffer() is
- // available
- if (compression_decode_buffer != NULL &&
- avail_type == CompressionType::None) {
+ if (avail_type == CompressionType::None) {
for (auto compression : supported_compressions) {
if (compression == "lzfse") {
avail_type = CompressionType::LZFSE;
@@ -1037,10 +1034,7 @@
#endif
#if defined(HAVE_LIBCOMPRESSION)
- // libcompression is weak linked so test if compression_decode_buffer() is
- // available
- if (compression_decode_buffer != NULL &&
- avail_type == CompressionType::None) {
+ if (avail_type == CompressionType::None) {
for (auto compression : supported_compressions) {
if (compression == "zlib-deflate") {
avail_type = CompressionType::ZlibDeflate;
@@ -1064,10 +1058,7 @@
#endif
#if defined(HAVE_LIBCOMPRESSION)
- // libcompression is weak linked so test if compression_decode_buffer() is
- // available
- if (compression_decode_buffer != NULL &&
- avail_type == CompressionType::None) {
+ if (avail_type == CompressionType::None) {
for (auto compression : supported_compressions) {
if (compression == "lz4") {
avail_type = CompressionType::LZ4;
@@ -1079,10 +1070,7 @@
#endif
#if defined(HAVE_LIBCOMPRESSION)
- // libcompression is weak linked so test if compression_decode_buffer() is
- // available
- if (compression_decode_buffer != NULL &&
- avail_type == CompressionType::None) {
+ if (avail_type == CompressionType::None) {
for (auto compression : supported_compressions) {
if (compression == "lzma") {
avail_type = CompressionType::LZMA;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40812: Remove no-op null checks, NFC
vsk added a comment. Ah, sorry, I should have mentioned that the header vending compression_decode_buffer() does not provide the 'weak_import' attribute (as far as I can see!). If you change your example to drop 'weak_import' from foo, I think it will be closer to the current situation. https://reviews.llvm.org/D40812 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40821: Fix const-correctness in RegisterContext methods, NFC
vsk created this revision.
Herald added a subscriber: javed.absar.
A few methods in RegisterContext classes accept const objects which are
cast to a non-const thread_state_t. Instead of dropping const-ness, it
might be simpler and more ergonomic to just not mark the objects const.
This fixes a slew of warnings.
One alternative is to drop const-ness in a C++-friendly way, e.g:
const_cast(static_cast(&X))
This would still leave us in a situation where some RegisterContext
methods accept const objects and others don't, and it doesn't seem
feasible to apply const everywhere here, because of non-const methods
like SetRegisterDataFrom_LC_THREAD.
https://reviews.llvm.org/D40821
Files:
source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
source/Plugins/Process/MacOSX-Kernel/RegisterContextKDP_arm.cpp
source/Plugins/Process/MacOSX-Kernel/RegisterContextKDP_arm.h
source/Plugins/Process/MacOSX-Kernel/RegisterContextKDP_arm64.cpp
source/Plugins/Process/MacOSX-Kernel/RegisterContextKDP_arm64.h
source/Plugins/Process/MacOSX-Kernel/RegisterContextKDP_i386.cpp
source/Plugins/Process/MacOSX-Kernel/RegisterContextKDP_i386.h
source/Plugins/Process/MacOSX-Kernel/RegisterContextKDP_x86_64.cpp
source/Plugins/Process/MacOSX-Kernel/RegisterContextKDP_x86_64.h
source/Plugins/Process/Utility/RegisterContextDarwin_arm.h
source/Plugins/Process/Utility/RegisterContextDarwin_arm64.h
source/Plugins/Process/Utility/RegisterContextDarwin_i386.h
source/Plugins/Process/Utility/RegisterContextDarwin_x86_64.h
source/Plugins/Process/Utility/RegisterContextMach_arm.cpp
source/Plugins/Process/Utility/RegisterContextMach_arm.h
source/Plugins/Process/Utility/RegisterContextMach_i386.cpp
source/Plugins/Process/Utility/RegisterContextMach_i386.h
source/Plugins/Process/Utility/RegisterContextMach_x86_64.cpp
source/Plugins/Process/Utility/RegisterContextMach_x86_64.h
Index: source/Plugins/Process/Utility/RegisterContextMach_x86_64.h
===
--- source/Plugins/Process/Utility/RegisterContextMach_x86_64.h
+++ source/Plugins/Process/Utility/RegisterContextMach_x86_64.h
@@ -31,11 +31,11 @@
int DoReadEXC(lldb::tid_t tid, int flavor, EXC &exc);
- int DoWriteGPR(lldb::tid_t tid, int flavor, const GPR &gpr);
+ int DoWriteGPR(lldb::tid_t tid, int flavor, GPR &gpr);
- int DoWriteFPU(lldb::tid_t tid, int flavor, const FPU &fpu);
+ int DoWriteFPU(lldb::tid_t tid, int flavor, FPU &fpu);
- int DoWriteEXC(lldb::tid_t tid, int flavor, const EXC &exc);
+ int DoWriteEXC(lldb::tid_t tid, int flavor, EXC &exc);
};
#endif // liblldb_RegisterContextMach_x86_64_h_
Index: source/Plugins/Process/Utility/RegisterContextMach_x86_64.cpp
===
--- source/Plugins/Process/Utility/RegisterContextMach_x86_64.cpp
+++ source/Plugins/Process/Utility/RegisterContextMach_x86_64.cpp
@@ -45,17 +45,17 @@
}
int RegisterContextMach_x86_64::DoWriteGPR(lldb::tid_t tid, int flavor,
- const GPR &gpr) {
+ GPR &gpr) {
return ::thread_set_state(tid, flavor, (thread_state_t)&gpr, GPRWordCount);
}
int RegisterContextMach_x86_64::DoWriteFPU(lldb::tid_t tid, int flavor,
- const FPU &fpu) {
+ FPU &fpu) {
return ::thread_set_state(tid, flavor, (thread_state_t)&fpu, FPUWordCount);
}
int RegisterContextMach_x86_64::DoWriteEXC(lldb::tid_t tid, int flavor,
- const EXC &exc) {
+ EXC &exc) {
return ::thread_set_state(tid, flavor, (thread_state_t)&exc, EXCWordCount);
}
Index: source/Plugins/Process/Utility/RegisterContextMach_i386.h
===
--- source/Plugins/Process/Utility/RegisterContextMach_i386.h
+++ source/Plugins/Process/Utility/RegisterContextMach_i386.h
@@ -30,11 +30,11 @@
int DoReadEXC(lldb::tid_t tid, int flavor, EXC &exc);
- int DoWriteGPR(lldb::tid_t tid, int flavor, const GPR &gpr);
+ int DoWriteGPR(lldb::tid_t tid, int flavor, GPR &gpr);
- int DoWriteFPU(lldb::tid_t tid, int flavor, const FPU &fpu);
+ int DoWriteFPU(lldb::tid_t tid, int flavor, FPU &fpu);
- int DoWriteEXC(lldb::tid_t tid, int flavor, const EXC &exc);
+ int DoWriteEXC(lldb::tid_t tid, int flavor, EXC &exc);
};
#endif // liblldb_RegisterContextMach_i386_h_
Index: source/Plugins/Process/Utility/RegisterContextMach_i386.cpp
===
--- source/Plugins/Process/Utility/RegisterContextMach_i386.cpp
+++ source/Plugins/Process/Utility/RegisterContextMach_i386.cpp
@@ -42,17 +42,17 @@
}
int RegisterContextMach_i386::DoWriteGPR(lldb::tid_t tid, int flavor,
- const GPR &gpr) {
+
[Lldb-commits] [PATCH] D40821: Fix const-correctness in RegisterContext methods, NFC
vsk added a comment. In https://reviews.llvm.org/D40821#945314, @clayborg wrote: > Seems wrong to remove the const on structs that don't need to change in order > to make the write happen. Can't we just quiet the warnings with a const_cast > inside the function call? Absolutely. I opted for dropping const to make the DoRead* and DoWrite* methods feel consistent. At first glance, it looked like there may be methods in RegisterContext which expect non-const GPR/FPU/etc objects, so it seemed reasonable to drop const. I'm open to just adding in the const_cast(static_cast(...)) pattern as needed to suppress warnings, and to constifying the rest of RegisterContext as a follow-up. Let me know what you'd prefer. https://reviews.llvm.org/D40821 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40821: Fix const-correctness in RegisterContext methods, NFC
vsk updated this revision to Diff 125639.
vsk edited the summary of this revision.
vsk added a comment.
- Address Greg's comments.
https://reviews.llvm.org/D40821
Files:
source/Plugins/Process/Utility/RegisterContextMach_arm.cpp
source/Plugins/Process/Utility/RegisterContextMach_i386.cpp
source/Plugins/Process/Utility/RegisterContextMach_x86_64.cpp
Index: source/Plugins/Process/Utility/RegisterContextMach_x86_64.cpp
===
--- source/Plugins/Process/Utility/RegisterContextMach_x86_64.cpp
+++ source/Plugins/Process/Utility/RegisterContextMach_x86_64.cpp
@@ -46,17 +46,23 @@
int RegisterContextMach_x86_64::DoWriteGPR(lldb::tid_t tid, int flavor,
const GPR &gpr) {
- return ::thread_set_state(tid, flavor, (thread_state_t)&gpr, GPRWordCount);
+ return ::thread_set_state(
+ tid, flavor, reinterpret_cast(const_cast(&gpr)),
+ GPRWordCount);
}
int RegisterContextMach_x86_64::DoWriteFPU(lldb::tid_t tid, int flavor,
const FPU &fpu) {
- return ::thread_set_state(tid, flavor, (thread_state_t)&fpu, FPUWordCount);
+ return ::thread_set_state(
+ tid, flavor, reinterpret_cast(const_cast(&fpu)),
+ FPUWordCount);
}
int RegisterContextMach_x86_64::DoWriteEXC(lldb::tid_t tid, int flavor,
const EXC &exc) {
- return ::thread_set_state(tid, flavor, (thread_state_t)&exc, EXCWordCount);
+ return ::thread_set_state(
+ tid, flavor, reinterpret_cast(const_cast(&exc)),
+ EXCWordCount);
}
#endif
Index: source/Plugins/Process/Utility/RegisterContextMach_i386.cpp
===
--- source/Plugins/Process/Utility/RegisterContextMach_i386.cpp
+++ source/Plugins/Process/Utility/RegisterContextMach_i386.cpp
@@ -43,17 +43,23 @@
int RegisterContextMach_i386::DoWriteGPR(lldb::tid_t tid, int flavor,
const GPR &gpr) {
- return ::thread_set_state(tid, flavor, (thread_state_t)&gpr, GPRWordCount);
+ return ::thread_set_state(
+ tid, flavor, reinterpret_cast(const_cast(&gpr)),
+ GPRWordCount);
}
int RegisterContextMach_i386::DoWriteFPU(lldb::tid_t tid, int flavor,
const FPU &fpu) {
- return ::thread_set_state(tid, flavor, (thread_state_t)&fpu, FPUWordCount);
+ return ::thread_set_state(
+ tid, flavor, reinterpret_cast(const_cast(&fpu)),
+ FPUWordCount);
}
int RegisterContextMach_i386::DoWriteEXC(lldb::tid_t tid, int flavor,
const EXC &exc) {
- return ::thread_set_state(tid, flavor, (thread_state_t)&exc, EXCWordCount);
+ return ::thread_set_state(
+ tid, flavor, reinterpret_cast(const_cast(&exc)),
+ EXCWordCount);
}
#endif
Index: source/Plugins/Process/Utility/RegisterContextMach_arm.cpp
===
--- source/Plugins/Process/Utility/RegisterContextMach_arm.cpp
+++ source/Plugins/Process/Utility/RegisterContextMach_arm.cpp
@@ -50,22 +50,30 @@
int RegisterContextMach_arm::DoWriteGPR(lldb::tid_t tid, int flavor,
const GPR &gpr) {
- return ::thread_set_state(tid, flavor, (thread_state_t)&gpr, GPRWordCount);
+ return ::thread_set_state(
+ tid, flavor, reinterpret_cast(const_cast(&gpr)),
+ GPRWordCount);
}
int RegisterContextMach_arm::DoWriteFPU(lldb::tid_t tid, int flavor,
const FPU &fpu) {
- return ::thread_set_state(tid, flavor, (thread_state_t)&fpu, FPUWordCount);
+ return ::thread_set_state(
+ tid, flavor, reinterpret_cast(const_cast(&fpu)),
+ FPUWordCount);
}
int RegisterContextMach_arm::DoWriteEXC(lldb::tid_t tid, int flavor,
const EXC &exc) {
- return ::thread_set_state(tid, flavor, (thread_state_t)&exc, EXCWordCount);
+ return ::thread_set_state(
+ tid, flavor, reinterpret_cast(const_cast(&exc)),
+ EXCWordCount);
}
int RegisterContextMach_arm::DoWriteDBG(lldb::tid_t tid, int flavor,
const DBG &dbg) {
- return ::thread_set_state(tid, flavor, (thread_state_t)&dbg, DBGWordCount);
+ return ::thread_set_state(
+ tid, flavor, reinterpret_cast(const_cast(&dbg)),
+ DBGWordCount);
}
#endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40757: Disable warnings related to anonymous types in the ObjC plugin
vsk updated this revision to Diff 125816.
vsk retitled this revision from "Disable warnings related to anonymous types"
to "Disable warnings related to anonymous types in the ObjC plugin".
vsk edited the summary of this revision.
vsk added a comment.
- Thanks @labath! I've disabled the GNU warnings in a narrower way. PTAL.
https://reviews.llvm.org/D40757
Files:
cmake/modules/AddLLDB.cmake
cmake/modules/LLDBConfig.cmake
source/Plugins/Language/ObjC/CMakeLists.txt
Index: source/Plugins/Language/ObjC/CMakeLists.txt
===
--- source/Plugins/Language/ObjC/CMakeLists.txt
+++ source/Plugins/Language/ObjC/CMakeLists.txt
@@ -1,3 +1,13 @@
+set(EXTRA_CXXFLAGS "")
+
+if (CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+ set(EXTRA_CXXFLAGS ${EXTRA_CXXFLAGS} -Wno-gnu-anonymous-struct)
+endif ()
+
+if (CXX_SUPPORTS_NO_NESTED_ANON_TYPES)
+ set(EXTRA_CXXFLAGS ${EXTRA_CXXFLAGS} -Wno-nested-anon-types)
+endif ()
+
add_lldb_library(lldbPluginObjCLanguage PLUGIN
ObjCLanguage.cpp
CF.cpp
@@ -21,4 +31,6 @@
lldbTarget
lldbUtility
lldbPluginAppleObjCRuntime
+
+ EXTRA_CXXFLAGS ${EXTRA_CXXFLAGS}
)
Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -231,6 +231,12 @@
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-vla-extension")
endif ()
+check_cxx_compiler_flag("-Wno-gnu-anonymous-struct"
+CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+
+check_cxx_compiler_flag("-Wno-nested-anon-types"
+CXX_SUPPORTS_NO_NESTED_ANON_TYPES)
+
# Disable MSVC warnings
if( MSVC )
add_definitions(
Index: cmake/modules/AddLLDB.cmake
===
--- cmake/modules/AddLLDB.cmake
+++ cmake/modules/AddLLDB.cmake
@@ -4,7 +4,7 @@
cmake_parse_arguments(PARAM
"MODULE;SHARED;STATIC;OBJECT;PLUGIN"
""
-"DEPENDS;LINK_LIBS;LINK_COMPONENTS"
+"EXTRA_CXXFLAGS;DEPENDS;LINK_LIBS;LINK_COMPONENTS"
${ARGN})
llvm_process_sources(srcs ${PARAM_UNPARSED_ARGUMENTS})
list(APPEND LLVM_LINK_COMPONENTS ${PARAM_LINK_COMPONENTS})
@@ -35,6 +35,8 @@
endif()
#PIC not needed on Win
+ # FIXME: Setting CMAKE_CXX_FLAGS here is a no-op, use target_compile_options
+ # or omit this logic instead.
if (NOT WIN32)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fPIC")
endif()
@@ -78,6 +80,9 @@
# headers without negatively impacting much of anything.
add_dependencies(${name} clang-tablegen-targets)
+ # Add in any extra C++ compilation flags for this library.
+ target_compile_options(${name} PRIVATE ${PARAM_EXTRA_CXXFLAGS})
+
set_target_properties(${name} PROPERTIES FOLDER "lldb libraries")
endfunction(add_lldb_library)
Index: source/Plugins/Language/ObjC/CMakeLists.txt
===
--- source/Plugins/Language/ObjC/CMakeLists.txt
+++ source/Plugins/Language/ObjC/CMakeLists.txt
@@ -1,3 +1,13 @@
+set(EXTRA_CXXFLAGS "")
+
+if (CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+ set(EXTRA_CXXFLAGS ${EXTRA_CXXFLAGS} -Wno-gnu-anonymous-struct)
+endif ()
+
+if (CXX_SUPPORTS_NO_NESTED_ANON_TYPES)
+ set(EXTRA_CXXFLAGS ${EXTRA_CXXFLAGS} -Wno-nested-anon-types)
+endif ()
+
add_lldb_library(lldbPluginObjCLanguage PLUGIN
ObjCLanguage.cpp
CF.cpp
@@ -21,4 +31,6 @@
lldbTarget
lldbUtility
lldbPluginAppleObjCRuntime
+
+ EXTRA_CXXFLAGS ${EXTRA_CXXFLAGS}
)
Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -231,6 +231,12 @@
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-vla-extension")
endif ()
+check_cxx_compiler_flag("-Wno-gnu-anonymous-struct"
+CXX_SUPPORTS_NO_GNU_ANONYMOUS_STRUCT)
+
+check_cxx_compiler_flag("-Wno-nested-anon-types"
+CXX_SUPPORTS_NO_NESTED_ANON_TYPES)
+
# Disable MSVC warnings
if( MSVC )
add_definitions(
Index: cmake/modules/AddLLDB.cmake
===
--- cmake/modules/AddLLDB.cmake
+++ cmake/modules/AddLLDB.cmake
@@ -4,7 +4,7 @@
cmake_parse_arguments(PARAM
"MODULE;SHARED;STATIC;OBJECT;PLUGIN"
""
-"DEPENDS;LINK_LIBS;LINK_COMPONENTS"
+"EXTRA_CXXFLAGS;DEPENDS;LINK_LIBS;LINK_COMPONENTS"
${ARGN})
llvm_process_sources(srcs ${PARAM_UNPARSED_ARGUMENTS})
list(APPEND LLVM_LINK_COMPONENTS ${PARAM_LINK_COMPONENTS})
@@ -35,6 +35,8 @@
endif()
#PIC not needed on Win
+ # FIXME: Setting CMAKE_CXX_FLAGS here is a no-op, use target_compile_options
+ # or omit this logic instead.
if (NOT WIN32)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fPIC")
endif()
@@ -78,6 +80,9 @@
# headers without negatively impacting much of anything.
add_dependencies(${name} clang-tablegen-targets)
+ # Add i
[Lldb-commits] [PATCH] D41101: [testsuite] Remove testing failures vestiges
vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. Thanks! The 6.0 compiler is ancient at this point. This lgtm. https://reviews.llvm.org/D41101 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42206: If kevent() is interrupted by signal (or is being debugged) and we get EINTR, retry
vsk requested changes to this revision. vsk added a comment. Why not take an approach similar to the one in https://reviews.llvm.org/D41008? It looks like it's possible to set up a poll loop, call signal(), and verify that the loop is still running. Repository: rLLD LLVM Linker https://reviews.llvm.org/D42206 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42206: If kevent() is interrupted by signal (or is being debugged) and we get EINTR, retry
vsk added a comment. In https://reviews.llvm.org/D42206#979570, @jasonmolenda wrote: > I tried sending signals to lldb-server via kill() and the signal handler > caught them, the bit of code I had printing out the return value & errno > value never got executed. The only way I was able to repo this was by > debugging lldb-server. Is it possible to unregister the handler within the unit test? Repository: rLLD LLVM Linker https://reviews.llvm.org/D42206 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42206: If kevent() is interrupted by signal (or is being debugged) and we get EINTR, retry
vsk added a comment. In https://reviews.llvm.org/D42206#979607, @clayborg wrote: > Are we going to test each unix call that can fail with EINTR? Seems a bit > overkill. I think going forward, we should test all functional changes unless it really is prohibitively expensive to do so. Useful changes like this shouldn't be lost to accidental/unrelated commits, and having a regression test is a good way to ensure that. Repository: rLLD LLVM Linker https://reviews.llvm.org/D42206 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42215: [CMake] Make check-lldb work with LLDB_CODESIGN_IDENTITY=''
vsk created this revision.
vsk added reviewers: davide, aprantl, jasonmolenda.
Herald added a subscriber: mgorny.
On Darwin, if a test machine isn't set up for code-signing (see
docs/code-signing.txt), running check-lldb should use the system
debugserver instead of the unsigned one built in-tree. This makes it
possible to run lldb's test suite without having code-signing set up,
which is really convenient.
https://reviews.llvm.org/D42215
Files:
docs/code-signing.txt
test/CMakeLists.txt
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -25,7 +25,9 @@
endif()
if(TARGET debugserver)
- list(APPEND LLDB_TEST_DEPS debugserver)
+ if(NOT CMAKE_HOST_APPLE OR LLDB_CODESIGN_IDENTITY)
+list(APPEND LLDB_TEST_DEPS debugserver)
+ endif()
endif()
if(TARGET lldb-mi)
@@ -95,7 +97,17 @@
endif()
if(CMAKE_HOST_APPLE)
- list(APPEND LLDB_TEST_COMMON_ARGS --server $)
+ if(LLDB_CODESIGN_IDENTITY)
+list(APPEND LLDB_TEST_COMMON_ARGS --server $)
+ else()
+execute_process(
+ COMMAND xcode-select -p
+ OUTPUT_VARIABLE XCODE_DEV_DIR)
+string(STRIP ${XCODE_DEV_DIR} XCODE_DEV_DIR)
+set(SYSTEM_DEBUGSERVER
+
"${XCODE_DEV_DIR}/../SharedFrameworks/LLDB.framework/Resources/debugserver")
+list(APPEND LLDB_TEST_COMMON_ARGS --server ${SYSTEM_DEBUGSERVER})
+ endif()
endif()
set(LLDB_DOTEST_ARGS ${LLDB_TEST_COMMON_ARGS};${LLDB_TEST_USER_ARGS})
Index: docs/code-signing.txt
===
--- docs/code-signing.txt
+++ docs/code-signing.txt
@@ -1,6 +1,11 @@
-On MacOSX lldb needs to be code signed. The Debug, DebugClang and Release
-builds are set to code sign using a code signing certificate named
-"lldb_codesign".
+To use the in-tree debug server on macOS, lldb needs to be code signed. The
+Debug, DebugClang and Release builds are set to code sign using a code signing
+certificate named "lldb_codesign". This document explains how to set up the
+signing certificate.
+
+Note that it is possible build and use lldb on macOS without setting up code
+signing by using the system's debug server. To configure lldb in this way with
+cmake, specify -DLLDB_CODESIGN_IDENTITY=''.
If you have re-installed a new OS, please delete all old lldb_codesign items
from your keychain. There will be a code signing certification and a public
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -25,7 +25,9 @@
endif()
if(TARGET debugserver)
- list(APPEND LLDB_TEST_DEPS debugserver)
+ if(NOT CMAKE_HOST_APPLE OR LLDB_CODESIGN_IDENTITY)
+list(APPEND LLDB_TEST_DEPS debugserver)
+ endif()
endif()
if(TARGET lldb-mi)
@@ -95,7 +97,17 @@
endif()
if(CMAKE_HOST_APPLE)
- list(APPEND LLDB_TEST_COMMON_ARGS --server $)
+ if(LLDB_CODESIGN_IDENTITY)
+list(APPEND LLDB_TEST_COMMON_ARGS --server $)
+ else()
+execute_process(
+ COMMAND xcode-select -p
+ OUTPUT_VARIABLE XCODE_DEV_DIR)
+string(STRIP ${XCODE_DEV_DIR} XCODE_DEV_DIR)
+set(SYSTEM_DEBUGSERVER
+ "${XCODE_DEV_DIR}/../SharedFrameworks/LLDB.framework/Resources/debugserver")
+list(APPEND LLDB_TEST_COMMON_ARGS --server ${SYSTEM_DEBUGSERVER})
+ endif()
endif()
set(LLDB_DOTEST_ARGS ${LLDB_TEST_COMMON_ARGS};${LLDB_TEST_USER_ARGS})
Index: docs/code-signing.txt
===
--- docs/code-signing.txt
+++ docs/code-signing.txt
@@ -1,6 +1,11 @@
-On MacOSX lldb needs to be code signed. The Debug, DebugClang and Release
-builds are set to code sign using a code signing certificate named
-"lldb_codesign".
+To use the in-tree debug server on macOS, lldb needs to be code signed. The
+Debug, DebugClang and Release builds are set to code sign using a code signing
+certificate named "lldb_codesign". This document explains how to set up the
+signing certificate.
+
+Note that it is possible build and use lldb on macOS without setting up code
+signing by using the system's debug server. To configure lldb in this way with
+cmake, specify -DLLDB_CODESIGN_IDENTITY=''.
If you have re-installed a new OS, please delete all old lldb_codesign items
from your keychain. There will be a code signing certification and a public
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42215: [CMake] Make check-lldb work with LLDB_CODESIGN_IDENTITY=''
vsk updated this revision to Diff 130325.
vsk added a comment.
Thanks for the feedback!
-Print out the path to the debug server as a status message
https://reviews.llvm.org/D42215
Files:
docs/code-signing.txt
test/CMakeLists.txt
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -25,7 +25,9 @@
endif()
if(TARGET debugserver)
- list(APPEND LLDB_TEST_DEPS debugserver)
+ if(NOT CMAKE_HOST_APPLE OR LLDB_CODESIGN_IDENTITY)
+list(APPEND LLDB_TEST_DEPS debugserver)
+ endif()
endif()
if(TARGET lldb-mi)
@@ -95,7 +97,18 @@
endif()
if(CMAKE_HOST_APPLE)
- list(APPEND LLDB_TEST_COMMON_ARGS --server $)
+ if(LLDB_CODESIGN_IDENTITY)
+set(DEBUGSERVER_PATH $)
+ else()
+execute_process(
+ COMMAND xcode-select -p
+ OUTPUT_VARIABLE XCODE_DEV_DIR)
+string(STRIP ${XCODE_DEV_DIR} XCODE_DEV_DIR)
+set(DEBUGSERVER_PATH
+
"${XCODE_DEV_DIR}/../SharedFrameworks/LLDB.framework/Resources/debugserver")
+ endif()
+ message(STATUS "Path to the lldb debugserver: ${DEBUGSERVER_PATH}")
+ list(APPEND LLDB_TEST_COMMON_ARGS --server ${DEBUGSERVER_PATH})
endif()
set(LLDB_DOTEST_ARGS ${LLDB_TEST_COMMON_ARGS};${LLDB_TEST_USER_ARGS})
Index: docs/code-signing.txt
===
--- docs/code-signing.txt
+++ docs/code-signing.txt
@@ -1,6 +1,11 @@
-On MacOSX lldb needs to be code signed. The Debug, DebugClang and Release
-builds are set to code sign using a code signing certificate named
-"lldb_codesign".
+To use the in-tree debug server on macOS, lldb needs to be code signed. The
+Debug, DebugClang and Release builds are set to code sign using a code signing
+certificate named "lldb_codesign". This document explains how to set up the
+signing certificate.
+
+Note that it is possible build and use lldb on macOS without setting up code
+signing by using the system's debug server. To configure lldb in this way with
+cmake, specify -DLLDB_CODESIGN_IDENTITY=''.
If you have re-installed a new OS, please delete all old lldb_codesign items
from your keychain. There will be a code signing certification and a public
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -25,7 +25,9 @@
endif()
if(TARGET debugserver)
- list(APPEND LLDB_TEST_DEPS debugserver)
+ if(NOT CMAKE_HOST_APPLE OR LLDB_CODESIGN_IDENTITY)
+list(APPEND LLDB_TEST_DEPS debugserver)
+ endif()
endif()
if(TARGET lldb-mi)
@@ -95,7 +97,18 @@
endif()
if(CMAKE_HOST_APPLE)
- list(APPEND LLDB_TEST_COMMON_ARGS --server $)
+ if(LLDB_CODESIGN_IDENTITY)
+set(DEBUGSERVER_PATH $)
+ else()
+execute_process(
+ COMMAND xcode-select -p
+ OUTPUT_VARIABLE XCODE_DEV_DIR)
+string(STRIP ${XCODE_DEV_DIR} XCODE_DEV_DIR)
+set(DEBUGSERVER_PATH
+ "${XCODE_DEV_DIR}/../SharedFrameworks/LLDB.framework/Resources/debugserver")
+ endif()
+ message(STATUS "Path to the lldb debugserver: ${DEBUGSERVER_PATH}")
+ list(APPEND LLDB_TEST_COMMON_ARGS --server ${DEBUGSERVER_PATH})
endif()
set(LLDB_DOTEST_ARGS ${LLDB_TEST_COMMON_ARGS};${LLDB_TEST_USER_ARGS})
Index: docs/code-signing.txt
===
--- docs/code-signing.txt
+++ docs/code-signing.txt
@@ -1,6 +1,11 @@
-On MacOSX lldb needs to be code signed. The Debug, DebugClang and Release
-builds are set to code sign using a code signing certificate named
-"lldb_codesign".
+To use the in-tree debug server on macOS, lldb needs to be code signed. The
+Debug, DebugClang and Release builds are set to code sign using a code signing
+certificate named "lldb_codesign". This document explains how to set up the
+signing certificate.
+
+Note that it is possible build and use lldb on macOS without setting up code
+signing by using the system's debug server. To configure lldb in this way with
+cmake, specify -DLLDB_CODESIGN_IDENTITY=''.
If you have re-installed a new OS, please delete all old lldb_codesign items
from your keychain. There will be a code signing certification and a public
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42277: Use test-specific module caches to avoid stale header conflicts
vsk created this revision. vsk added reviewers: aprantl, davide, jasonmolenda, labath. Herald added a subscriber: eraman. Stale global module caches cause problems for the bots. The modules become invalid when clang headers are updated by version control, and tests which use these modules fail to compile, e.g: fatal error: file '.../__stddef_max_align_t.h' has been modified since the module file '/var/.../Darwin.pcm' was built note: please rebuild precompiled header '/var/.../Darwin.pcm' Eventually we should transition to having just a single module cache to speed tests up. This patch should be just enough to fix the spurious bot failures due to stale caches. rdar://36479805 https://reviews.llvm.org/D42277 Files: packages/Python/lldbsuite/test/lang/objc/modules-auto-import/Makefile packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile packages/Python/lldbsuite/test/make/Makefile.rules Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -237,13 +237,15 @@ CFLAGS += -gsplit-dwarf endif +CLANG_MODULE_CACHE_DIR := module-cache + +MANDATORY_MODULE_BUILD_CFLAGS := -fmodules -fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR) + ifeq "$(MAKE_GMODULES)" "YES" - CFLAGS += -fmodules -gmodules + CFLAGS += -gmodules $(MANDATORY_MODULE_BUILD_CFLAGS) endif -CXXFLAGS += -std=c++11 -# FIXME: C++ modules aren't supported on all platforms. -CXXFLAGS += $(subst -fmodules,, $(CFLAGS)) +CXXFLAGS += -std=c++11 $(CFLAGS) LD = $(CC) LDFLAGS ?= $(CFLAGS) LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS) @@ -636,7 +638,7 @@ dsym: $(DSYM) all: $(EXE) $(DSYM) clean:: - $(RM) $(OBJECTS) $(PREREQS) $(PREREQS:.d=.d.tmp) $(DWOS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS) + $(RM) -rf $(OBJECTS) $(PREREQS) $(PREREQS:.d=.d.tmp) $(DWOS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS) $(CLANG_MODULE_CACHE_DIR) ifneq "$(DYLIB_NAME)" "" $(RM) -r $(DYLIB_FILENAME).dSYM $(RM) $(DYLIB_OBJECTS) $(DYLIB_PREREQS) $(DYLIB_PREREQS:.d=.d.tmp) $(DYLIB_DWOS) $(DYLIB_FILENAME) $(DYLIB_FILENAME).debug Index: packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile === --- packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile +++ packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile @@ -6,4 +6,4 @@ include $(LEVEL)/Makefile.rules -CFLAGS += -fmodules -I$(PWD) +CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(PWD) Index: packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile === --- packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile +++ packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile @@ -4,5 +4,5 @@ include $(LEVEL)/Makefile.rules -CFLAGS += -fmodules -I$(PWD) +CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(PWD) LDFLAGS += -framework Foundation Index: packages/Python/lldbsuite/test/lang/objc/modules-auto-import/Makefile === --- packages/Python/lldbsuite/test/lang/objc/modules-auto-import/Makefile +++ packages/Python/lldbsuite/test/lang/objc/modules-auto-import/Makefile @@ -1,6 +1,6 @@ LEVEL = ../../../make OBJC_SOURCES := main.m -CFLAGS += -fmodules -gmodules -g +CFLAGS += -gmodules $(MANDATORY_MODULE_BUILD_CFLAGS) include $(LEVEL)/Makefile.rules Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -237,13 +237,15 @@ CFLAGS += -gsplit-dwarf endif +CLANG_MODULE_CACHE_DIR := module-cache + +MANDATORY_MODULE_BUILD_CFLAGS := -fmodules -fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR) + ifeq "$(MAKE_GMODULES)" "YES" - CFLAGS += -fmodules -gmodules + CFLAGS += -gmodules $(MANDATORY_MODULE_BUILD_CFLAGS) endif -CXXFLAGS += -std=c++11 -# FIXME: C++ modules aren't supported on all platforms. -CXXFLAGS += $(subst -fmodules,, $(CFLAGS)) +CXXFLAGS += -std=c++11 $(CFLAGS) LD = $(CC) LDFLAGS ?= $(CFLAGS) LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS) @@ -636,7 +638,7 @@ dsym: $(DSYM) all: $(EXE) $(DSYM) clean:: - $(RM) $(OBJECTS) $(PREREQS) $(PREREQS:.d=.d.tmp) $(DWOS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS) + $(RM) -rf $(OBJECTS) $(PREREQS) $(PREREQS:.d=.d.tmp) $(DWOS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS) $(CLANG_MODULE_CACHE_DIR) ifneq "$(DYLIB_NAME)" "" $(RM) -r $(DYLIB_FILENAME).dSYM $(RM) $(DYLIB_OBJECTS) $(DYLIB_PREREQS) $(DYLIB_PREREQS:.d=.d.tmp) $(DYLIB_DWOS) $(DYLIB_FILENAME) $(DYLIB_FILENAME).debug Index: packages/Python/lldbsuite/test/
[Lldb-commits] [PATCH] D42277: Use test-specific module caches to avoid stale header conflicts
vsk added inline comments. Comment at: packages/Python/lldbsuite/test/make/Makefile.rules:240 +CLANG_MODULE_CACHE_DIR := module-cache + aprantl wrote: > Is it safe that this is a relative path? I admit it's not great, but it's at least in keeping with the other targets we define in this Makefile (like a.out, the dsym, etc). As I'm writing this, I've realized I did not upload a diff with context, so this might not have been apparent! Comment at: packages/Python/lldbsuite/test/make/Makefile.rules:242 + +MANDATORY_MODULE_BUILD_CFLAGS := -fmodules -fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR) + aprantl wrote: > Should we also specify `-fcxxmodules` here? Or will that break some bots with > older clangs? I've tried to avoid changing the flags used by each Makefile when possible. The only exception is my removal of "$(subst -fmodules,, $(CFLAGS))" in CXXFLAGS, which I needed to do because the substitution would make us pass "-cache-path=foo". We might consider adding in -fcxxmodules as a followup (maybe just in CXXFLAGS?). https://reviews.llvm.org/D42277 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42277: Use test-specific module caches to avoid stale header conflicts
vsk updated this revision to Diff 130566. vsk added a comment. - Upload a diff with context. https://reviews.llvm.org/D42277 Files: packages/Python/lldbsuite/test/lang/objc/modules-auto-import/Makefile packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile packages/Python/lldbsuite/test/make/Makefile.rules Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -237,13 +237,15 @@ CFLAGS += -gsplit-dwarf endif +CLANG_MODULE_CACHE_DIR := module-cache + +MANDATORY_MODULE_BUILD_CFLAGS := -fmodules -fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR) + ifeq "$(MAKE_GMODULES)" "YES" - CFLAGS += -fmodules -gmodules + CFLAGS += -gmodules $(MANDATORY_MODULE_BUILD_CFLAGS) endif -CXXFLAGS += -std=c++11 -# FIXME: C++ modules aren't supported on all platforms. -CXXFLAGS += $(subst -fmodules,, $(CFLAGS)) +CXXFLAGS += -std=c++11 $(CFLAGS) LD = $(CC) LDFLAGS ?= $(CFLAGS) LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS) @@ -636,7 +638,7 @@ dsym: $(DSYM) all: $(EXE) $(DSYM) clean:: - $(RM) $(OBJECTS) $(PREREQS) $(PREREQS:.d=.d.tmp) $(DWOS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS) + $(RM) -rf $(OBJECTS) $(PREREQS) $(PREREQS:.d=.d.tmp) $(DWOS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS) $(CLANG_MODULE_CACHE_DIR) ifneq "$(DYLIB_NAME)" "" $(RM) -r $(DYLIB_FILENAME).dSYM $(RM) $(DYLIB_OBJECTS) $(DYLIB_PREREQS) $(DYLIB_PREREQS:.d=.d.tmp) $(DYLIB_DWOS) $(DYLIB_FILENAME) $(DYLIB_FILENAME).debug Index: packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile === --- packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile +++ packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile @@ -6,4 +6,4 @@ include $(LEVEL)/Makefile.rules -CFLAGS += -fmodules -I$(PWD) +CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(PWD) Index: packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile === --- packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile +++ packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile @@ -4,5 +4,5 @@ include $(LEVEL)/Makefile.rules -CFLAGS += -fmodules -I$(PWD) +CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(PWD) LDFLAGS += -framework Foundation Index: packages/Python/lldbsuite/test/lang/objc/modules-auto-import/Makefile === --- packages/Python/lldbsuite/test/lang/objc/modules-auto-import/Makefile +++ packages/Python/lldbsuite/test/lang/objc/modules-auto-import/Makefile @@ -1,6 +1,6 @@ LEVEL = ../../../make OBJC_SOURCES := main.m -CFLAGS += -fmodules -gmodules -g +CFLAGS += -gmodules $(MANDATORY_MODULE_BUILD_CFLAGS) include $(LEVEL)/Makefile.rules Index: packages/Python/lldbsuite/test/make/Makefile.rules === --- packages/Python/lldbsuite/test/make/Makefile.rules +++ packages/Python/lldbsuite/test/make/Makefile.rules @@ -237,13 +237,15 @@ CFLAGS += -gsplit-dwarf endif +CLANG_MODULE_CACHE_DIR := module-cache + +MANDATORY_MODULE_BUILD_CFLAGS := -fmodules -fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR) + ifeq "$(MAKE_GMODULES)" "YES" - CFLAGS += -fmodules -gmodules + CFLAGS += -gmodules $(MANDATORY_MODULE_BUILD_CFLAGS) endif -CXXFLAGS += -std=c++11 -# FIXME: C++ modules aren't supported on all platforms. -CXXFLAGS += $(subst -fmodules,, $(CFLAGS)) +CXXFLAGS += -std=c++11 $(CFLAGS) LD = $(CC) LDFLAGS ?= $(CFLAGS) LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS) @@ -636,7 +638,7 @@ dsym: $(DSYM) all: $(EXE) $(DSYM) clean:: - $(RM) $(OBJECTS) $(PREREQS) $(PREREQS:.d=.d.tmp) $(DWOS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS) + $(RM) -rf $(OBJECTS) $(PREREQS) $(PREREQS:.d=.d.tmp) $(DWOS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS) $(CLANG_MODULE_CACHE_DIR) ifneq "$(DYLIB_NAME)" "" $(RM) -r $(DYLIB_FILENAME).dSYM $(RM) $(DYLIB_OBJECTS) $(DYLIB_PREREQS) $(DYLIB_PREREQS:.d=.d.tmp) $(DYLIB_DWOS) $(DYLIB_FILENAME) $(DYLIB_FILENAME).debug Index: packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile === --- packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile +++ packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile @@ -6,4 +6,4 @@ include $(LEVEL)/Makefile.rules -CFLAGS += -fmodules -I$(PWD) +CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(PWD) Index: packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile === --- packages/Python/lldbsuite/test/lang/objc/modu
[Lldb-commits] [PATCH] D42280: Wrap all references to build artifacts in the LLDB testsuite in TestBase::getBuildArtifact()
vsk added a comment. In https://reviews.llvm.org/D42280#982190, @jingham wrote: > We have a lot of ugly boilerplate in the testsuite. I added: > > (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, > "Set a breakpoint here", self.main_source_file) > > > Because that's what a lot of tests did. I converted a handful of tests and > then ran out of time. > > If we're going to touch how all the tests get started with their executable > it would be nice to see how many of them can be easily converted to this > interface. I can also add a file & line version of this, though in fact most > tests that use file & line breakpoints actually use a source match to get a > line number and then feed the line to lldb, so they could easily be converted > to this. > > Then all the business of locating the binary could be centralized. > > I guess it's a little late for this comment, however... I think lldbutil.run_to_source_breakpoint is definitely worth standardizing on, but as it stands, this patch lgtm as an initial step. https://reviews.llvm.org/D42280 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42277: Use test-specific module caches to avoid stale header conflicts
vsk updated this revision to Diff 130686.
vsk added a comment.
Per an offline comment by Adrian, include -gmodules in the mandatory set of
module flags.
https://reviews.llvm.org/D42277
Files:
packages/Python/lldbsuite/test/lang/objc/modules-auto-import/Makefile
packages/Python/lldbsuite/test/lang/objc/modules-auto-import/TestModulesAutoImport.py
packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile
packages/Python/lldbsuite/test/lang/objc/modules-incomplete/TestIncompleteModules.py
packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile
packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
packages/Python/lldbsuite/test/make/Makefile.rules
Index: packages/Python/lldbsuite/test/make/Makefile.rules
===
--- packages/Python/lldbsuite/test/make/Makefile.rules
+++ packages/Python/lldbsuite/test/make/Makefile.rules
@@ -237,13 +237,15 @@
CFLAGS += -gsplit-dwarf
endif
+CLANG_MODULE_CACHE_DIR := module-cache
+
+MANDATORY_MODULE_BUILD_CFLAGS := -fmodules -gmodules -fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR)
+
ifeq "$(MAKE_GMODULES)" "YES"
- CFLAGS += -fmodules -gmodules
+ CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS)
endif
-CXXFLAGS += -std=c++11
-# FIXME: C++ modules aren't supported on all platforms.
-CXXFLAGS += $(subst -fmodules,, $(CFLAGS))
+CXXFLAGS += -std=c++11 $(CFLAGS)
LD = $(CC)
LDFLAGS ?= $(CFLAGS)
LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
@@ -636,7 +638,7 @@
dsym: $(DSYM)
all: $(EXE) $(DSYM)
clean::
- $(RM) $(OBJECTS) $(PREREQS) $(PREREQS:.d=.d.tmp) $(DWOS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS)
+ $(RM) -rf $(OBJECTS) $(PREREQS) $(PREREQS:.d=.d.tmp) $(DWOS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS) $(CLANG_MODULE_CACHE_DIR)
ifneq "$(DYLIB_NAME)" ""
$(RM) -r $(DYLIB_FILENAME).dSYM
$(RM) $(DYLIB_OBJECTS) $(DYLIB_PREREQS) $(DYLIB_PREREQS:.d=.d.tmp) $(DYLIB_DWOS) $(DYLIB_FILENAME) $(DYLIB_FILENAME).debug
Index: packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
===
--- packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
+++ packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
@@ -27,7 +27,7 @@
self.line = line_number('main.m', '// Set breakpoint here.')
@skipUnlessDarwin
-@skipIf(macos_version=["<", "10.12"])
+@skipIf(macos_version=["<", "10.12"], debug_info=no_match(["gmodules"]))
def test_expr(self):
self.build()
exe = os.path.join(os.getcwd(), "a.out")
Index: packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile
===
--- packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile
+++ packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile
@@ -6,4 +6,4 @@
include $(LEVEL)/Makefile.rules
-CFLAGS += -fmodules -I$(PWD)
+CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(PWD)
Index: packages/Python/lldbsuite/test/lang/objc/modules-incomplete/TestIncompleteModules.py
===
--- packages/Python/lldbsuite/test/lang/objc/modules-incomplete/TestIncompleteModules.py
+++ packages/Python/lldbsuite/test/lang/objc/modules-incomplete/TestIncompleteModules.py
@@ -24,7 +24,7 @@
@skipUnlessDarwin
@unittest2.expectedFailure("rdar://20416388")
-@skipIf(macos_version=["<", "10.12"])
+@skipIf(macos_version=["<", "10.12"], debug_info=no_match(["gmodules"]))
def test_expr(self):
self.build()
exe = os.path.join(os.getcwd(), "a.out")
Index: packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile
===
--- packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile
+++ packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile
@@ -4,5 +4,5 @@
include $(LEVEL)/Makefile.rules
-CFLAGS += -fmodules -I$(PWD)
+CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(PWD)
LDFLAGS += -framework Foundation
Index: packages/Python/lldbsuite/test/lang/objc/modules-auto-import/TestModulesAutoImport.py
===
--- packages/Python/lldbsuite/test/lang/objc/modules-auto-import/TestModulesAutoImport.py
+++ packages/Python/lldbsuite/test/lang/objc/modules-auto-import/TestModulesAutoImport.py
@@ -26,7 +26,7 @@
self.line = line_number('main.m', '// Set breakpoint 0 here.')
@skipUnlessDarwin
-@skipIf(macos_version=["<", "10.12"])
+@skipIf(macos_version=["<", "10.12"], debug_info=no_match(["gmodules"]))
def test_expr(self):
self.build()
exe = os.path.join(os.getcwd(), "a.out")
Index: packages/Python/lldbsuite/test/lang/objc/modules-auto-i
[Lldb-commits] [PATCH] D42277: Use test-specific module caches to avoid stale header conflicts
vsk updated this revision to Diff 131000.
vsk added a comment.
- Skip tests which fail when -fmodules is passed
(https://bugs.llvm.org/show_bug.cgi?id=36048).
https://reviews.llvm.org/D42277
Files:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-globals/TestDataFormatterGlobals.py
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/vector/TestDataFormatterLibcxxVector.py
packages/Python/lldbsuite/test/lang/objc/modules-auto-import/Makefile
packages/Python/lldbsuite/test/lang/objc/modules-auto-import/TestModulesAutoImport.py
packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile
packages/Python/lldbsuite/test/lang/objc/modules-incomplete/TestIncompleteModules.py
packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile
packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
packages/Python/lldbsuite/test/make/Makefile.rules
Index: packages/Python/lldbsuite/test/make/Makefile.rules
===
--- packages/Python/lldbsuite/test/make/Makefile.rules
+++ packages/Python/lldbsuite/test/make/Makefile.rules
@@ -237,13 +237,15 @@
CFLAGS += -gsplit-dwarf
endif
+CLANG_MODULE_CACHE_DIR := module-cache
+
+MANDATORY_MODULE_BUILD_CFLAGS := -fmodules -gmodules -fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR)
+
ifeq "$(MAKE_GMODULES)" "YES"
- CFLAGS += -fmodules -gmodules
+ CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS)
endif
-CXXFLAGS += -std=c++11
-# FIXME: C++ modules aren't supported on all platforms.
-CXXFLAGS += $(subst -fmodules,, $(CFLAGS))
+CXXFLAGS += -std=c++11 $(CFLAGS)
LD = $(CC)
LDFLAGS ?= $(CFLAGS)
LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
@@ -636,7 +638,7 @@
dsym: $(DSYM)
all: $(EXE) $(DSYM)
clean::
- $(RM) $(OBJECTS) $(PREREQS) $(PREREQS:.d=.d.tmp) $(DWOS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS)
+ $(RM) -rf $(OBJECTS) $(PREREQS) $(PREREQS:.d=.d.tmp) $(DWOS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS) $(CLANG_MODULE_CACHE_DIR)
ifneq "$(DYLIB_NAME)" ""
$(RM) -r $(DYLIB_FILENAME).dSYM
$(RM) $(DYLIB_OBJECTS) $(DYLIB_PREREQS) $(DYLIB_PREREQS:.d=.d.tmp) $(DYLIB_DWOS) $(DYLIB_FILENAME) $(DYLIB_FILENAME).debug
Index: packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
===
--- packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
+++ packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
@@ -27,7 +27,7 @@
self.line = line_number('main.m', '// Set breakpoint here.')
@skipUnlessDarwin
-@skipIf(macos_version=["<", "10.12"])
+@skipIf(macos_version=["<", "10.12"], debug_info=no_match(["gmodules"]))
def test_expr(self):
self.build()
exe = self.getBuildArtifact("a.out")
Index: packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile
===
--- packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile
+++ packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile
@@ -6,4 +6,4 @@
include $(LEVEL)/Makefile.rules
-CFLAGS += -fmodules -I$(PWD)
+CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(PWD)
Index: packages/Python/lldbsuite/test/lang/objc/modules-incomplete/TestIncompleteModules.py
===
--- packages/Python/lldbsuite/test/lang/objc/modules-incomplete/TestIncompleteModules.py
+++ packages/Python/lldbsuite/test/lang/objc/modules-incomplete/TestIncompleteModules.py
@@ -24,7 +24,7 @@
@skipUnlessDarwin
@unittest2.expectedFailure("rdar://20416388")
-@skipIf(macos_version=["<", "10.12"])
+@skipIf(macos_version=["<", "10.12"], debug_info=no_match(["gmodules"]))
def test_expr(self):
self.build()
exe = self.getBuildArtifact("a.out")
Index: packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile
===
--- packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile
+++ packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile
@@ -4,5 +4,5 @@
include $(LEVEL)/Makefile.rules
-CFLAGS += -fmodules -I$(PWD)
+CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(PWD)
LDFLAGS += -framework Foundation
Index: packages/Python/lldbsuite/test/lang/objc/modules-auto-import/TestModulesAutoImport.py
===
--- packages/Python/lldbsuite/test/lang/objc/modules-auto-import/TestModulesAutoImport.py
+++ pa
[Lldb-commits] [PATCH] D42277: Use test-specific module caches to avoid stale header conflicts
vsk added a comment. In https://reviews.llvm.org/D42277#985002, @aprantl wrote: > > Skip tests which fail when -fmodules is passed > > (https://bugs.llvm.org/show_bug.cgi?id=36048). > > Wait.. this patch is not supposed to change the set of tests that get > -fmodules passed to them. It should only add -fmodules-cache-path to tests > that do pass -fmodules. Why is this necessary? It's not necessary, but we should make this change. It falls out of removing this FIXME: # FIXME: C++ modules aren't supported on all platforms. CXXFLAGS += $(subst -fmodules,, $(CFLAGS)) If we keep it around, we'd need an extra hack to strip out "-fmodules-cache-path=module-cache" as well. Without that we'd pass "-cache-path=module-cache", which clang would reject. Another benefit of making this change is that it lets us know which tests are actually failing with modules enabled, instead of hiding them. https://reviews.llvm.org/D42277 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42277: Use test-specific module caches to avoid stale header conflicts
vsk added a comment. Is this OK to commit? https://reviews.llvm.org/D42277 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`
vsk added a comment. What's the failure mode? Have we had any issues with this on the bots? Generally I'm all for removing flaky tests, I'd like to understand what makes this flaky so we can avoid whatever it is in the future. In this case, we should be able test this like we test clang code completion, a la c-index-test. https://reviews.llvm.org/D42656 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D43024: [test] Enable test category for inline tests.
vsk accepted this revision. vsk added a comment. This lgtm Jonas, thank you! https://reviews.llvm.org/D43024 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D43024: [test] Enable test category for inline tests.
vsk reopened this revision. vsk added a comment. This revision is now accepted and ready to land. Ah, I see the first part landed in https://reviews.llvm.org/rL324488 and this is a follow-up. https://reviews.llvm.org/D43024 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
