jasonmolenda added a comment.
Overall this changeset looks good to me. A few small comments.
================
Comment at:
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/fast_conditional_breakpoints/TestFastConditionalBreakpoints.py:142
+ expected_fn_name = "$__lldb_jitted_conditional_bp_trampoline"
+ self.assertTrue(frame1 and frame1.GetFunctionName() ==
expected_fn_name)
+
----------------
Personal style thing, but I would do these as two separate tests -- test that
frame1.IsValid() and test that frame1.GetFunctionName returns the expected
result. So when the test fails, you know which is the problem just from the
line number.
================
Comment at:
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/fast_conditional_breakpoints/TestFastConditionalBreakpoints.py:159
+ #self.assertEqual(var.GetValue(), "9")
+
def inject_invalid_fast_conditional_breakpoint(self):
----------------
Is this an SB API problem, or a more basic lldb problem? If you did 'frame
variable local_count' in this frame, would you get 9 or a different value?
================
Comment at: lldb/source/Symbol/UnwindPlan.cpp:161
+ s.PutChar('=');
+ s.Printf("%#llx (const-value)", m_location.const_value);
+ break;
----------------
We tend to do "0x%" PRIx64 " (const-value)", .... in this kind of instance.
================
Comment at: lldb/source/Target/ABI.cpp:47
+lldb::ModuleSP ABI::PrepareUnwinding(lldb::addr_t address, std::size_t size,
+ lldb::addr_t return_address) {
----------------
I think we can probably come up with a more descriptive name for this.
ABI::CreateModuleForFastConditionalBreakpointTrampoline ? Not wedded to that
name, but we should give a little more idea of what this method is doing.
Right now, I wouldn't be able to figure it out unless I read the log/error
messages.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66250/new/
https://reviews.llvm.org/D66250
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
