[RFA] Fix small problems in rs6000-tdep.c:skip_prologue()

[Joel Brobecker <brobecker at gnat dot com>]

Hello,

We have noticed that GDB skips too much prologue when inserting
a breakpoint on a given function. I will provide the source code
below, but it's not very important to understand the cause of the
problem. Here is an extract:

     12       procedure print is
     13          A : Integer;
     14       begin
     15          A := I;
     16          null;
     17       end print;

Right now, when we insert a breakpoint on function ``print''
(which encoded name is
``generics__test_generics__other_instance__print.0''), we end
up placing the breakpoint at line 17, but the expected location
was at line 15:

    (gdb) b "generics__test_generics__other_instance__print.0"
    Breakpoint 1 at 0x100112e0: file generics.adb, line 17.

A quick look at the assembly code for function ``print'' shows
what causes skip_prologue() to go too far:

    
    Line 12:
        <print.0+0>:        stw     r31,-4(r1)
        <print.0+4>:        stwu    r1,-48(r1)
        <print.0+8>:        mr      r31,r1
        <print.0+12>:       stw     r11,24(r31)
    Line 15:
        <print.0+16>:       li      r0,12
        <print.0+20>:       stw     r0,28(r31)
    Line 17:
        <print.0+24>:       lwz     r1,0(r1)
        <print.0+28>:       lwz     r31,-4(r1)
        <print.0+32>:       blr

The first problem is with the insn at +16 (li [...]). If part of the
prologue, this instruction should be part of a pair of instructions
saving vector registers. This is not the case here. However,
skip_prologue() doesn't check that, and instead considers by default
that this insn is part of the prologue (ie "prev_insn_was_prologue_insn"
remains set to 1). So the first change I made was to *not* consider this
instruction as part of the prologue unless the next one is part of the
prologue. Normally, you would rather check that the next instruction is
even one of the instructions that come as a pair with that "li"
instruction. But this is not necessary in our case, because we're not
trying to tag each instruction individually, we're trying to find the
upper bound of the prologue. So long as some later instruction is
recognized as part of the prologue (whether it be part of that
instruction pair or not), we need to update our upper bound accordingly.
On the contrary, if none of the following instructions we scan belong
to the prologue, then we should not include that instruction in the
function prologue. Hence my first change (I hope I was clear enough?).

The second problem is with the following instruction: "stw r0,28(r31)".
The prologue analyzer tags this instruction as being part of the
prologue, but this is incorrect, I believe. According to the PPC
ABI document I have, only r3 to r10 are used for parameter passing:

        For PowerPC, up to eight words are passed in general purpose
        registers, loaded sequentially into general purpose registers r3
        through r10.

Unfortunately, skip_prologue() detects the "stw Rx, NUM(R31)" sequence,
but forgot to check the register number. Here, it's the scratch register
zero, so the instruction should not be considered part of the prologue
either.

The same applies to instructions involving saving floating point
arguments passed via registers: Only f2 to f8 are used for that
purpose.

So my second change was to create a new function extracted from
skip_prologue() named store_parameter_on_stack_prologue_insn_p(),
and then improve it to also check for register numbers.

2004-04-02  Joel Brobecker  <brobecker@gnat.com>

        * rs6000-tdep.c (store_parameter_on_stack_prologue_insn_p):
        New function, an improved version of some code extracted from
        skip_prologue().
        (skip_prologue): Use store_parameter_on_stack_prologue_insn_p()
        to detect instructions saving a parameter on the stack.
        Do not mark "li rx, SIMM" instructions as part of the prologue,
        unless the following instruction is also part of the prologue.

Tested on ppc-aix-5.1. This fixes several FAILs and XFAILs as well as
our problem:

In gdb.base:
    condbreak.exp: run until breakpoint at marker1              XFAIL PASS
    condbreak.exp: run until breakpoint at marker2              XFAIL PASS
    ena-dis-br.exp: continue to auto-disabled break marker2     XFAIL PASS
    ena-dis-br.exp: continue to break marker1                   XFAIL PASS
    ena-dis-br.exp: continue to break marker1, 2nd time         XFAIL PASS
    funcargs.exp: continue to call5b                            FAIL  PASS
    funcargs.exp: print *stp                                    FAIL  PASS
    funcargs.exp: print st                                      FAIL  PASS
    funcargs.exp: print un                                      FAIL  PASS
    funcargs.exp: run to call5a                                 FAIL  PASS

I also get some improved results in gdb.threads, and it seems related to
breakpointing, but I am always cautious with varying results in that
part of the testsuite, and I haven't had time to study them.

OK to apply?

Thanks,
-- 
Joel

Here is the entire Ada code we used to reproduce the problem. To build
them, place the sources in a file, say sources.ada, and then do:

        % gnatchop sources.ada
        % gnatmake -g generics_main

<<
package body Generics is

   procedure Test_Generics is
      package other_instance is new the_generic(12);
   begin
      other_instance.print;
      null;
   end Test_Generics;

   package body the_generic is

      procedure print is
         A : Integer;
      begin
         A := I;
         null;
      end print;
   end the_generic;

end Generics;
package Generics is

   generic
      i : integer;
   package the_generic is
      procedure print;
   end the_generic;

   procedure Test_Generics;

end Generics;
with generics;
procedure generics_main is
begin
    generics.test_generics;
end generics_main;
>>

Attachment: rs6000-tdep.c.diff
Description: Text document