Issue 202672
Summary lldb-server does not account for writes over software breakpoint sites that it manages
Labels lldb
Assignees
Reporter DavidSpickett
    Discovered while reasearching https://github.com/llvm/llvm-project/issues/197564.

Some background information first:
* Inside LLDB we classify breakpoints as hardware, software, or external.
* External means the debug server will deal with it. It could in reality be software, hardware, simulator specific trap setting, whatever, we don't need to care.
* We assume that if the debug server replaces an instruction to place the breakpoint, it will put that original instruction back into memory reads so it appears unchanged (which is not always the case, as discovered in https://github.com/llvm/llvm-project/issues/197564).
* We assume, or have never thought to check, that the same happens for memory writes. That the saved instruction would be updated if a memory write overwrote it.
* Nothing in the GDB remote protocol docs says whose responsibility it is to handle either situation.

This issue is about the writing part.

This session demonstrates the problem.
```
$ ./bin/lldb /tmp/test.o
(lldb) target create "/tmp/test.o"
Current executable set to '/tmp/test.o' (aarch64).
(lldb) log enable gdb-remote packets
```
I'm disabling the cache to prevent any chance of lldb trying to fix this itself.
```
(lldb) settings set target.process.disable-memory-cache true
```
This breakpoint will become a software break on main once we run.
```
(lldb) b main
Breakpoint 1: where = test.o`main at test.c:1:21, address = 0x0000000000000714
(lldb) breakpoint list
Current breakpoints:
1: name = 'main', locations = 1
  1.1: where = test.o`main at test.c:1:21, address = test.o[0x0000000000000714], unresolved, hit count = 0 
```
Note the second part of the MultiBreakpoint packet, that's the `main` breakpoint.
```
(lldb) run
<...>
lldb             <  87> send packet: $jMultiBreakpoint:{"breakpoint_requests":["Z0,fffff7fc4c20,4","Z0,aaaaaaaa0714,4"]}]#fe
lldb <  28> read packet: ${"results":["OK","OK"]}]#85
<...>
Process 27261 stopped
* thread #1, name = 'test.o', stop reason = breakpoint 1.1
       frame #0: 0x0000aaaaaaaa0714 test.o`main at test.c:1:21
-> 1   	int main() { return 0; }
(lldb) breakpoint list
Current breakpoints:
1: name = 'main', locations = 1, resolved = 1, hit count = 1
  1.1: where = test.o`main at test.c:1:21, address = 0x0000aaaaaaaa0714, resolved, hit count = 1 
```
When we read the instructions back we will not see the breakpoint instruction, lldb-server correctly substitutes in the original instruction.

(I use the raw address here because _expression_ evaluation creates a bunch of irrelevant packets)
```
(lldb) register read pc
 pc = 0x0000aaaaaaaa0714  test.o`main at test.c:1:21
(lldb) memory read 0x0000aaaaaaaa0714
lldb             <  20> send packet: $xaaaaaaaa0714,20#da
lldb             <  36> read packet: $00008052c0035fd61f2003d5fd7bbfa9fd030091fd7bc1a8c0035fd601000200#2e
0xaaaaaaaa0714: 00 00 80 52 c0 03 5f d6 1f 20 03 d5 fd 7b bf a9  ...R.._.. ...{..
0xaaaaaaaa0724: fd 03 00 91 fd 7b c1 a8 c0 03 5f d6 01 00 02 00 .....{...._.....
```
Now we overwrite the breakpoint site. We know that this is not cached in lldb because we can see the `M` packet being sent out.
```
(lldb) memory write 0xaaaaaaaa0714 0xca 0xfe 0xf0 0x0d
lldb <  34> send packet: $qMemoryRegionInfo:aaaaaaaa0714#e8
lldb <  83> read packet: $start:aaaaaaaa0000;size:1000;permissions:rx;flags:;name:2f746d702f746573742e6f;#89
lldb <  28> send packet: $Maaaaaaaa0714,4:cafef00d#74
lldb <   6> read packet: $OK#9a
```
If lldb-server had caught this and updated the saved instruction for the breakpoint site, we should see the new value now, but we do not.
```
(lldb) memory read 0x0000aaaaaaaa0714
lldb <  20> send packet: $xaaaaaaaa0714,20#da
lldb             <  36> read packet: $00008052c0035fd61f2003d5fd7bbfa9fd030091fd7bc1a8c0035fd601000200#2e
0xaaaaaaaa0714: 00 00 80 52 c0 03 5f d6 1f 20 03 d5 fd 7b bf a9  ...R.._.. ...{..
0xaaaaaaaa0724: fd 03 00 91 fd 7b c1 a8 c0 03 5f d6 01 00 02 00 .....{...._.....
```
When we single step we normally disable the breakpoint, single step, then re-enable it. In this case the disable fails:
```
(lldb) ni
lldb             <  67> send packet: $jMultiBreakpoint:{"breakpoint_requests":["z0,aaaaaaaa0714,4"]}]#a1
lldb <  24> read packet: ${"results":["E09"]}]#8f
<...>
Process 27261 stopped
* thread #1, name = 'test.o', stop reason = signal SIGILL: illegal opcode
       frame #0: 0x0000aaaaaaaa0714 test.o`main at test.c:1:21
-> 1   	int main() { return 0; }
```
Though we now see the new value in memory somehow:
```
(lldb) memory read 0x0000aaaaaaaa0714
lldb             <  20> send packet: $xaaaaaaaa0714,20#da
lldb             <  36> read packet: $cafef00dc0035fd61f2003d5fd7bbfa9fd030091fd7bc1a8c0035fd601000200#21
0xaaaaaaaa0714: ca fe f0 0d c0 03 5f d6 1f 20 03 d5 fd 7b bf a9  ......_.. ...{..
0xaaaaaaaa0724: fd 03 00 91 fd 7b c1 a8 c0 03 5f d6 01 00 02 00 .....{...._.....
```
Which was pretty confusing at first, but what's happening is this:
* lldb-server saves the original opcode A and substitutes it into any reads of the site.
* A software breakpoint instruction B is written into memory.
* lldb writes over that location, lldb-server writes the new value C over B, and continues to show A in any memory reads.
* When asked to remove the breakpoint, lldb-server checks that the location contains B, it does not, so it fails to remove it because it doesn't want to make the situation worse.

We can see this in lldb-server logging:
```
$ ~/build-llvm-aarch64/bin/lldb-server gdbserver 127.0.0.1:1234 --log-channels "lldb break" -- /tmp/test.o
Launched '/tmp/test.o' as process 27096...
lldb-server-local_build
Connection established.
1781013220.950878143 NativeProcessProtocol.cpp:SetSoftwareBreakpoint              addr = 0xaaaaaaaa0714, size_hint = 4
1781013220.950940847 NativeProcessProtocol.cpp:EnableSoftwareBreakpoint           Overwriting bytes at 0xaaaaaaaa0714: 0x0, 0x0, 0x80, 0x52
1781013220.950979948 NativeProcessProtocol.cpp:EnableSoftwareBreakpoint           addr = 0xaaaaaaaa0714: SUCCESS
<...>
1781013264.103750229 NativeProcessProtocol.cpp:RemoveSoftwareBreakpoint           addr = 0xaaaaaaaa0714
1781013264.103790522 GDBRemoteCommunicationServerLLGS.cpp:ExecuteRemoveBreakpoint pid 27096 failed to remove breakpoint: Original breakpoint trap is no longer in memory.
```
lldb will do substitutions client side for writes in `Process::WriteMemory`, but only for `eSoftware` breakpoints. eExternal we expect the server to do it. The server calls `GDBRemoteCommunicationServerLLGS::ExecuteRemoveBreakpoint`, which ends up in `NativeProcessProtocol::RemoveSoftwareBreakpoint` which does:
```
  // Make sure the breakpoint opcode exists at this address
  if (llvm::ArrayRef(curr_break_op) != bkpt.breakpoint_opcodes) {
    if (curr_break_op != bkpt.saved_opcodes)
      return Status::FromErrorString(
          "Original breakpoint trap is no longer in memory.");
    LLDB_LOG(log,
             "Saved opcodes ({0:@[x]}) have already been restored at {1:x}.",
 llvm::make_range(saved.begin(), saved.end()), addr);
```

Overwriting instructions is an uncommon thing to do but it's not unheard of. Usually you're NOP-ing out something or jumping over some code manually. I could see someone setting a breakpoint on code they want to NOP out.

Also it would be nice to say we account for both reads and writes when dealing with external software breakpoints, not just reads.


_______________________________________________
llvm-bugs mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs

Reply via email to