[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
Comment at: lldb/test/API/commands/breakpoint/set/address-nomodule/TestBreakpointAddressNoModule.py:11
+    def get_address_from_symbol(self, symbol):
+        target = lldbutil.run_to_breakpoint_make_target(self, "a.out", True)
+        bp = target.BreakpointCreateByName(symbol, None)
----------------
Can we reuse this target? If so you can store it as `self.target` and reuse it from `test_set_address_no_module`.


================
Comment at: lldb/test/API/commands/breakpoint/set/address-nomodule/TestBreakpointAddressNoModule.py:21-22
+
+        tgt = lldbutil.run_to_breakpoint_make_target(self)
+        dbg = tgt.GetDebugger()
+
----------------
Consistency nit: I don't think we use `tgt` anywhere. We do use `dbg` but only for the global debugger stored in `self.dbg`. I'd use `debugger` and `target` instead to avoid confusion.


================
Comment at: lldb/test/API/commands/breakpoint/set/address-nomodule/TestBreakpointAddressNoModule.py:25
+        dbg.HandleCommand(f"break set -a {main_address:#x}")
+        self.assertTrue(tgt.GetNumBreakpoints() == 1)
+
----------------
Our test harness has a bunch of helper functions that generate better error messages if the assertion fails. For example, here you could use `assertEqual(1, tgt.GetNumBreakpoints())`. If the assertion fails, it will print something like:

> tgt.GetNumBreakpoints() was expected to be 1 but was 2

which is much more informative than 

? tgt.GetNumBreakpoints() == 1 was expected to be True but was False


================
Comment at: lldb/test/API/commands/breakpoint/set/address-nomodule/TestBreakpointAddressNoModule.py:31
+        _, _, thread, _ = lldbutil.run_to_breakpoint_do_run(self, tgt, bp)
+        self.assertTrue(thread.GetNumFrames() >= 1)
+
----------------



================
Comment at: lldb/test/API/commands/breakpoint/set/address-nomodule/TestBreakpointAddressNoModule.py:34
+        thread_pc = thread.GetFrameAtIndex(0).GetPCAddress()
+        self.assertTrue(thread_pc != None)
+
----------------



================
Comment at: lldb/test/API/commands/breakpoint/set/address-nomodule/TestBreakpointAddressNoModule.py:36
+
+        self.assertTrue(main_address==thread_pc.GetFileAddress())
----------------



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124731/new/

https://reviews.llvm.org/D124731