Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] stack.c: Always set current_source_{symtab,line}
@ 2002-08-14 16:25 Keith Seitz
  2002-08-22 11:09 ` Keith Seitz
  2002-08-29  8:07 ` Elena Zannoni
  0 siblings, 2 replies; 13+ messages in thread
From: Keith Seitz @ 2002-08-14 16:25 UTC (permalink / raw)
  To: gdb-patches

Hi,

Another MI fallout. This has changed a few times in the past few months, 
but I believe that this is how it should be for anything that uses the 
CLI/command interpreter.

Without this, an -interpreter-exec call in MI will get "lost". (Use MI to 
stop at breakpoint, step a few times, use console interp to "break" -- 
breakpoint would be set someplace other than current location.)

I've run this through the testsuite, and it introduces no new failures. 
(And of course, it still works with Insight.)

Keith

ChangeLog
2002-08-14  Keith Seitz  <keiths@redhat.com>

        * stack.c (print_frame_info_base): Always set current_source_symtab
        and current_source_line.

Patch
Index: stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.40
diff -p -r1.40 stack.c
*** stack.c	11 Jul 2002 19:29:08 -0000	1.40
--- stack.c	14 Aug 2002 23:17:59 -0000
*************** print_frame_info_base (struct frame_info
*** 398,403 ****
--- 398,408 ----
      print_frame (fi, level, source, args, sal);
  
    source_print = (source == SRC_LINE || source == SRC_AND_LOC);
+   if (sal.symtab != NULL)
+     {
+       current_source_symtab = sal.symtab;
+       current_source_line = sal.line;
+     }
  
    if (source_print && sal.symtab)
      {
*************** print_frame_info_base (struct frame_info
*** 410,419 ****
        if (!done)
  	{
  	  if (print_frame_info_listing_hook)
! 	    {
! 	      print_frame_info_listing_hook (sal.symtab, sal.line, sal.line + 1, 0);
! 	      current_source_symtab = sal.symtab;
! 	    }
  	  else
  	    {
  	      /* We used to do this earlier, but that is clearly
--- 415,421 ----
        if (!done)
  	{
  	  if (print_frame_info_listing_hook)
! 	    print_frame_info_listing_hook (sal.symtab, sal.line, sal.line + 1, 0);
  	  else
  	    {
  	      /* We used to do this earlier, but that is clearly




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFA] stack.c: Always set current_source_{symtab,line}
  2002-08-14 16:25 [RFA] stack.c: Always set current_source_{symtab,line} Keith Seitz
@ 2002-08-22 11:09 ` Keith Seitz
  2002-08-26 15:57   ` Kevin Buettner
  2002-08-29  8:07 ` Elena Zannoni
  1 sibling, 1 reply; 13+ messages in thread
From: Keith Seitz @ 2002-08-22 11:09 UTC (permalink / raw)
  To: gdb-patches

Ping.

On Wed, 14 Aug 2002, Keith Seitz wrote:

> Hi,
> 
> Another MI fallout. This has changed a few times in the past few months, 
> but I believe that this is how it should be for anything that uses the 
> CLI/command interpreter.
> 
> Without this, an -interpreter-exec call in MI will get "lost". (Use MI to 
> stop at breakpoint, step a few times, use console interp to "break" -- 
> breakpoint would be set someplace other than current location.)
> 
> I've run this through the testsuite, and it introduces no new failures. 
> (And of course, it still works with Insight.)
> 
> Keith
> 
> ChangeLog
> 2002-08-14  Keith Seitz  <keiths@redhat.com>
> 
>         * stack.c (print_frame_info_base): Always set current_source_symtab
>         and current_source_line.
> 
> Patch
> Index: stack.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/stack.c,v
> retrieving revision 1.40
> diff -p -r1.40 stack.c
> *** stack.c	11 Jul 2002 19:29:08 -0000	1.40
> --- stack.c	14 Aug 2002 23:17:59 -0000
> *************** print_frame_info_base (struct frame_info
> *** 398,403 ****
> --- 398,408 ----
>       print_frame (fi, level, source, args, sal);
>   
>     source_print = (source == SRC_LINE || source == SRC_AND_LOC);
> +   if (sal.symtab != NULL)
> +     {
> +       current_source_symtab = sal.symtab;
> +       current_source_line = sal.line;
> +     }
>   
>     if (source_print && sal.symtab)
>       {
> *************** print_frame_info_base (struct frame_info
> *** 410,419 ****
>         if (!done)
>   	{
>   	  if (print_frame_info_listing_hook)
> ! 	    {
> ! 	      print_frame_info_listing_hook (sal.symtab, sal.line, sal.line + 1, 0);
> ! 	      current_source_symtab = sal.symtab;
> ! 	    }
>   	  else
>   	    {
>   	      /* We used to do this earlier, but that is clearly
> --- 415,421 ----
>         if (!done)
>   	{
>   	  if (print_frame_info_listing_hook)
> ! 	    print_frame_info_listing_hook (sal.symtab, sal.line, sal.line + 1, 0);
>   	  else
>   	    {
>   	      /* We used to do this earlier, but that is clearly
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFA] stack.c: Always set current_source_{symtab,line}
  2002-08-22 11:09 ` Keith Seitz
@ 2002-08-26 15:57   ` Kevin Buettner
  0 siblings, 0 replies; 13+ messages in thread
From: Kevin Buettner @ 2002-08-26 15:57 UTC (permalink / raw)
  To: Keith Seitz, gdb-patches

On Aug 22, 11:05am, Keith Seitz wrote:

> Ping.
> 
> On Wed, 14 Aug 2002, Keith Seitz wrote:
> 
> > Hi,
> > 
> > Another MI fallout. This has changed a few times in the past few months, 
> > but I believe that this is how it should be for anything that uses the 
> > CLI/command interpreter.
> > 
> > Without this, an -interpreter-exec call in MI will get "lost". (Use MI to 
> > stop at breakpoint, step a few times, use console interp to "break" -- 
> > breakpoint would be set someplace other than current location.)
> > 
> > I've run this through the testsuite, and it introduces no new failures. 
> > (And of course, it still works with Insight.)
> > 
> > Keith
> > 
> > ChangeLog
> > 2002-08-14  Keith Seitz  <keiths@redhat.com>
> > 
> >         * stack.c (print_frame_info_base): Always set current_source_symtab
> >         and current_source_line.

Keith,

I don't claim any deep understanding of the code affected, but I've
looked over your patch and it seems reasonable to me.

Kevin


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFA] stack.c: Always set current_source_{symtab,line}
  2002-08-14 16:25 [RFA] stack.c: Always set current_source_{symtab,line} Keith Seitz
  2002-08-22 11:09 ` Keith Seitz
@ 2002-08-29  8:07 ` Elena Zannoni
  2002-08-29  9:10   ` Keith Seitz
  1 sibling, 1 reply; 13+ messages in thread
From: Elena Zannoni @ 2002-08-29  8:07 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches

Keith Seitz writes:
 > Hi,
 > 
 > Another MI fallout. This has changed a few times in the past few months, 
 > but I believe that this is how it should be for anything that uses the 
 > CLI/command interpreter.
 > 
 > Without this, an -interpreter-exec call in MI will get "lost". (Use MI to 
 > stop at breakpoint, step a few times, use console interp to "break" -- 
 > breakpoint would be set someplace other than current location.)
 > 

Seems an ok patch, but do you have an example command sequence
before/after that shows where things go wrong? I am having a bit of a
hard time visualizing the interaction.

Elena


 > I've run this through the testsuite, and it introduces no new failures. 
 > (And of course, it still works with Insight.)
 > 
 > Keith
 > 
 > ChangeLog
 > 2002-08-14  Keith Seitz  <keiths@redhat.com>
 > 
 >         * stack.c (print_frame_info_base): Always set current_source_symtab
 >         and current_source_line.
 > 
 > Patch
 > Index: stack.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/stack.c,v
 > retrieving revision 1.40
 > diff -p -r1.40 stack.c
 > *** stack.c	11 Jul 2002 19:29:08 -0000	1.40
 > --- stack.c	14 Aug 2002 23:17:59 -0000
 > *************** print_frame_info_base (struct frame_info
 > *** 398,403 ****
 > --- 398,408 ----
 >       print_frame (fi, level, source, args, sal);
 >   
 >     source_print = (source == SRC_LINE || source == SRC_AND_LOC);
 > +   if (sal.symtab != NULL)
 > +     {
 > +       current_source_symtab = sal.symtab;
 > +       current_source_line = sal.line;
 > +     }
 >   
 >     if (source_print && sal.symtab)
 >       {
 > *************** print_frame_info_base (struct frame_info
 > *** 410,419 ****
 >         if (!done)
 >   	{
 >   	  if (print_frame_info_listing_hook)
 > ! 	    {
 > ! 	      print_frame_info_listing_hook (sal.symtab, sal.line, sal.line + 1, 0);
 > ! 	      current_source_symtab = sal.symtab;
 > ! 	    }
 >   	  else
 >   	    {
 >   	      /* We used to do this earlier, but that is clearly
 > --- 415,421 ----
 >         if (!done)
 >   	{
 >   	  if (print_frame_info_listing_hook)
 > ! 	    print_frame_info_listing_hook (sal.symtab, sal.line, sal.line + 1, 0);
 >   	  else
 >   	    {
 >   	      /* We used to do this earlier, but that is clearly
 > 
 > 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFA] stack.c: Always set current_source_{symtab,line}
  2002-08-29  8:07 ` Elena Zannoni
@ 2002-08-29  9:10   ` Keith Seitz
  2002-08-29 12:32     ` Elena Zannoni
  2002-08-29 20:28     ` Elena Zannoni
  0 siblings, 2 replies; 13+ messages in thread
From: Keith Seitz @ 2002-08-29  9:10 UTC (permalink / raw)
  To: Elena Zannoni; +Cc: gdb-patches

On Thu, 29 Aug 2002, Elena Zannoni wrote:

> Seems an ok patch, but do you have an example command sequence
> before/after that shows where things go wrong? I am having a bit of a
> hard time visualizing the interaction.

Sure. From the interpreter branch I've been using. [Sorry, I had the 
sequence wrong. The problem is with the "list" command, not "break". We 
had to do something similar in Insight, aka print_frame_info_listing_hook. 
If this patch gets approved, I can whack that hook (from Insight, at 
least).]

(I've removed all the event notifications to facilitate reading)

Without patch:
$ ./gdb -nx -q -i=mi gdb
(gdb)
-break-insert main
^done
(gdb)
-exec-run
^running
(gdb)
*stopped,reason="breakpoint-hit",bkptno="1",thread-id="0",frame={addr="0x08075006",func="main",args=[{name="argc",value="1"},{name="argv",value="0xbffff544"}],file="../../src/gdb/main.c",line="743"}
(gdb)
-break-insert catcher
=breakpoint-create,number="2"
^done
(gdb)
-exec-continue
^running
(gdb)
*stopped,reason="breakpoint-hit",bkptno="2",thread-id="0",frame={addr="0x080fa8d9",func="catcher",args=[{name="func",value="0x80faad0 
<do_catch_errors>"},{name="func_uiout",value="0x829bb60"},{name="func_args",value="0xbffff4a8"},{name="func_val",value="0xbffff4b4"},{name="func_caught",value="0xbffff4b0"},{name="errstring",value="0x8226f20 
\"\""},{name="mask",value="6"}],file="../../src/gdb/top.c",line="401"}
(gdb)
-interpreter-exec console "list"
~"734\t      catch_errors (captured_command_loop, 0, \"\", RETURN_MASK_ALL);\n"
~"735\t    }\n"
~"736\t  /* No exit -- exit is through quit_command.  */\n"
~"737\t}\n"
~"738\t\n"
~"739\tint\n"
~"740\tmain (int argc, char **argv)\n"
~"741\t{\n"
~"742\t  struct captured_main_args args;\n"
~"743\t  args.argc = argc;\n"
^done
(gdb)

Note that we stopped in catcher(), but when we asked the console for a 
"list", it returned main().

With patch:
$ ./gdb -nx -q -i=mi gdb
(gdb)
-break-insert main
^done
(gdb)
-exec-run
^running
(gdb)
*stopped,reason="breakpoint-hit",bkptno="1",thread-id="0",frame={addr="0x08075006",func="main",args=[{name="argc",value="1"},{name="argv",value="0xbfffe744"}],file="../../src/gdb/main.c",line="743"}
(gdb)
-break-insert catcher
^done
(gdb)
-exec-continue
^running
(gdb)
*stopped,reason="breakpoint-hit",bkptno="2",thread-id="0",frame={addr="0x080fa8e9",func="catcher",args=[{name="func",value="0x80faae0 
<do_catch_errors>"},{name="func_uiout",value="0x829bb80"},{name="func_args",value="0xbfffe6a8"},{name="func_val",value="0xbfffe6b4"},{name="func_caught",value="0xbfffe6b0"},{name="errstring",value="0x8226f40 
\"\""},{name="mask",value="6"}],file="../../src/gdb/top.c",line="401"}
(gdb)
-interpreter-exec console list
~"401\t  saved_error_pre_print = error_pre_print;\n"
~"402\t  saved_quit_pre_print = quit_pre_print;\n"
~"403\t\n"
~"404\t  if (mask & RETURN_MASK_ERROR)\n"
~"405\t    error_pre_print = errstring;\n"
~"406\t  if (mask & RETURN_MASK_QUIT)\n"
~"407\t    quit_pre_print = errstring;\n"
~"408\t\n"
~"409\t  /* Override the global ``struct ui_out'' builder.  */\n"
~"410\t\n"
^done
(gdb) 

Now we get the proper source listing from the console.

Keith



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFA] stack.c: Always set current_source_{symtab,line}
  2002-08-29  9:10   ` Keith Seitz
@ 2002-08-29 12:32     ` Elena Zannoni
  2002-08-29 12:43       ` Keith Seitz
  2002-08-29 20:28     ` Elena Zannoni
  1 sibling, 1 reply; 13+ messages in thread
From: Elena Zannoni @ 2002-08-29 12:32 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Elena Zannoni, gdb-patches

Keith Seitz writes:
 > On Thu, 29 Aug 2002, Elena Zannoni wrote:
 > 
 > > Seems an ok patch, but do you have an example command sequence
 > > before/after that shows where things go wrong? I am having a bit of a
 > > hard time visualizing the interaction.
 > 
 > Sure. From the interpreter branch I've been using. [Sorry, I had the 
 > sequence wrong. The problem is with the "list" command, not "break". We 
 > had to do something similar in Insight, aka print_frame_info_listing_hook. 
 > If this patch gets approved, I can whack that hook (from Insight, at 
 > least).]
 > 

The tui uses it as well. Would it be feasible to clean that up too?

Ok, I am going to be a PITA: does this odd behavior occur with non-mi
gdb?  I am not clear on why it is happening only with mi. Or at
least I don't see it in regular gdb.

Elena


 > (I've removed all the event notifications to facilitate reading)
 > 
 > Without patch:
 > $ ./gdb -nx -q -i=mi gdb
 > (gdb)
 > -break-insert main
 > ^done
 > (gdb)
 > -exec-run
 > ^running
 > (gdb)
 > *stopped,reason="breakpoint-hit",bkptno="1",thread-id="0",frame={addr="0x08075006",func="main",args=[{name="argc",value="1"},{name="argv",value="0xbffff544"}],file="../../src/gdb/main.c",line="743"}
 > (gdb)
 > -break-insert catcher
 > =breakpoint-create,number="2"
 > ^done
 > (gdb)
 > -exec-continue
 > ^running
 > (gdb)
 > *stopped,reason="breakpoint-hit",bkptno="2",thread-id="0",frame={addr="0x080fa8d9",func="catcher",args=[{name="func",value="0x80faad0 
 > <do_catch_errors>"},{name="func_uiout",value="0x829bb60"},{name="func_args",value="0xbffff4a8"},{name="func_val",value="0xbffff4b4"},{name="func_caught",value="0xbffff4b0"},{name="errstring",value="0x8226f20 
 > \"\""},{name="mask",value="6"}],file="../../src/gdb/top.c",line="401"}
 > (gdb)
 > -interpreter-exec console "list"
 > ~"734\t      catch_errors (captured_command_loop, 0, \"\", RETURN_MASK_ALL);\n"
 > ~"735\t    }\n"
 > ~"736\t  /* No exit -- exit is through quit_command.  */\n"
 > ~"737\t}\n"
 > ~"738\t\n"
 > ~"739\tint\n"
 > ~"740\tmain (int argc, char **argv)\n"
 > ~"741\t{\n"
 > ~"742\t  struct captured_main_args args;\n"
 > ~"743\t  args.argc = argc;\n"
 > ^done
 > (gdb)
 > 
 > Note that we stopped in catcher(), but when we asked the console for a 
 > "list", it returned main().
 > 
 > With patch:
 > $ ./gdb -nx -q -i=mi gdb
 > (gdb)
 > -break-insert main
 > ^done
 > (gdb)
 > -exec-run
 > ^running
 > (gdb)
 > *stopped,reason="breakpoint-hit",bkptno="1",thread-id="0",frame={addr="0x08075006",func="main",args=[{name="argc",value="1"},{name="argv",value="0xbfffe744"}],file="../../src/gdb/main.c",line="743"}
 > (gdb)
 > -break-insert catcher
 > ^done
 > (gdb)
 > -exec-continue
 > ^running
 > (gdb)
 > *stopped,reason="breakpoint-hit",bkptno="2",thread-id="0",frame={addr="0x080fa8e9",func="catcher",args=[{name="func",value="0x80faae0 
 > <do_catch_errors>"},{name="func_uiout",value="0x829bb80"},{name="func_args",value="0xbfffe6a8"},{name="func_val",value="0xbfffe6b4"},{name="func_caught",value="0xbfffe6b0"},{name="errstring",value="0x8226f40 
 > \"\""},{name="mask",value="6"}],file="../../src/gdb/top.c",line="401"}
 > (gdb)
 > -interpreter-exec console list
 > ~"401\t  saved_error_pre_print = error_pre_print;\n"
 > ~"402\t  saved_quit_pre_print = quit_pre_print;\n"
 > ~"403\t\n"
 > ~"404\t  if (mask & RETURN_MASK_ERROR)\n"
 > ~"405\t    error_pre_print = errstring;\n"
 > ~"406\t  if (mask & RETURN_MASK_QUIT)\n"
 > ~"407\t    quit_pre_print = errstring;\n"
 > ~"408\t\n"
 > ~"409\t  /* Override the global ``struct ui_out'' builder.  */\n"
 > ~"410\t\n"
 > ^done
 > (gdb) 
 > 
 > Now we get the proper source listing from the console.
 > 
 > Keith
 > 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFA] stack.c: Always set current_source_{symtab,line}
  2002-08-29 12:32     ` Elena Zannoni
@ 2002-08-29 12:43       ` Keith Seitz
  2002-08-29 12:54         ` Elena Zannoni
  0 siblings, 1 reply; 13+ messages in thread
From: Keith Seitz @ 2002-08-29 12:43 UTC (permalink / raw)
  To: Elena Zannoni; +Cc: gdb-patches

On Thu, 29 Aug 2002, Elena Zannoni wrote:

> Ok, I am going to be a PITA: does this odd behavior occur with non-mi
> gdb?  I am not clear on why it is happening only with mi. Or at
> least I don't see it in regular gdb.

This happens because we suppress printing the listing when we stop 
(infrun.c/normal_stop):

      if (stop_print_frame && selected_frame)
	{
	  int bpstat_ret;
	  int source_flag;
	  int do_frame_printing = 1;

	  bpstat_ret = bpstat_print (stop_bpstat);
	  switch (bpstat_ret)
	    {
	    case PRINT_UNKNOWN:
	      if (stop_step
		  && step_frame_address == FRAME_FP (get_current_frame ())
		  && step_start_function == find_pc_function (stop_pc))
		source_flag = SRC_LINE;	/* finished step, just print source line */
	      else
		source_flag = SRC_AND_LOC;	/* print location and source line */
	      break;
	    case PRINT_SRC_AND_LOC:
	      source_flag = SRC_AND_LOC;	/* print location and source line */
	      break;
	    case PRINT_SRC_ONLY:
	      source_flag = SRC_LINE;
	      break;
	    case PRINT_NOTHING:
	      source_flag = SRC_LINE;	/* something bogus */
	      do_frame_printing = 0;
	      break;
	    default:
	      internal_error (__FILE__, __LINE__, "Unknown value.");
	    }
	  /* For mi, have the same behavior every time we stop:
	     print everything but the source line. */
	  if (ui_out_is_mi_like_p (uiout))
	    source_flag = LOC_AND_ADDRESS;

We use LOC_AND_ADDRESS to suppress source listings, but a side affect is 
that it doesn't set current_source_symtab, either, because 
current_source_symtab is set in print_source_lines_base, called from 
print_source_lines, from print_frame_info_base:

  source_print = (source == SRC_LINE || source == SRC_AND_LOC);

  if (source_print && sal.symtab)
    {
      int done = 0;
      int mid_statement = (source == SRC_LINE) && (fi->pc != sal.pc);

      if (annotation_level)
	done = identify_source_line (sal.symtab, sal.line, mid_statement,
				     fi->pc);
      if (!done)
	{
	  if (print_frame_info_listing_hook)
	    {
	      print_frame_info_listing_hook (sal.symtab, sal.line, sal.line + 1, 0);
	      current_source_symtab = sal.symtab;
	    }
	  else
	    {
	      /* We used to do this earlier, but that is clearly
		 wrong. This function is used by many different
		 parts of gdb, including normal_stop in infrun.c,
		 which uses this to print out the current PC
		 when we stepi/nexti into the middle of a source
		 line. Only the command line really wants this
		 behavior. Other UIs probably would like the
		 ability to decide for themselves if it is desired. */
	      if (addressprint && mid_statement)
		{
		  ui_out_field_core_addr (uiout, "addr", fi->pc);
		  ui_out_text (uiout, "\t");
		}

	      print_source_lines (sal.symtab, sal.line, sal.line + 1, 0);
	    }
	}
      current_source_line = max (sal.line - lines_to_list / 2, 1);
    }

Keith



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFA] stack.c: Always set current_source_{symtab,line}
  2002-08-29 12:43       ` Keith Seitz
@ 2002-08-29 12:54         ` Elena Zannoni
  2002-08-29 13:16           ` Keith Seitz
  0 siblings, 1 reply; 13+ messages in thread
From: Elena Zannoni @ 2002-08-29 12:54 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Elena Zannoni, gdb-patches

Keith Seitz writes:
 > On Thu, 29 Aug 2002, Elena Zannoni wrote:
 > 
 > > Ok, I am going to be a PITA: does this odd behavior occur with non-mi
 > > gdb?  I am not clear on why it is happening only with mi. Or at
 > > least I don't see it in regular gdb.
 > 
 > This happens because we suppress printing the listing when we stop 
 > (infrun.c/normal_stop):
 > 


Ahhh, right. Ok, I am convinced.
Go ahead.

Elena


 >       if (stop_print_frame && selected_frame)
 > 	{
 > 	  int bpstat_ret;
 > 	  int source_flag;
 > 	  int do_frame_printing = 1;
 > 
 > 	  bpstat_ret = bpstat_print (stop_bpstat);
 > 	  switch (bpstat_ret)
 > 	    {
 > 	    case PRINT_UNKNOWN:
 > 	      if (stop_step
 > 		  && step_frame_address == FRAME_FP (get_current_frame ())
 > 		  && step_start_function == find_pc_function (stop_pc))
 > 		source_flag = SRC_LINE;	/* finished step, just print source line */
 > 	      else
 > 		source_flag = SRC_AND_LOC;	/* print location and source line */
 > 	      break;
 > 	    case PRINT_SRC_AND_LOC:
 > 	      source_flag = SRC_AND_LOC;	/* print location and source line */
 > 	      break;
 > 	    case PRINT_SRC_ONLY:
 > 	      source_flag = SRC_LINE;
 > 	      break;
 > 	    case PRINT_NOTHING:
 > 	      source_flag = SRC_LINE;	/* something bogus */
 > 	      do_frame_printing = 0;
 > 	      break;
 > 	    default:
 > 	      internal_error (__FILE__, __LINE__, "Unknown value.");
 > 	    }
 > 	  /* For mi, have the same behavior every time we stop:
 > 	     print everything but the source line. */
 > 	  if (ui_out_is_mi_like_p (uiout))
 > 	    source_flag = LOC_AND_ADDRESS;
 > 
 > We use LOC_AND_ADDRESS to suppress source listings, but a side affect is 
 > that it doesn't set current_source_symtab, either, because 
 > current_source_symtab is set in print_source_lines_base, called from 
 > print_source_lines, from print_frame_info_base:
 > 
 >   source_print = (source == SRC_LINE || source == SRC_AND_LOC);
 > 
 >   if (source_print && sal.symtab)
 >     {
 >       int done = 0;
 >       int mid_statement = (source == SRC_LINE) && (fi->pc != sal.pc);
 > 
 >       if (annotation_level)
 > 	done = identify_source_line (sal.symtab, sal.line, mid_statement,
 > 				     fi->pc);
 >       if (!done)
 > 	{
 > 	  if (print_frame_info_listing_hook)
 > 	    {
 > 	      print_frame_info_listing_hook (sal.symtab, sal.line, sal.line + 1, 0);
 > 	      current_source_symtab = sal.symtab;
 > 	    }
 > 	  else
 > 	    {
 > 	      /* We used to do this earlier, but that is clearly
 > 		 wrong. This function is used by many different
 > 		 parts of gdb, including normal_stop in infrun.c,
 > 		 which uses this to print out the current PC
 > 		 when we stepi/nexti into the middle of a source
 > 		 line. Only the command line really wants this
 > 		 behavior. Other UIs probably would like the
 > 		 ability to decide for themselves if it is desired. */
 > 	      if (addressprint && mid_statement)
 > 		{
 > 		  ui_out_field_core_addr (uiout, "addr", fi->pc);
 > 		  ui_out_text (uiout, "\t");
 > 		}
 > 
 > 	      print_source_lines (sal.symtab, sal.line, sal.line + 1, 0);
 > 	    }
 > 	}
 >       current_source_line = max (sal.line - lines_to_list / 2, 1);
 >     }
 > 
 > Keith
 > 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFA] stack.c: Always set current_source_{symtab,line}
  2002-08-29 12:54         ` Elena Zannoni
@ 2002-08-29 13:16           ` Keith Seitz
  0 siblings, 0 replies; 13+ messages in thread
From: Keith Seitz @ 2002-08-29 13:16 UTC (permalink / raw)
  To: gdb-patches

On Thu, 29 Aug 2002, Elena Zannoni wrote:

> Ahhh, right. Ok, I am convinced.
> Go ahead.

Thanks, committed.

Keith



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFA] stack.c: Always set current_source_{symtab,line}
  2002-08-29  9:10   ` Keith Seitz
  2002-08-29 12:32     ` Elena Zannoni
@ 2002-08-29 20:28     ` Elena Zannoni
  2002-08-30 11:12       ` Keith Seitz
  1 sibling, 1 reply; 13+ messages in thread
From: Elena Zannoni @ 2002-08-29 20:28 UTC (permalink / raw)
  To: Keith Seitz; +Cc: Elena Zannoni, gdb-patches

Keith Seitz writes:
 > On Thu, 29 Aug 2002, Elena Zannoni wrote:
 > 
 > > Seems an ok patch, but do you have an example command sequence
 > > before/after that shows where things go wrong? I am having a bit of a
 > > hard time visualizing the interaction.
 > 
 > Sure. From the interpreter branch I've been using. [Sorry, I had the 
 > sequence wrong. The problem is with the "list" command, not "break". We 
 > had to do something similar in Insight, aka print_frame_info_listing_hook. 
 > If this patch gets approved, I can whack that hook (from Insight, at 
 > least).]
 > 
 > (I've removed all the event notifications to facilitate reading)
 > 

Another  thought: could a testcase be added, just like the one below?

thanks
Elena


 > Without patch:
 > $ ./gdb -nx -q -i=mi gdb
 > (gdb)
 > -break-insert main
 > ^done
 > (gdb)
 > -exec-run
 > ^running
 > (gdb)
 > *stopped,reason="breakpoint-hit",bkptno="1",thread-id="0",frame={addr="0x08075006",func="main",args=[{name="argc",value="1"},{name="argv",value="0xbffff544"}],file="../../src/gdb/main.c",line="743"}
 > (gdb)
 > -break-insert catcher
 > =breakpoint-create,number="2"
 > ^done
 > (gdb)
 > -exec-continue
 > ^running
 > (gdb)
 > *stopped,reason="breakpoint-hit",bkptno="2",thread-id="0",frame={addr="0x080fa8d9",func="catcher",args=[{name="func",value="0x80faad0 
 > <do_catch_errors>"},{name="func_uiout",value="0x829bb60"},{name="func_args",value="0xbffff4a8"},{name="func_val",value="0xbffff4b4"},{name="func_caught",value="0xbffff4b0"},{name="errstring",value="0x8226f20 
 > \"\""},{name="mask",value="6"}],file="../../src/gdb/top.c",line="401"}
 > (gdb)
 > -interpreter-exec console "list"
 > ~"734\t      catch_errors (captured_command_loop, 0, \"\", RETURN_MASK_ALL);\n"
 > ~"735\t    }\n"
 > ~"736\t  /* No exit -- exit is through quit_command.  */\n"
 > ~"737\t}\n"
 > ~"738\t\n"
 > ~"739\tint\n"
 > ~"740\tmain (int argc, char **argv)\n"
 > ~"741\t{\n"
 > ~"742\t  struct captured_main_args args;\n"
 > ~"743\t  args.argc = argc;\n"
 > ^done
 > (gdb)
 > 
 > Note that we stopped in catcher(), but when we asked the console for a 
 > "list", it returned main().
 > 
 > With patch:
 > $ ./gdb -nx -q -i=mi gdb
 > (gdb)
 > -break-insert main
 > ^done
 > (gdb)
 > -exec-run
 > ^running
 > (gdb)
 > *stopped,reason="breakpoint-hit",bkptno="1",thread-id="0",frame={addr="0x08075006",func="main",args=[{name="argc",value="1"},{name="argv",value="0xbfffe744"}],file="../../src/gdb/main.c",line="743"}
 > (gdb)
 > -break-insert catcher
 > ^done
 > (gdb)
 > -exec-continue
 > ^running
 > (gdb)
 > *stopped,reason="breakpoint-hit",bkptno="2",thread-id="0",frame={addr="0x080fa8e9",func="catcher",args=[{name="func",value="0x80faae0 
 > <do_catch_errors>"},{name="func_uiout",value="0x829bb80"},{name="func_args",value="0xbfffe6a8"},{name="func_val",value="0xbfffe6b4"},{name="func_caught",value="0xbfffe6b0"},{name="errstring",value="0x8226f40 
 > \"\""},{name="mask",value="6"}],file="../../src/gdb/top.c",line="401"}
 > (gdb)
 > -interpreter-exec console list
 > ~"401\t  saved_error_pre_print = error_pre_print;\n"
 > ~"402\t  saved_quit_pre_print = quit_pre_print;\n"
 > ~"403\t\n"
 > ~"404\t  if (mask & RETURN_MASK_ERROR)\n"
 > ~"405\t    error_pre_print = errstring;\n"
 > ~"406\t  if (mask & RETURN_MASK_QUIT)\n"
 > ~"407\t    quit_pre_print = errstring;\n"
 > ~"408\t\n"
 > ~"409\t  /* Override the global ``struct ui_out'' builder.  */\n"
 > ~"410\t\n"
 > ^done
 > (gdb) 
 > 
 > Now we get the proper source listing from the console.
 > 
 > Keith
 > 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFA] stack.c: Always set current_source_{symtab,line}
  2002-08-29 20:28     ` Elena Zannoni
@ 2002-08-30 11:12       ` Keith Seitz
  2002-08-30 12:44         ` Andrew Cagney
  0 siblings, 1 reply; 13+ messages in thread
From: Keith Seitz @ 2002-08-30 11:12 UTC (permalink / raw)
  To: gdb-patches

On Thu, 29 Aug 2002, Elena Zannoni wrote:

> Another  thought: could a testcase be added, just like the one below?

Already in mi-cli.exp on the branch. (I think I submitted it, too, to this 
list.) The method is different, but it demonstrates the failure using 
"list" with the console interpreter.

Keith



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFA] stack.c: Always set current_source_{symtab,line}
  2002-08-30 11:12       ` Keith Seitz
@ 2002-08-30 12:44         ` Andrew Cagney
  2002-08-30 12:57           ` Keith Seitz
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Cagney @ 2002-08-30 12:44 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches, Elena Zannoni

> On Thu, 29 Aug 2002, Elena Zannoni wrote:
> 
> 
>> Another  thought: could a testcase be added, just like the one below?
> 
> 
> Already in mi-cli.exp on the branch. (I think I submitted it, too, to this 
> list.) The method is different, but it demonstrates the failure using 
> "list" with the console interpreter.

For MI, the testcase needs to go in at the same time as the fix (this is 
stronger then the rest of GDB where it is a ``should'').  It's also 
important to check that the test actually fails without the fix.  That 
way we know that the change at least fixed something :-)

