dlav-sc wrote:

I'd like to share some concerns regarding `StopInfo{Watchpoint, Breakpoint}`. 
You mentioned that you want to unify the logic of these classes, so this 
information might be relevant. Or perhaps you are already aware of the issue I 
want to demonstrate.

>From my point of view, performing any checks regarding whether there was a 
>watchpoint hit or not in `StopInfo` is quite questionable. I'll try to 
>illustrate my concerns with an example that you can reproduce by yourself 
>using `ToT lldb`. Let's consider a program:
```
int main() {
  int watch_value = 10;
  int a = 5;
  watch_value += a;
  int b = 5;
  watch_value = 42;
}
```
Consider this scenario:

1. I set a watchpoint on `watch_value`
2. I configure a large ignore count for this watchpoint
3. I attempt to step to the next instruction using `next`

```
(lldb) b main
(lldb) r
-> int watch_value = 10;
(lldb) wa s v watch_value // set hw wp on watch_value
(lldb) watchpoint ignore -i 100 1 // set ignore_count=100
(lldb) next
Process 3706163 exited with status = 0 (0x00000000)
```
I expected to obtain a control at the next line, however as we can see, lldb 
doesn't return control to the user.
\
\
\
To understand the source of the issue, I decided to examine the `lldb step` 
logs. Here's what I saw after the `next` command:
```
intern-state     ThreadList::ShouldStop: 1 threads, 1 unsuspended threads       
                                                                                
                        
intern-state     Thread::ShouldStop(0x5652ead54180) for tid = 0x3a7483 
0x3a7483, pc = 0x000055555555513b                                               
                                 
intern-state     ^^^^^^^^ Thread::ShouldStop Begin ^^^^^^^^                     
                                                                                
                        
intern-state     Plan stack initial state:                                      
                                                                                
                        
  thread #1: tid = 0x3a7483:                                                    
                                                                                
                        
    Active plan stack:                                                          
                                                                                
                        
      Element 0: Base thread plan.                                              
                                                                                
                        
      Element 1: Stepping over line ig_c.c:2:7 using ranges: 
[0x0000555555555134-0x000055555555513b).                                        
                                           
      Element 2: Single stepping past breakpoint site 2 at 0x555555555134

intern-state     Step over breakpoint stopped for reason: watchpoint.
intern-state     ThreadPlanStepOverRange got asked if it explains the stop for 
some reason other than step.
intern-state     Plan base plan explains stop. 
intern-state     Base plan discarding thread plans for thread tid = 0x3a7483 
(breakpoint hit.)
intern-state     Discarding thread plans for thread (tid = 0x3a7483, force 0)
intern-state     Plan Step over breakpoint trap being discarded in cleanup, it 
says it is already done.
intern-state     Discarding plan: "Step over breakpoint trap", tid = 0x3a7483.
intern-state     Step range plan out of range to 0x55555555513b
intern-state     Plan Step range stepping over being discarded in cleanup, it 
says it is already done.
intern-state     Popping plan: "Step range stepping over", tid = 0x3a7483.
intern-state     Plan stack final state:
  thread #1: tid = 0x3a7483:
    Active plan stack:
      Element 0: Base thread plan.
    Completed plan stack:
      Element 0: Stepping over line ig_c.c:2:7 using ranges: 
[0x0000555555555134-0x000055555555513b).
    Discarded plan stack:
      Element 0: Single stepping past breakpoint site 2 at 0x555555555134
```
As we can see, `lldb-server` sends a `watchpoint stop reason` to us. This is 
expected because before the first instruction of the `main()`, `watch_value` 
was uninitialized and contained garbage, and after the instruction it was 
initialized with 10. `lldb-server` detected the memory access and stopped 
execution with a `watchpoint stop reason`.

`lldb-client` starts its usual `ShouldStop` procedure, which involves walking 
through the thread plan stack. `StepOverThreadPlan` doesn't handle `watchpoint 
stop reason` (it expects `trace stop reason`), therefore we falls through to 
`ThreadPlanBase`.

After that, we initialize `StopInfoWatchpoint` and only in `PerformAction` do 
we discover that the ignore count condition is not satisfied. In that case, we 
don't report the watchpoint hit to the user and continue execution instead. 
Meanwhile `StepOverThreadPlan` has already been marked complete and removed 
from the stack. Therefore, before resuming, we have only the `ThreadPlanBase` 
plan on the stack:

```
intern-state     Plan stack initial state:
  thread #1: tid = 0x3a7483:
    Active plan stack:
      Element 0: Base thread plan.
```
As a result, with only `ThreadPlanBase` remaining, execution resumes without 
stopping.
\
\
\
The fundamental problem for me is that watchpoint checks are performed too late.

This behavior led me to believe that we should perform **all** 
watchpoint/breakpoint checks elsewhere, **before** creating 
`StopInfo{Watchpoint, Breakpoint}`.

I'm curious to hear your thoughts about the issue. Do you see this as a valid 
concern?

https://github.com/llvm/llvm-project/pull/163695
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to