Re: [patch] fix for PR2424

[Aleksandar Ristovski <aristovski at qnx dot com>]

Vladimir Prus wrote:
> The GDB output you have provided above actually includes thread id, so what problem
> does CDT have with figuring thread? In fact, CDT4's RxThread.java has the following:
>
> 		// We were stopped for some unknown reason, for example
> 		// GDB for temporary breakpoints will not send the
> 		// "reason" ??? still fire a stopped event.
> 		if (list.isEmpty()) {
> 			if (session.getMIInferior().isRunning()) {
> 				session.getMIInferior().setSuspended();
> 				MIEvent event = new MIStoppedEvent(session, rr);
> 				session.fireEvent(event);
> 			}
> 		}
>
> > in addition to  
> > not knowing the reason (I am not working on CDT but I was explained that missing 
> > "reason" is to blame, and after the patch I proposed I was told things now work 
> > as expected).
>
> So, could it be a CDT issue, after all?

I believe the problem is that all threads will be reported as suspended, but 
there is no distinction between the thread that hit the breakpoint and other 
threads suspended due to stop. Normally, ide will show something like 
(Breakpoint-hit) beside "Suspended" next to thread number that hit the 
breakpoint. But I will double check with people working on CDT.

> > @Nick: I think the breakpoint should be reported. The fact that it is temporary 
> > doesn't make it much different than a regular breakpoint... but maybe I'm 
> > missing something.
>
> Independent of actually CDT issue, I still think accurately reporting stop reason
> would be good. Can we probably look at breakpoints 'disp' field and either
> print "Breakpoint" or "Temporary breakpoint", and likewise either "breakpoint-hit" or
> "temporary-breakpoint-hit", in breakpoint.c:print_it_typical?

I made the changes, it is not a problem. I have, however, made two versions. The 
first as suggested by Vladimir, the second slightly different but with the same 
goal.

------------- version 1 -------------------
CLI:

(gdb) tbreak main
Temporary breakpoint 1 at 0x80483a0: file ./main.c, line 15.
(gdb) r
Starting program: /space/src/testcases/sigsegv/main

Temporary breakpoint 1, main () at ./main.c:15
15        foo (p);

MI:
(gdb)
-break-insert -t main
^done,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x080483a0",func="main",file="./main.c",fullname="/space/src/testcases/sigsegv/main.c",line="15",times="0"}
(gdb)
-exec-run
^running
(gdb)
*stopped,reason="temporary-breakpoint-hit",bkptno="1",thread-id="0",frame={addr="0x080483a0",func="main",args=[],file="./main.c",fullname="/space/src/testcases/sigsegv/main.c",line="15"}
---------------------------------------------

But now that I am sifting through testcases to replace all "Breakpoint..." with 
"Temporary breakpoint..." I am thinking: we need to communicate to the user that 
the breakpoint is of temporary nature, but should not introduce new "look" or 
type for the temporary breakpoint. The same applies for the reason, it should 
still be "breakpoint-hit". This is particularly true for MI.

How about this output:

-------------------- version 2 -----------------------
CLI:
(gdb) tbreak main
Breakpoint (temp.) 1 at 0x80483a0: file ./main.c, line 15.
(gdb) r
Starting program: /space/src/testcases/sigsegv/main

Breakpoint (temp.) 1, main () at ./main.c:15
15        foo (p);

MI:
(gdb)
-break-insert -t main
^done,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x080483a0",func="main",file="./main.c",fullname="/space/src/testcases/sigsegv/main.c",line="15",times="0"}
(gdb)
-exec-run
^running
(gdb)
*stopped,reason="breakpoint-hit",disp="del",bkptno="1",thread-id="0",frame={addr="0x080483a0",func="main",args=[],file="./main.c",fullname="/space/src/testcases/sigsegv/main.c",line="15"}
---------------------------------------------------------

I think the second version would introduce less impact to existing frontends 
while clarifying the output in a more consistent way.

Thoughts?