[Lldb-commits] [lldb] [lldb] Fix crash after second run when set a previous watchpoint. (PR #136682)

[via lldb-commits]

hapeeeeee wrote:

> I'll need to test this, I don't think it's right but I haven't looked closely enough at the code.
> 
> We have two cases we need to support, and one of them only happens on AArch64/arm64 type architectures, where the watchpoint hit is reported before the memory is modified.
> 
> On an arm/aarch64 processor, when a watchpoint is triggered, the instruction is unwound and the processor halts processing with a watchpoint exception. The value hasn't changed at this point. The debugger has to disable the watchpoint, instruction step, and then re-enable the watchpoint. It gathers the new value, and shows the old & new values to the user.
> 
> The bug that we're looking at here, is where a user sets a watchpoint in a process, the process exits/is killed. The Target still has the watchpoint present, but it is in a Disabled state. The user starts a new process session, and sets the watchpoint again. The disabled watchpoint in the Target is re-enabled. However, instead of collecting a new "current value", the old "current value" is kept around. The problem is that the old "current value" is all in terms of the OLD process, which no longer exists.
> 
> And later, when we try to examine that old value, we crash when it can't be accessed.
> 
> I'm afraid this patch as-is will break watchpoint behavior on aarch64/arm64, but it's a bit of a guess. I mentioned in the original bug report that I think there are two changes needed: When a Process has exited, the Target needs to clear the ValueObject storing the value for the watchpoints it still has. And when re-enabling, if we have an emtpy ValueObject, we need to re-collect that value.

I discovered that the current program crashes due to a null pointer of `ValueObjectSP`. This pointer is `reset` when the program is re-executed. Do you mean that we should handle the `ValueObject` itself instead of `ValueObjectSP`? Additionally, I will attempt to test this patch on aarch64.

https://github.com/llvm/llvm-project/pull/136682