enjoy,
Andrew



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFA] stack.c: Always set current_source_{symtab,line}
  2002-08-30 12:44         ` Andrew Cagney
@ 2002-08-30 12:57           ` Keith Seitz
  0 siblings, 0 replies; 13+ messages in thread
From: Keith Seitz @ 2002-08-30 12:57 UTC (permalink / raw)
  To: gdb-patches

On Fri, 30 Aug 2002, Andrew Cagney wrote:

> > Already in mi-cli.exp on the branch. (I think I submitted it, too, to this 
> > list.) The method is different, but it demonstrates the failure using 
> > "list" with the console interpreter.
> 
> For MI, the testcase needs to go in at the same time as the fix (this is 
> stronger then the rest of GDB where it is a ``should'').  It's also 
> important to check that the test actually fails without the fix.  That 
> way we know that the change at least fixed something :-)

Ok, but, as with all of this, it is IMPOSSIBLE to get this testcase to do 
anything UNTIL all the interps stuff is in. This bug only affects the 
command "-interpreter-exec console list". Well, this command doesn't yet 
exist in MI, because the infrastructure has not been approved for trunk.

Yes, I have confirmed that there is a difference with and without the 
patch on my branch. mi-cli.exp shows an additional failure without the 
patch.

I'm really at a loss about all this. Testcases, documentation, just about 
everything exists on the (public) branch. I can submit (and already did, 
in this case) a testcase to demonstrate the bug. The problem is that you 
can't run it at all in CVS head right now -- not until all the rest of the 
changes have been approved and committed.

Help me out here. I'm obviously approaching the approval process 
incorrectly with these (relatively) small, one-at-a-time changes. Is there 
something I can do to facilitate review/approval of the mega-changes 
needed to support interpreters in gdb? Can I do something better? Is 
there a better process that I can follow?

Keith


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2002-08-30 19:47 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-08-14 16:25 [RFA] stack.c: Always set current_source_{symtab,line} Keith Seitz
2002-08-22 11:09 ` Keith Seitz
2002-08-26 15:57   ` Kevin Buettner
2002-08-29  8:07 ` Elena Zannoni
2002-08-29  9:10   ` Keith Seitz
2002-08-29 12:32     ` Elena Zannoni
2002-08-29 12:43       ` Keith Seitz
2002-08-29 12:54         ` Elena Zannoni
2002-08-29 13:16           ` Keith Seitz
2002-08-29 20:28     ` Elena Zannoni
2002-08-30 11:12       ` Keith Seitz
2002-08-30 12:44         ` Andrew Cagney
2002-08-30 12:57           ` Keith Seitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox