[RFC] Unexpected automatic language switch - get_frame_language()

[Joel Brobecker <brobecker at gnat dot com>]

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: a.adb
Description: Text document