Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFC] Unexpected automatic language switch - get_frame_language()
@ 2003-12-05 22:48 Joel Brobecker
  2003-12-05 23:57 ` Michael Snyder
  2003-12-10 17:47 ` Daniel Jacobowitz
  0 siblings, 2 replies; 15+ messages in thread
From: Joel Brobecker @ 2003-12-05 22:48 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 5131 bytes --]

Hello,

here is a little problem I just spotted today. Given the little Ada
program attached, and compiled with the following command:

        % gnatamke -g a -bargs -shared

(so build with the shared version of the GNAT runtime)

The following GDB transcript demonstrates that GDB unexpectedly
switches the current language to C upon hitting the third exception
breakpoint:

        (gdb) begin
        Breakpoint 1 at 0x8049697: file a.adb, line 6.
        a () at a.adb:6
        6             raise Program_Error;
        (gdb) b exception
        Breakpoint 2 at 0x40091342: file a-except.adb, line 865.
        (gdb) c
        Continuing.

        Breakpoint 2, PROGRAM_ERROR at 0x080496ab in a () at a.adb:6
        6             raise Program_Error;
        (gdb) c
        Continuing.

        Breakpoint 2, CONSTRAINT_ERROR at 0x0804971a in a () at a.adb:14
        14            raise Constraint_Error;
        (gdb) c
        Continuing.

        Breakpoint 2, CONSTRAINT_ERROR at 0x08049790 in a () at a.adb:24
        24            Var := - Var;
  -->   Current language:  auto; currently c

Some additional information about "break exception". We have slightly
modified the handling of the "break" command when in ada mode to special
case "break exception". This places a breakpoint on a known GNAT runtime
routine that's called upon exception raise. That's more or less how
exception breakpoints are implemented for Ada.

For the user's convenience, when the breakpoint is hit, we automatically
go up the call stack until we find a "user frame" (meaning a frame which
has debug info and is not inside the GNAT runtime), and select that
frame. So the user usually sees the location where the exception was
raised, instead of the runtime machinery that triggers and handles the
exception raise.

Back to the problem above, at the third hit of the exception breakpoin,
I found that GDB automatically selects frame #4. While selecting that
frame, GDB tries to find its associated language, and finds that it is
C, not Ada!

Why would that be? We need to look at the code in frame.c:select_frame()
for that:

    /* Ensure that symbols for this frame are read in.  Also, determine the
       source language of this frame, and switch to it if desired.  */
    if (fi)
      {
        s = find_pc_symtab (get_frame_pc (fi));
        if (s
            && s->language != current_language->la_language
            && s->language != language_unknown
            && language_mode == language_mode_auto)
          {
            set_language (s->language);
          }
      }

So we determine the language by looking up the symtab associated to
the given frame pc. Unfortunately, we were not very lucky in our case
because the call instruction was the last instruction of the function.
And because get_frame_pc actually gives a return address (except for
the bottom frame), the pc return is actually pointing to a different
function. And this is where we're not lucky for the second time, because
the next function is in a different unit written in a different
language!

Witness:

    (gdb) p /x $pc
    $1 = 0x8049790
    (gdb) disass $pc
    Dump of assembler code for function size_of_encoded_value:
    0x08049790 <size_of_encoded_value+0>:   push   %ebp
    0x08049791 <size_of_encoded_value+1>:   mov    %esp,%ebp
    0x08049793 <size_of_encoded_value+3>:   sub    $0x8,%esp

And then a confirmation that the call is the last instruction of
our function:

    (gdb) disass $pc-1
    Dump of assembler code for function _ada_a:
    [...]
    0x08049784 <_ada_a+244>:        movl   $0x804cd80,(%esp,1)
    0x0804978b <_ada_a+251>:        call   0x8049210
    End of assembler dump.

So I think the correct way of doing this is to use a decremented PC
for any frame but the bottom one. Something like this:

    if (fi)
      {
        CORE_ADDR pc = get_frame_pc (fi);
        
        if (frame_relative_level (fi) > 0)
          pc--; /* Will add a comment why.  */
        s = find_pc_symtab (pc);
        [... etc ...]
      }

I experimented this change and it works, except that now I get a warning
that the current language does not match the frame language. Ah, same
mistake in execute_command that calls get_frame_language:

      s = find_pc_symtab (get_frame_pc (deprecated_selected_frame));
      if (s)
        flang = s->language;

