From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15772 invoked by alias); 2 Apr 2004 18:36:40 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 15724 invoked from network); 2 Apr 2004 18:36:38 -0000 Received: from unknown (HELO takamaka.act-europe.fr) (142.179.108.108) by sources.redhat.com with SMTP; 2 Apr 2004 18:36:38 -0000 Received: by takamaka.act-europe.fr (Postfix, from userid 507) id 19ACD47D62; Fri, 2 Apr 2004 10:36:38 -0800 (PST) Date: Fri, 02 Apr 2004 18:36:00 -0000 From: Joel Brobecker To: gdb-patches@sources.redhat.com Subject: [RFA] Fix small problems in rs6000-tdep.c:skip_prologue() Message-ID: <20040402183637.GC871@gnat.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="5/uDoXvLw7AC5HRs" Content-Disposition: inline User-Agent: Mutt/1.4i X-SW-Source: 2004-04/txt/msg00062.txt.bz2 --5/uDoXvLw7AC5HRs Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 5852 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: : stw r31,-4(r1) : stwu r1,-48(r1) : mr r31,r1 : stw r11,24(r31) Line 15: : li r0,12 : stw r0,28(r31) Line 17: : lwz r1,0(r1) : lwz r31,-4(r1) : 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 * 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; >> --5/uDoXvLw7AC5HRs Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="rs6000-tdep.c.diff" Content-length: 4328 Index: rs6000-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v retrieving revision 1.191 diff -u -p -r1.191 rs6000-tdep.c --- rs6000-tdep.c 1 Apr 2004 21:00:59 -0000 1.191 +++ rs6000-tdep.c 1 Apr 2004 23:03:50 -0000 @@ -420,6 +420,69 @@ refine_prologue_limit (CORE_ADDR pc, COR } +/* Return nonzero if the given instruction OP can be part of the prologue + of a function that saves a parameter on the stack. FRAMEP should be + set if one of the previous instructions in the function has set the + Frame Pointer. */ + +static int +store_parameter_on_stack_prologue_insn_p (unsigned long op, int framep) +{ + /* Move parameters from argument registers to temporary register. */ + if ((op & 0xfc0007fe) == 0x7c000378) /* mr(.) Rx,Ry */ + { + /* Rx must be scratch register r0. */ + const int rx_regno = (op >> 16) & 31; + /* Ry: Only r3 - r10 are used for parameter passing. */ + const int ry_regno = GET_SRC_REG (op); + + return (rx_regno == 0 && ry_regno >= 3 && ry_regno <= 10); + } + + /* Save a General Purpose Register on stack. */ + + if ((op & 0xfc1f0003) == 0xf8010000 || /* std Rx,NUM(r1) */ + (op & 0xfc1f0000) == 0xd8010000) /* stfd Rx,NUM(r1) */ + { + /* Rx: Only r3 - r10 are used for parameter passing. */ + const int rx_regno = GET_SRC_REG (op); + + return (rx_regno >= 3 && rx_regno <= 10); + } + + /* Save a General Purpose Register on stack via the Frame Pointer. */ + + if (framep && + ((op & 0xfc1f0000) == 0x901f0000 || /* st rx,NUM(r31) */ + (op & 0xfc1f0000) == 0x981f0000 || /* stb Rx,NUM(r31) */ + (op & 0xfc1f0000) == 0xd81f0000)) /* stfd Rx,NUM(r31) */ + { + /* Rx: Only r3 - r10 are used for parameter passing. */ + const int rx_regno = GET_SRC_REG (op); + + return (rx_regno >= 3 && rx_regno <= 10); + } + + if ((op & 0xfc1f0000) == 0xfc010000) /* frsp, fp?,NUM(r1) */ + { + /* Only f2 - f8 are used for parameter passing. */ + const int src_regno = GET_SRC_REG (op); + + return (src_regno >= 2 && src_regno <= 8); + } + + if (framep && ((op & 0xfc1f0000) == 0xfc1f0000)) /* frsp, fp?,NUM(r31) */ + { + /* Only f2 - f8 are used for parameter passing. */ + const int src_regno = GET_SRC_REG (op); + + return (src_regno >= 2 && src_regno <= 8); + } + + /* Not an insn that saves a parameter on stack. */ + return 0; +} + static CORE_ADDR skip_prologue (CORE_ADDR pc, CORE_ADDR lim_pc, struct rs6000_framedata *fdata) { @@ -701,26 +764,7 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l /* store parameters in stack */ } /* Move parameters from argument registers to temporary register. */ - else if ((op & 0xfc0007fe) == 0x7c000378 && /* mr(.) Rx,Ry */ - (((op >> 21) & 31) >= 3) && /* R3 >= Ry >= R10 */ - (((op >> 21) & 31) <= 10) && - (((op >> 16) & 31) == 0)) /* Rx: scratch register r0 */ - { - continue; - } - else if ((op & 0xfc1f0003) == 0xf8010000 || /* std rx,NUM(r1) */ - (op & 0xfc1f0000) == 0xd8010000 || /* stfd Rx,NUM(r1) */ - (op & 0xfc1f0000) == 0xfc010000) /* frsp, fp?,NUM(r1) */ - { - continue; - - /* store parameters in stack via frame pointer */ - } - else if (framep && - ((op & 0xfc1f0000) == 0x901f0000 || /* st rx,NUM(r31) */ - (op & 0xfc1f0000) == 0x981f0000 || /* stb Rx,NUM(r31) */ - (op & 0xfc1f0000) == 0xd81f0000 || /* stfd Rx,NUM(r31) */ - (op & 0xfc1f0000) == 0xfc1f0000)) /* frsp, fp?,NUM(r31) */ + else if (store_parameter_on_stack_prologue_insn_p (op, framep)) { continue; @@ -791,6 +835,11 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l { li_found_pc = pc; vr_saved_offset = SIGNED_SHORT (op); + + /* This insn by itself is not part of the prologue, unless + if part of the pair of insns mentioned above. So do not + record this insn as part of the prologue yet. */ + prev_insn_was_prologue_insn = 0; } /* Store vector register S at (r31+r0) aligned to 16 bytes. */ /* 011111 sssss 11111 00000 00111001110 */ --5/uDoXvLw7AC5HRs--