Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] OSF/1 - "next" over prologueless function call
@ 2003-12-02  4:26 Joel Brobecker
  2003-12-02  4:30 ` Daniel Jacobowitz
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Joel Brobecker @ 2003-12-02  4:26 UTC (permalink / raw)
  To: gdb-patches

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

Hello,

We have recently noticed this failure. To demonstrate it, you'll need to
use the GNAT compiler because the sources that caused the problem are in
the GNAT runtime.

Use the following little Ada program:

        with Ada.Text_IO; use Ada.Text_IO;
        
        procedure Foo is
        begin
           Put_Line ("Hello World.");
           Put_Line ("Me again.");
        end Foo;

Compile it using the following command:

        % gnatmake -g foo

The following GDB transcript demonstrates the problem:

        (gdb) b foo.adb:5
        Breakpoint 1 at 0x120015a18: file foo.adb, line 5.
        (gdb) run
        Starting program: /[...]/foo 
        
        Breakpoint 1, _ada_foo () at foo.adb:5
        5          Put_Line ("Hello World.");
        (gdb) n
        0x0000000120023fe8 in ada__text_io__put_line__2 () at a-textio.adb:6
        6       a-textio.adb: No such file or directory.
                in a-textio.adb

The expected behavior was for GDB to stop at line 6 of foo.adb, not
inside Put_Line:

        (gdb) n
        Hello World.
        6          Put_Line ("Me again.");

Here is what happens. First, after having hit the breakpoint at line 5,
GDB sees the following code for line 5:

        #.stabn        68,0,5,$LM2
        lda $1,$LC0
        lda $2,$LC1
        bis $31,$1,$16
        bis $31,$2,$17
        jsr $26,ada__text_io__put_line__2
        ldgp $29,0($26)

(for the curious, LC0 and LC1 are the string "Hello World." and its
bounds).

That's the assembly code as generated by GCC. However, if you look at
the actual assembly code, as produced after the link, you will see
that the OSF/1 linker has done a little optimization (which I find is
done quite often). Here is how the jsr instruction has been modified:

        bsr     ra,0x120023fe8 <ada__text_io__put_line__2+8>

The jump is actually made to the third instruction of Put_Line.
So when GDB starts doing the "next" operation, it eventually receives
a sigtrap. Normally, GDB should detect that we are inside a function
call and therefore should step_over_function(). But it fails to recognize
this situation because of the conjunction of the following two factors:

        1. The stop_pc is not at the "official" Put_Line function
           start address

        2. The function does not have a prologue

So the following test fails in infrun.c:handle_inferior_event() fails,
and GDB incorrectly thinks that we have landed at the next line of code:

  if (((stop_pc == ecs->stop_func_start /* Quick test */
        || in_prologue (stop_pc, ecs->stop_func_start))
       && !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, ecs->stop_func_name))
      || IN_SOLIB_CALL_TRAMPOLINE (stop_pc, ecs->stop_func_name)
      || ecs->stop_func_name == 0)

Just FYI, the first instructions of Put_Line are:

    <ada__text_io__put_line__2+0>:       ldah    gp,8191(t12)
    <ada__text_io__put_line__2+4>:       lda     gp,-1088(gp)
    <ada__text_io__put_line__2+8>:       ldq     t8,-28008(gp)
    <ada__text_io__put_line__2+12>:      nop
    <ada__text_io__put_line__2+16>:      mov     a0,t0
    <ada__text_io__put_line__2+20>:      mov     a1,a2
    <ada__text_io__put_line__2+24>:      ldq     a0,0(t8)
    <ada__text_io__put_line__2+28>:      mov     t0,a1
    <ada__text_io__put_line__2+32>:      ldq     t12,-30040(gp)
    <ada__text_io__put_line__2+36>:      jmp     zero,(t12),0x120023d50 <ada__text_io__put_line>

If it wasn't for the linker optimization, the check for "top_pc ==
ecs->stop_func_start" would probably have kicked in, and all would
have been fine.

So I suggest we refine this test to use a new gdbarch function which,
by default, would be the exact equivalent of this equality check. But
I then provide an OSF/1-specific version of this function that checks
that we stopped either at the first instruction of the function, or
right after the couple of instructions forming the ldgp macro.

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

        * gdbarch.sh (at_function_start): New gdbarch function.
        * gdbarch.h: Regenerate.
        * gdbarch.c: Regenerate.
        * arch-utils.c (default_at_function_start): New function.
        * arch-utils.h (default_at_function_start): Add prototype.
        * infrun.c (handle_inferior_event): Use new gdbarch function
        at_function_start to properly detect function calls during
        STEP_OVER_ALL.
        * alpha-osf1-tdep.c (alpha_osf1_at_function_start): New function.
        (alpha_osf1_init_abi): Set the gdbarch at_function_call function.

Tested on alpha-osf1 and x86-linux. No regression.

OK to apply?

Thanks,
-- 
Joel

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

Index: gdbarch.sh
===================================================================
RCS file: /cvs/src/src/gdb/gdbarch.sh,v
retrieving revision 1.288
diff -u -p -r1.288 gdbarch.sh
--- gdbarch.sh	14 Nov 2003 21:22:42 -0000	1.288
+++ gdbarch.sh	2 Dec 2003 03:52:38 -0000
@@ -771,6 +771,24 @@ F::FETCH_POINTER_ARGUMENT:CORE_ADDR:fetc
 # Return the appropriate register set for a core file section with
 # name SECT_NAME and size SECT_SIZE.
 M:::const struct regset *:regset_from_core_section:const char *sect_name, size_t sect_size:sect_name, sect_size
+
+# Return nonzero if PC points to the first "effective" instruction of
+# the function starting at FUNC_START.
+#
+# The default implementation which should be appropriate for most
+# targets is to return nonzero if PC is equal to FUNC_START.  However,
+# it can happen that the address of a subroutine call instruction
+# is changed to jump slightly after the begining of the function,
+# thus making the "effective" function start address a bit different
+# from the start address read from the symbol table or the debugging
+# information.
+#
+# For instance, on alpha-tru64, the first two instructions of a function
+# are often setting up the GP. The linker sometimes determines that
+# these instructions are not needed for the proper execution of the
+# program and therefore virtually optimizes them out by changing the
+# jump address to branch 2 instructions later.
+f:2:AT_FUNCTION_START:int:at_function_start:CORE_ADDR pc, CORE_ADDR func_start:pc, func_start::0:default_at_function_start::0
 EOF
 }
 
Index: arch-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/arch-utils.c,v
retrieving revision 1.108
diff -u -p -r1.108 arch-utils.c
--- arch-utils.c	14 Nov 2003 21:22:42 -0000	1.108
+++ arch-utils.c	2 Dec 2003 03:52:39 -0000
@@ -370,6 +370,12 @@ default_stabs_argument_has_addr (struct 
   return 0;
 }
 
+int
+default_at_function_start (CORE_ADDR pc, CORE_ADDR func_start)
+{
+  return pc == func_start;
+}
+
 \f
 /* Functions to manipulate the endianness of the target.  */
 
Index: arch-utils.h
===================================================================
RCS file: /cvs/src/src/gdb/arch-utils.h,v
retrieving revision 1.65
diff -u -p -r1.65 arch-utils.h
--- arch-utils.h	23 Nov 2003 21:32:42 -0000	1.65
+++ arch-utils.h	2 Dec 2003 03:52:39 -0000
@@ -142,6 +142,8 @@ extern void legacy_value_to_register (st
 extern int default_stabs_argument_has_addr (struct gdbarch *gdbarch,
 					    struct type *type);
 
+extern int default_at_function_start (CORE_ADDR pc, CORE_ADDR func_start);
+
 /* For compatibility with older architectures, returns
    (LEGACY_SIM_REGNO_IGNORE) when the register doesn't have a valid
    name.  */
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.122
diff -u -p -r1.122 infrun.c
--- infrun.c	25 Nov 2003 16:01:36 -0000	1.122
+++ infrun.c	2 Dec 2003 03:52:41 -0000
@@ -2472,7 +2472,7 @@ process_event_stop_test:
       return;
     }
 
-  if (((stop_pc == ecs->stop_func_start	/* Quick test */
+  if (((AT_FUNCTION_START (stop_pc, ecs->stop_func_start)
 	|| in_prologue (stop_pc, ecs->stop_func_start))
        && !IN_SOLIB_RETURN_TRAMPOLINE (stop_pc, ecs->stop_func_name))
       || IN_SOLIB_CALL_TRAMPOLINE (stop_pc, ecs->stop_func_name)
Index: alpha-osf1-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/alpha-osf1-tdep.c,v
retrieving revision 1.16
diff -u -p -r1.16 alpha-osf1-tdep.c
--- alpha-osf1-tdep.c	27 Aug 2003 19:02:03 -0000	1.16
+++ alpha-osf1-tdep.c	2 Dec 2003 03:52:41 -0000
@@ -45,6 +45,47 @@ alpha_osf1_sigcontext_addr (struct frame
     return (read_memory_integer (get_frame_base (frame), 8));
 }
 
+static int
+alpha_osf1_at_function_start (CORE_ADDR pc, CORE_ADDR func_start)
+{
+  /* A function prologue typically starts with 2 instructions setting
+     up the GP.  But the linker sometimes changes the jump address of
+     subroutine calls because it has determined that these 2 instructions
+     do not make any difference in the execution of the program.  So it
+     has virtually optimized them out by changing the jump address
+     inside the function call to branch just after these 2 instructions.
+
+     As a consequence, the "effective" start address of a given function
+     can be either the first instruction of this function, or the third
+     instruction if the first two instructions are setting up the GP.
+     
+     It is important to recognize these two cases during a "next" in
+     order to properly detect function calls that land into a prologueless
+     function which first two instructions have been shunted by the linker.  */
+
+  if (pc == func_start)
+    return 1;
+
+  if (pc == func_start + 8)
+    {
+      unsigned int inst;
+
+      /* ldah $gp,n($t12) */
+      inst = alpha_read_insn (func_start);
+      if ((inst & 0xffff0000) != 0x27bb0000)
+        return 0;
+
+      /* lda $gp,n($gp) */
+      inst = alpha_read_insn (func_start + 4);
+      if ((inst & 0xffff0000) != 0x23bd0000)
+        return 0;
+
+      return 1;
+    }
+
+  return 0;
+}
+
 static void
 alpha_osf1_init_abi (struct gdbarch_info info,
                      struct gdbarch *gdbarch)
@@ -59,6 +100,7 @@ alpha_osf1_init_abi (struct gdbarch_info
      on multi-processor machines. We need to use software single stepping
      instead.  */
   set_gdbarch_software_single_step (gdbarch, alpha_software_single_step);
+  set_gdbarch_at_function_start (gdbarch, alpha_osf1_at_function_start);
 
   tdep->sigcontext_addr = alpha_osf1_sigcontext_addr;
 

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [RFA] OSF/1 - "next" over prologueless function call
@ 2003-12-02  5:49 Michael Elizabeth Chastain
  0 siblings, 0 replies; 18+ messages in thread
From: Michael Elizabeth Chastain @ 2003-12-02  5:49 UTC (permalink / raw)
  To: brobecker, drow; +Cc: gdb-patches

drow> But about the OSF/1 specific target hook: no, this should be an
drow> Alpha-specific target hook instead.

I agree with Daniel here.

I have little expertise on 64-bit architectures, but I expect
this would be a common optimization for architectures where
there is a $gp or similar register which needs to be loaded
at the beginning of each function, and a linker can figure out
that $gp has the same value across many different functions.

Michael C


^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [RFA] OSF/1 - "next" over prologueless function call
@ 2003-12-02  7:53 Michael Elizabeth Chastain
  0 siblings, 0 replies; 18+ messages in thread
From: Michael Elizabeth Chastain @ 2003-12-02  7:53 UTC (permalink / raw)
  To: brobecker, ezannoni, rth; +Cc: gdb-patches

Hi Joel,

> ... I see some target-specific code uses a grab-bag field of the
> minimal_symbol structure to store some target-specific information
> (field "info"). I don't particularly fancy typeless fields like this,
> but, just thinking aloud,  we could add a new fields holding some flags
> which would be defined in gdb in an manor independent of the target.

I would like to clean up the "info" field.
The only thing it's used for is these target-specific bit flags.
It's really stupid that it's a "void *"!

  http://sources.redhat.com/ml/gdb/2003-11/msg00094.html

When I get some more cycles, and Elena gets some more cycles,
I'll bring it up for discussion again.

Michael C
too tired to work, not too tired to blabber


^ permalink raw reply	[flat|nested] 18+ messages in thread
end of thread, other threads:[~2003-12-09 23:10 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-12-02  4:26 [RFA] OSF/1 - "next" over prologueless function call Joel Brobecker
2003-12-02  4:30 ` Daniel Jacobowitz
2003-12-02  6:06   ` Joel Brobecker
2003-12-02  6:35 ` Richard Henderson
2003-12-02  7:21   ` Joel Brobecker
2003-12-02 15:14     ` Daniel Jacobowitz
2003-12-03  1:54       ` Joel Brobecker
2003-12-02 13:55   ` Daniel Jacobowitz
2003-12-03  4:19 ` Andrew Cagney
2003-12-04  0:55   ` Joel Brobecker
2003-12-04  1:49     ` Andrew Cagney
2003-12-04 23:24       ` Joel Brobecker
2003-12-04 23:28         ` Daniel Jacobowitz
2003-12-05 20:19         ` Andrew Cagney
2003-12-08 23:25           ` Joel Brobecker
2003-12-09 23:10             ` Andrew Cagney
2003-12-02  5:49 Michael Elizabeth Chastain
2003-12-02  7:53 Michael Elizabeth Chastain

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