So my suggestion is:
  * First patch:
     1. Upgrade get_selected_frame to take a frame as an argument,
        instead of using "deprecated_selected_frame"
     2. We can define a new function named "get_selected_frame_language"
        that would essentially be equal to:
              get_frame_language (get_selected_frame ()).
        This is really not necessary, but people may like the argless
        function
     3. Update all current calls to get_frame_language (not that many)
  * Second patch:
     1. Fix the PC problem in get_frame_language()
     2. Update select_frame to use get_frame_language to get the frame
        language.

Sounds OK?

-- 
Joel

PS: I couldn't reduce the testcase more, the test is too sensitive to
code generation and placement...

[-- Attachment #2: a.adb --]
[-- Type: text/plain, Size: 444 bytes --]

procedure A is
begin

   --  Raise Program_Error and catch it.
   begin
      raise Program_Error;
   exception
      when others =>
         null;
   end;

   --  Raise Constraint_Error and catch it.
   Begin
      raise Constraint_Error;
   Exception
      when others =>
         null;
   End;

   --  Cause an exception to be raised, and do not handle it.
   declare
      Var : Natural := 10;
   begin
      Var := - Var;
   end;

end A;


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

* Re: [RFC] Unexpected automatic language switch - get_frame_language()
  2003-12-05 22:48 [RFC] Unexpected automatic language switch - get_frame_language() Joel Brobecker
@ 2003-12-05 23:57 ` Michael Snyder
  2003-12-06  0:18   ` Joel Brobecker
  2003-12-10 17:47 ` Daniel Jacobowitz
  1 sibling, 1 reply; 15+ messages in thread
From: Michael Snyder @ 2003-12-05 23:57 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

Joel Brobecker wrote:
> 
> So we determine the language by looking up the symtab associated to
> the given frame pc. Unfortunately, we were not very lucky in our case
> because the call instruction was the last instruction of the function.
> And because get_frame_pc actually gives a return address (except for
> the bottom frame), the pc return is actually pointing to a different
> function. And this is where we're not lucky for the second time, because
> the next function is in a different unit written in a different
> language!


Sounds kinda like a debugging-optimized-code problem.  Your function
is tail-return optimized -- doesn't really return.  That's outside
the expected API.

I think you have to do "special" things for non-returning functions.
I've seen the same sort of thing for eg. _exit.

[...]
> 
> So I think the correct way of doing this is to use a decremented PC
> for any frame but the bottom one. 

I think that's fixing the wrong problem.  And it's not really portable.

[...]
 > PS: I couldn't reduce the testcase more, the test is too sensitive
 > to code generation and placement...

That's generally a good sign of an optimization problem.




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

* Re: [RFC] Unexpected automatic language switch - get_frame_language()
  2003-12-05 23:57 ` Michael Snyder
@ 2003-12-06  0:18   ` Joel Brobecker
  2003-12-06  0:28     ` Andrew Cagney
  2003-12-06  0:42     ` Michael Snyder
  0 siblings, 2 replies; 15+ messages in thread
From: Joel Brobecker @ 2003-12-06  0:18 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches

> Sounds kinda like a debugging-optimized-code problem.  Your function
> is tail-return optimized -- doesn't really return.  That's outside
> the expected API.

Kind of, yes,  but the code in question was compiled at -O0! It's a
noreturn function because the compiler could statically determine that
the exception would be unhandled.

> I think you have to do "special" things for non-returning functions.
> I've seen the same sort of thing for eg. _exit.

Do you remember how these similar problems were approached and solved?

> >So I think the correct way of doing this is to use a decremented PC
> >for any frame but the bottom one. 
> 
> I think that's fixing the wrong problem.  And it's not really portable.

Hmm, I thought this was "portable" since I sort of remember that we are
already using this sort of technique in similar situations. I can't
remember exactly which changes, but decrementing the PC was something we
already did in other places. Maybe your objecting to the part that says
"do the decrement only for the non bottom one"?

-- 
Joel


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

* Re: [RFC] Unexpected automatic language switch - get_frame_language()
  2003-12-06  0:18   ` Joel Brobecker
@ 2003-12-06  0:28     ` Andrew Cagney
  2003-12-10  1:50       ` Joel Brobecker
  2003-12-06  0:42     ` Michael Snyder
  1 sibling, 1 reply; 15+ messages in thread
From: Andrew Cagney @ 2003-12-06  0:28 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Michael Snyder, gdb-patches

>> Sounds kinda like a debugging-optimized-code problem.  Your function
>> is tail-return optimized -- doesn't really return.  That's outside
>> the expected API.
> 
> 
> Kind of, yes,  but the code in question was compiled at -O0! It's a
> noreturn function because the compiler could statically determine that
> the exception would be unhandled.
> 
> 
>> I think you have to do "special" things for non-returning functions.
>> I've seen the same sort of thing for eg. _exit.
> 
> 
> Do you remember how these similar problems were approached and solved?
> 
> 
>> >So I think the correct way of doing this is to use a decremented PC
>> >for any frame but the bottom one. 
> 
>> 
>> I think that's fixing the wrong problem.  And it's not really portable.

Looked at frame_addr_in_block?

> Hmm, I thought this was "portable" since I sort of remember that we are
> already using this sort of technique in similar situations. I can't
> remember exactly which changes, but decrementing the PC was something we
> already did in other places. Maybe your objecting to the part that says
> "do the decrement only for the non bottom one"?
> 
> -- Joel 



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

* Re: [RFC] Unexpected automatic language switch - get_frame_language()
  2003-12-06  0:18   ` Joel Brobecker
  2003-12-06  0:28     ` Andrew Cagney
@ 2003-12-06  0:42     ` Michael Snyder
  1 sibling, 0 replies; 15+ messages in thread
From: Michael Snyder @ 2003-12-06  0:42 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

Joel Brobecker wrote:
>>Sounds kinda like a debugging-optimized-code problem.  Your function
>>is tail-return optimized -- doesn't really return.  That's outside
>>the expected API.
> 
> 
> Kind of, yes,  but the code in question was compiled at -O0!

Never the less...

> It's a
> noreturn function because the compiler could statically determine that
> the exception would be unhandled.

Maybe the compiler shouldn't do that at -O0?


>>I think you have to do "special" things for non-returning functions.
>>I've seen the same sort of thing for eg. _exit.
> 
> 
> Do you remember how these similar problems were approached and solved?

Ummm...  ok, this is embarassing.  Well, note that this was written
(by me) for a specific target architecture, and never expected to be
portable.  This little piece of work was called from
iq2000_frame_saved_pc:


/* Function: horrible_noreturn_hack
    If the return address points to the first instruction of a function,
    decrement the return address by one instruction.  */

static void
iq2000_horrible_noreturn_hack (struct frame_info *fi)
{
   /* Some functions (notably those that are not expected to return)
      seem to end with a jump_and_link instruction (plus a no-op).
      When this happens, the return address seems to be the first
      instruction of the next function.  This messes up the backtrace.

      I can think of no legitimate way that the return address
      could actually be the first address of a function,
      [NOTE: except for the entry point address for a call dummy],
      so if we detect this, we'll decrement it by one instruction
      (so that it hopefully points into the previous function). */

   if (fi && fi->extra_info && fi->extra_info->frame_cached_pc)
     if (get_pc_function_start (fi->extra_info->frame_cached_pc) ==
         fi->extra_info->frame_cached_pc)
       if (fi->extra_info->frame_cached_pc != entry_point_address ())
         fi->extra_info->frame_cached_pc -= SIZEOF_IQ2000_INSN;
}



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

* Re: [RFC] Unexpected automatic language switch - get_frame_language()
  2003-12-06  0:28     ` Andrew Cagney
@ 2003-12-10  1:50       ` Joel Brobecker
  2003-12-10 16:14         ` Andrew Cagney
  0 siblings, 1 reply; 15+ messages in thread
From: Joel Brobecker @ 2003-12-10  1:50 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: Michael Snyder, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 508 bytes --]

> Looked at frame_addr_in_block?

That's (almost) the function I was trying to remember about. Thanks a
lot!  The following patch addresses the problem in a way that I hope
will satisfy MichaelS' concerns.

2003-12-09  J. Brobecker  <brobecker@gnat.com>

        * frame.c (select_frame): Get the current frame PC using
        get_frame_address_in_block() instead of get_frame_pc().
        * stack.c (get_frame_language): Likewise.

Tested on x86-linux, with no regression.

OK to apply?

Thanks,
-- 
Joel

[-- Attachment #2: addr_in_block.diff --]
[-- Type: text/plain, Size: 1180 bytes --]

Index: frame.c
===================================================================
RCS file: /nile.c/cvs/Dev/gdb/gdb-6.0/gdb/frame.c,v
retrieving revision 1.2
diff -u -p -r1.2 frame.c
--- frame.c	6 Oct 2003 05:20:55 -0000	1.2
+++ frame.c	9 Dec 2003 00:19:00 -0000
@@ -985,7 +985,7 @@ select_frame (struct frame_info *fi)
      source language of this frame, and switch to it if desired.  */
   if (fi)
     {
-      s = find_pc_symtab (get_frame_pc (fi));
+      s = find_pc_symtab (get_frame_address_in_block (fi));
       if (s
 	  && s->language != current_language->la_language
 	  && s->language != language_unknown
Index: stack.c
===================================================================
RCS file: /nile.c/cvs/Dev/gdb/gdb-6.0/gdb/stack.c,v
retrieving revision 1.3
diff -u -p -r1.3 stack.c
--- stack.c	13 Nov 2003 11:56:27 -0000	1.3
+++ stack.c	9 Dec 2003 00:20:14 -0000
@@ -2047,7 +2047,7 @@ get_frame_language (void)
 
   if (deprecated_selected_frame)
     {
-      s = find_pc_symtab (get_frame_pc (deprecated_selected_frame));
+      s = find_pc_symtab (get_frame_address_in_block (deprecated_selected_frame));
       if (s)
 	flang = s->language;
       else

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

* Re: [RFC] Unexpected automatic language switch - get_frame_language()
  2003-12-10  1:50       ` Joel Brobecker
@ 2003-12-10 16:14         ` Andrew Cagney
  2003-12-10 17:42           ` Joel Brobecker
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Cagney @ 2003-12-10 16:14 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Michael Snyder, gdb-patches

> Looked at frame_addr_in_block?
> 
> 
> That's (almost) the function I was trying to remember about. Thanks a
> lot!  The following patch addresses the problem in a way that I hope
> will satisfy MichaelS' concerns.
> 
> 2003-12-09  J. Brobecker  <brobecker@gnat.com>
> 
>         * frame.c (select_frame): Get the current frame PC using
>         get_frame_address_in_block() instead of get_frame_pc().
>         * stack.c (get_frame_language): Likewise.
> 
> Tested on x86-linux, with no regression.
> 
> OK to apply?

Yes, but just add a comment at each point reminding us why the PC is 
wrong before committing.

Andrew



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

* Re: [RFC] Unexpected automatic language switch - get_frame_language()
  2003-12-10 16:14         ` Andrew Cagney
@ 2003-12-10 17:42           ` Joel Brobecker
  0 siblings, 0 replies; 15+ messages in thread
From: Joel Brobecker @ 2003-12-10 17:42 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: Michael Snyder, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

> >2003-12-09  J. Brobecker  <brobecker@gnat.com>
> >
> >        * frame.c (select_frame): Get the current frame PC using
> >        get_frame_address_in_block() instead of get_frame_pc().
> >        * stack.c (get_frame_language): Likewise.
> >
> >Tested on x86-linux, with no regression.
> >
> >OK to apply?
> 
> Yes, but just add a comment at each point reminding us why the PC is 
> wrong before committing.

Thanks. Here is the patch that I ended up committing. Let me know if
somebody would like the comments to be worded differently.

-- 
Joel

[-- Attachment #2: addr_in_block.diff --]
[-- Type: text/plain, Size: 2079 bytes --]

Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.152
diff -u -p -r1.152 frame.c
--- frame.c	23 Nov 2003 21:49:12 -0000	1.152
+++ frame.c	10 Dec 2003 17:21:46 -0000
@@ -917,7 +917,13 @@ select_frame (struct frame_info *fi)
      source language of this frame, and switch to it if desired.  */
   if (fi)
     {
-      s = find_pc_symtab (get_frame_pc (fi));
+      /* We retrieve the frame's symtab by using the frame PC.  However
+         we cannot use the frame pc as is, because it usually points to
+         the instruction following the "call", which is sometimes the
+         first instruction of another function.  So we rely on
+         get_frame_address_in_block() which provides us with a PC which
+         is guaranteed to be inside the frame's code block.  */
+      s = find_pc_symtab (get_frame_address_in_block (fi));
       if (s
 	  && s->language != current_language->la_language
 	  && s->language != language_unknown
Index: stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.97
diff -u -p -r1.97 stack.c
--- stack.c	23 Nov 2003 20:41:17 -0000	1.97
+++ stack.c	10 Dec 2003 17:21:47 -0000
@@ -2036,7 +2036,14 @@ get_frame_language (void)
 
   if (deprecated_selected_frame)
     {
-      s = find_pc_symtab (get_frame_pc (deprecated_selected_frame));
+      /* We determine the current frame language by looking up its
+         associated symtab.  To retrieve this symtab, we use the frame PC.
+         However we cannot use the frame pc as is, because it usually points
+         to the instruction following the "call", which is sometimes the first
+         instruction of another function.  So we rely on
+         get_frame_address_in_block(), it provides us with a PC which is
+         guaranteed to be inside the frame's code block.  */
+      s = find_pc_symtab (get_frame_address_in_block (deprecated_selected_frame));
       if (s)
 	flang = s->language;
       else

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

* Re: [RFC] Unexpected automatic language switch - get_frame_language()
  2003-12-05 22:48 [RFC] Unexpected automatic language switch - get_frame_language() Joel Brobecker
  2003-12-05 23:57 ` Michael Snyder
@ 2003-12-10 17:47 ` Daniel Jacobowitz
  2003-12-10 18:10   ` Joel Brobecker
  1 sibling, 1 reply; 15+ messages in thread
From: Daniel Jacobowitz @ 2003-12-10 17:47 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

Hi Joel,

I've got no problems with your final patch, which is why I didn't
respond until now, but I'd like to go off on a little tangent...

On Fri, Dec 05, 2003 at 02:48:07PM -0800, Joel Brobecker wrote:
> Some additional information about "break exception". We have slightly
> modified the handling of the "break" command when in ada mode to special
> case "break exception". This places a breakpoint on a known GNAT runtime
> routine that's called upon exception raise. That's more or less how
> exception breakpoints are implemented for Ada.

This is really unfortunate.  I suppose you've released products that do
this?  Is there any way you could switch to something like "catch
throw" / "catch catch" which GDB implements for C++ for the same
functionality - though not very well yet.  Adding catch raise as an
alias for catch throw if that's more Ada-appropriate would be easy.

> For the user's convenience, when the breakpoint is hit, we automatically
> go up the call stack until we find a "user frame" (meaning a frame which
> has debug info and is not inside the GNAT runtime), and select that
> frame. So the user usually sees the location where the exception was
> raised, instead of the runtime machinery that triggers and handles the
> exception raise.

Does the real frame show up in backtraces?

What you're describing is what I tried to do for C++, but I couldn't
get it to work right.  I'd love to unify this code.


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

* Re: [RFC] Unexpected automatic language switch - get_frame_language()
  2003-12-10 17:47 ` Daniel Jacobowitz
@ 2003-12-10 18:10   ` Joel Brobecker
  2003-12-10 18:20     ` Daniel Jacobowitz
  2003-12-10 18:33     ` Ada's throw/catch; Was: " Andrew Cagney
  0 siblings, 2 replies; 15+ messages in thread
From: Joel Brobecker @ 2003-12-10 18:10 UTC (permalink / raw)
  To: gdb-patches

Hello Daniel,

> > Some additional information about "break exception". We have slightly
> > modified the handling of the "break" command when in ada mode to special
> > case "break exception". This places a breakpoint on a known GNAT runtime
> > routine that's called upon exception raise. That's more or less how
> > exception breakpoints are implemented for Ada.
> 
> This is really unfortunate.  I suppose you've released products that do
> this?

Yes. This actually was implemented even before I joing ACT which was
three years ago (time flies).

> Is there any way you could switch to something like "catch
> throw" / "catch catch" which GDB implements for C++ for the same
> functionality - though not very well yet.  Adding catch raise as an
> alias for catch throw if that's more Ada-appropriate would be easy.

It's funny you should ask this, because we actually internally raised
this question yesterday, while we were discussing something else.
Some of us felt that it was more appropriate to switch to "catch raise",
which others found "break exception" more natural. So we ended up
deciding to keep things as they are for now.

Your suggestion adds weight for the "catch" approach, so I will restart
the discussions internally. Independently of the user's perspective,
I think your suggestion will make the implementation much cleaner
(have a look at our GDB 5.3 sources at libre.act-europe.fr how we
rewrite the breakpoint location for breakpoint exceptions if you
are curious).

Before I start the discussion and maybe cause us to change our
implementation, what is the feeling of the GDB maintainers in
general. Suppose for instance that we would submit our current
implementation for inclusion, would it be approved, or refused?
Right now, here is our change against breakpoint.c relevant to this
functionality:

  - At the begining of break_command_1, we do:

+
+  /* Ada introduces some language-specific breakpoint syntax.  Translate
+     to equivalent vanilla breakpoints. */
+  addr_start = arg = ada_breakpoint_rewrite (arg, &break_on_exception);
+

  - There is also the special handling of this breakpoint. When such
    a breakpoint is hit, we print the exception name in the breakpoint
    message. Something like this:

    Breakpoint 1, CONSTRAINT_ERROR at file:line.

    So we added the following call in print_one_breakpoint() to
    that effect:

+  ada_print_exception_breakpoint_task (b);
+

> > For the user's convenience, when the breakpoint is hit, we automatically
> > go up the call stack until we find a "user frame" (meaning a frame which
> > has debug info and is not inside the GNAT runtime), and select that
> > frame. So the user usually sees the location where the exception was
> > raised, instead of the runtime machinery that triggers and handles the
> > exception raise.
> 
> Does the real frame show up in backtraces?

Yes, here is what happens when hitting an exception breakpoint:

Breakpoint 1, CONSTRAINT_ERROR at 0x08049457 in a () at a.adb:3
3          raise Constraint_Error;
(gdb) bt
#0  <__gnat_raise_nodefer_with_msg> (e=0x80548f8) at a-except.adb:863
#1  0x0804bdce in ada.exceptions.raise_with_location_and_msg (e=0x80548f8, 
    f=0x804fee7, l=3, m=0x80510cb) at a-except.adb:977
#2  0x0804bc7d in <__gnat_raise_constraint_error_msg> (file=0x804fee7, line=3, 
    msg=0x80510cb) at a-except.adb:853
#3  0x0804bee9 in <__gnat_rcheck_04> (file=0x804fee7, line=3)
    at a-except.adb:1041
#4  0x08049457 in a () at a.adb:3
#5  0x0804942d in main (argc=1, argv=(system.address) 0xbffffa14, 
    envp=(system.address) 0xbffffa1c) at b~a.adb:109

> What you're describing is what I tried to do for C++, but I couldn't
> get it to work right.  I'd love to unify this code.

Me too. As a matter of fact, I think there is a lot that Ada & C++ have
in common, but I know very little about C++, and I'd love to see the
code being shared between the two. There is the symbol stuff (like name
mangling, etc) that's currently under way, the handling of overloading, etc.

-- 
Joel


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

* Re: [RFC] Unexpected automatic language switch - get_frame_language()
  2003-12-10 18:10   ` Joel Brobecker
@ 2003-12-10 18:20     ` Daniel Jacobowitz
  2003-12-10 19:16       ` Joel Brobecker
  2003-12-10 18:33     ` Ada's throw/catch; Was: " Andrew Cagney
  1 sibling, 1 reply; 15+ messages in thread
From: Daniel Jacobowitz @ 2003-12-10 18:20 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On Wed, Dec 10, 2003 at 01:10:30PM -0500, Joel Brobecker wrote:
> Before I start the discussion and maybe cause us to change our
> implementation, what is the feeling of the GDB maintainers in
> general. Suppose for instance that we would submit our current
> implementation for inclusion, would it be approved, or refused?
> Right now, here is our change against breakpoint.c relevant to this
> functionality:
> 
>   - At the begining of break_command_1, we do:
> 
> +
> +  /* Ada introduces some language-specific breakpoint syntax.  Translate
> +     to equivalent vanilla breakpoints. */
> +  addr_start = arg = ada_breakpoint_rewrite (arg, &break_on_exception);
> +
> 
>   - There is also the special handling of this breakpoint. When such
>     a breakpoint is hit, we print the exception name in the breakpoint
>     message. Something like this:
> 
>     Breakpoint 1, CONSTRAINT_ERROR at file:line.
> 
>     So we added the following call in print_one_breakpoint() to
>     that effect:
> 
> +  ada_print_exception_breakpoint_task (b);
> +

You may want to look in the archives for the breakpoint_ops and catch
exception patches I posted earlier this year; they're broader but do
essentially the same thing.

Personally I don't like this "language dependent breakpoint syntax"
idea.  Here's one reason why: judging from your implementation I bet
that a breakpoint on the starting instruction of the exception raising
function will cause this magic to happen.  With "catch exception" that
information is stored at a higher level, so you can break *0x08010102
and _not_ have GDB try to walk up a couple of frames.

Seems friendlier to me; I like to keep a line between GDB's
user-friendliness features and its abilities as a system debugger, for
when I'm trying to figure out why a language feature isn't working.

> > > For the user's convenience, when the breakpoint is hit, we automatically
> > > go up the call stack until we find a "user frame" (meaning a frame which
> > > has debug info and is not inside the GNAT runtime), and select that
> > > frame. So the user usually sees the location where the exception was
> > > raised, instead of the runtime machinery that triggers and handles the
> > > exception raise.
> > 
> > Does the real frame show up in backtraces?
> 
> Yes, here is what happens when hitting an exception breakpoint:
> 
> Breakpoint 1, CONSTRAINT_ERROR at 0x08049457 in a () at a.adb:3
> 3          raise Constraint_Error;
> (gdb) bt
> #0  <__gnat_raise_nodefer_with_msg> (e=0x80548f8) at a-except.adb:863
> #1  0x0804bdce in ada.exceptions.raise_with_location_and_msg (e=0x80548f8, 
>     f=0x804fee7, l=3, m=0x80510cb) at a-except.adb:977
> #2  0x0804bc7d in <__gnat_raise_constraint_error_msg> (file=0x804fee7, line=3, 
>     msg=0x80510cb) at a-except.adb:853
> #3  0x0804bee9 in <__gnat_rcheck_04> (file=0x804fee7, line=3)
>     at a-except.adb:1041
> #4  0x08049457 in a () at a.adb:3
> #5  0x0804942d in main (argc=1, argv=(system.address) 0xbffffa14, 
>     envp=(system.address) 0xbffffa1c) at b~a.adb:109

Hmm, that's quite nice.

> > What you're describing is what I tried to do for C++, but I couldn't
> > get it to work right.  I'd love to unify this code.
> 
> Me too. As a matter of fact, I think there is a lot that Ada & C++ have
> in common, but I know very little about C++, and I'd love to see the
> code being shared between the two. There is the symbol stuff (like name
> mangling, etc) that's currently under way, the handling of overloading, etc.

Time will tell :)

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

* Ada's throw/catch; Was: [RFC] Unexpected automatic language switch - get_frame_language()
  2003-12-10 18:10   ` Joel Brobecker
  2003-12-10 18:20     ` Daniel Jacobowitz
@ 2003-12-10 18:33     ` Andrew Cagney
  2003-12-10 19:04       ` Joel Brobecker
  1 sibling, 1 reply; 15+ messages in thread
From: Andrew Cagney @ 2003-12-10 18:33 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

> Before I start the discussion and maybe cause us to change our
> implementation, what is the feeling of the GDB maintainers in
> general. Suppose for instance that we would submit our current
> implementation for inclusion, would it be approved, or refused?
> Right now, here is our change against breakpoint.c relevant to this
> functionality:

Given that GDB already has an established generic documented and 
"working" throw/catch mechanism, I'm not sure that there are advantages 
to be had in adding a second.  While Act would certainly benefit, I 
don't know that the same can be argued for GDB as a whole.

Andrew



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

* Re: Ada's throw/catch; Was: [RFC] Unexpected automatic language switch - get_frame_language()
  2003-12-10 18:33     ` Ada's throw/catch; Was: " Andrew Cagney
@ 2003-12-10 19:04       ` Joel Brobecker
  0 siblings, 0 replies; 15+ messages in thread
From: Joel Brobecker @ 2003-12-10 19:04 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

> Given that GDB already has an established generic documented and 
> "working" throw/catch mechanism, I'm not sure that there are advantages 
> to be had in adding a second.  While Act would certainly benefit, I 
> don't know that the same can be argued for GDB as a whole.

I understand completely, but I thought I'd make sure :).
This is certainly a strong argument for us to change our current
approach, since we would prefer our version of GDB and the FSF
one to be as close as possible.

-- 
Joel


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

* Re: [RFC] Unexpected automatic language switch - get_frame_language()
  2003-12-10 18:20     ` Daniel Jacobowitz
@ 2003-12-10 19:16       ` Joel Brobecker
  2003-12-10 19:18         ` Daniel Jacobowitz
  0 siblings, 1 reply; 15+ messages in thread
From: Joel Brobecker @ 2003-12-10 19:16 UTC (permalink / raw)
  To: gdb-patches

[quick reply before I run to catch my flight] 
> You may want to look in the archives for the breakpoint_ops and catch
> exception patches I posted earlier this year; they're broader but do
> essentially the same thing.

Wildo.

> Personally I don't like this "language dependent breakpoint syntax"
> idea.  Here's one reason why: judging from your implementation I bet
> that a breakpoint on the starting instruction of the exception raising
> function will cause this magic to happen.

I don't think so, because we actually also added a field in the
breakpoint structure that says whether the breakpoint is an exception
breakpoint or not. So what happens is that we only try to walk up the
stack only if that flag is set. 

I should have mentionned that in my previous message, my bad.

Incidentally, this makes our change a bit more intrusive than it
actually looks from my previous message.  Given yours and Andrew's
feedback, I am sold on the "catch" approach, but I need to discuss this
within ACT. We do have a backward compatibilty problem, since this
command has been used for a long time by our users, now... In any case,
this is orthogonal to how this will be supported by the FSF version
of GDB, so it's not something for you to worry about, our problem.

> With "catch exception" that information is stored at a higher level,
> so you can break *0x08010102 and _not_ have GDB try to walk up a
> couple of frames.

Same for Ada exception breakpoints.

> > Yes, here is what happens when hitting an exception breakpoint:
> > 
> > Breakpoint 1, CONSTRAINT_ERROR at 0x08049457 in a () at a.adb:3
> > 3          raise Constraint_Error;
> > (gdb) bt
> > #0  <__gnat_raise_nodefer_with_msg> (e=0x80548f8) at a-except.adb:863
> > #1  0x0804bdce in ada.exceptions.raise_with_location_and_msg (e=0x80548f8, 
> >     f=0x804fee7, l=3, m=0x80510cb) at a-except.adb:977
> > #2  0x0804bc7d in <__gnat_raise_constraint_error_msg> (file=0x804fee7, line=3, 
> >     msg=0x80510cb) at a-except.adb:853
> > #3  0x0804bee9 in <__gnat_rcheck_04> (file=0x804fee7, line=3)
> >     at a-except.adb:1041
> > #4  0x08049457 in a () at a.adb:3
> > #5  0x0804942d in main (argc=1, argv=(system.address) 0xbffffa14, 
> >     envp=(system.address) 0xbffffa1c) at b~a.adb:109
> 
> Hmm, that's quite nice.

Thanks :)

-- 
Joel


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

* Re: [RFC] Unexpected automatic language switch - get_frame_language()
  2003-12-10 19:16       ` Joel Brobecker
@ 2003-12-10 19:18         ` Daniel Jacobowitz
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Jacobowitz @ 2003-12-10 19:18 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On Wed, Dec 10, 2003 at 02:16:14PM -0500, Joel Brobecker wrote:
> [quick reply before I run to catch my flight] 
> > You may want to look in the archives for the breakpoint_ops and catch
> > exception patches I posted earlier this year; they're broader but do
> > essentially the same thing.
> 
> Wildo.
> 
> > Personally I don't like this "language dependent breakpoint syntax"
> > idea.  Here's one reason why: judging from your implementation I bet
> > that a breakpoint on the starting instruction of the exception raising
> > function will cause this magic to happen.
> 
> I don't think so, because we actually also added a field in the
> breakpoint structure that says whether the breakpoint is an exception
> breakpoint or not. So what happens is that we only try to walk up the
> stack only if that flag is set. 
> 
> I should have mentionned that in my previous message, my bad.
> 
> Incidentally, this makes our change a bit more intrusive than it
> actually looks from my previous message.  Given yours and Andrew's
> feedback, I am sold on the "catch" approach, but I need to discuss this
> within ACT. We do have a backward compatibilty problem, since this
> command has been used for a long time by our users, now... In any case,
> this is orthogonal to how this will be supported by the FSF version
> of GDB, so it's not something for you to worry about, our problem.

It also makes the issue very easy for you to handle at ACT.  Just
add the first hook in the same place and translate "break exception" to
"catch raise" and you should be fine.  Switching the internal
implementation to make catch raise work should not be all that
difficult.

> > With "catch exception" that information is stored at a higher level,
> > so you can break *0x08010102 and _not_ have GDB try to walk up a
> > couple of frames.
> 
> Same for Ada exception breakpoints.

OK, that makes me feel much better about it :)

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


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

end of thread, other threads:[~2003-12-10 19:18 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-12-05 22:48 [RFC] Unexpected automatic language switch - get_frame_language() Joel Brobecker
2003-12-05 23:57 ` Michael Snyder
2003-12-06  0:18   ` Joel Brobecker
2003-12-06  0:28     ` Andrew Cagney
2003-12-10  1:50       ` Joel Brobecker
2003-12-10 16:14         ` Andrew Cagney
2003-12-10 17:42           ` Joel Brobecker
2003-12-06  0:42     ` Michael Snyder
2003-12-10 17:47 ` Daniel Jacobowitz
2003-12-10 18:10   ` Joel Brobecker
2003-12-10 18:20     ` Daniel Jacobowitz
2003-12-10 19:16       ` Joel Brobecker
2003-12-10 19:18         ` Daniel Jacobowitz
2003-12-10 18:33     ` Ada's throw/catch; Was: " Andrew Cagney
2003-12-10 19:04       ` Joel Brobecker

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