From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eli Zaretskii To: shebs@apple.com Cc: ezannoni@cygnus.com, gdb-patches@sourceware.cygnus.com Subject: Re: [PATCH] GDB command-line switches and annotations docs Date: Sat, 01 Apr 2000 00:00:00 -0000 Message-id: <200003161210.HAA02820@indy.delorie.com> References: <14537.5048.99180.910863@kwikemart.cygnus.com> <200003101809.NAA22092@indy.delorie.com> <14537.18119.116000.442262@kwikemart.cygnus.com> <200003120754.CAA24393@indy.delorie.com> <14539.49127.700844.807495@kwikemart.cygnus.com> <200003151414.JAA01131@indy.delorie.com> <38CFF074.B275A986@apple.com> X-SW-Source: 2000-q1/msg00739.html > > > > If it would help, I could add a note telling that the event loop is > > > > still not fully asynchronous, with a FIXME comment that the note > > > > should be removed when the target side is done. Okay? > > > > > > Perfect, thanks! > > > > I attach below the changes which address the above, and also what Stan > > said about moving the Annotations chapter before the appendices. > > These changes are to be applied _on top_ of those I sent in the > > previous message. > > > > Is it okay to commit this and the previous patch? > > Go for it! Done. >From ac131313@cygnus.com Sat Apr 01 00:00:00 2000 From: Andrew Cagney To: Jim Blandy Cc: Jimmy Guo , gdb-patches@sourceware.cygnus.com Subject: Re: 2nd try: (patch) hpjyg03: (buildsym|language).[ch] Date: Sat, 01 Apr 2000 00:00:00 -0000 Message-id: <38B0A4A0.D9109A9E@cygnus.com> References: X-SW-Source: 2000-q1/msg00264.html Content-length: 1537 Jim Blandy wrote: > > > > Revision of patch hpjyg03: > > > Use buildsym.c:add_free_pendings() to manage addition to free_pendings; > > > Yanked out subfiles change to end_symtab() in the original patch > > > submission. > > > > Okay. I think David Taylor still needs to approve the language.[ch] > > changes, but I approve the buildsym.[ch] changes. > > I applied this. > > 2000-02-18 Jim Blandy > > From Jimmy Guo : > * buildsym.h (add_free_pendings): Declare. > * buildsym.c (add_free_pendings): New function. > (make_blockvector): 32x64 fix using longest_local_hex_string(). > (start_subfile): initialize variable 'subfile'. Unfortunatly, there is no prototype for longest_local_hex_string() visible in buildsym.c. I've ended up commiting the following. enjoy, Andrew Mon Feb 21 12:50:57 2000 Andrew Cagney * buildsym.c: Include "language.h" for longest_local_hex_string_custom. Index: buildsym.c =================================================================== RCS file: /cvs/src/src/gdb/buildsym.c,v retrieving revision 1.2 diff -p -r1.2 buildsym.c *** buildsym.c 2000/02/18 22:15:46 1.2 --- buildsym.c 2000/02/21 02:34:07 *************** *** 34,39 **** --- 34,40 ---- #include "gdbtypes.h" #include "complaints.h" #include "gdb_string.h" + #include "language.h" /* For "longest_local_hex_string_custom" */ /* Ask buildsym.h to define the vars it normally declares `extern'. */ #define EXTERN >From ac131313@cygnus.com Sat Apr 01 00:00:00 2000 From: Andrew Cagney To: Kevin Buettner Cc: gdb-patches@sourceware.cygnus.com Subject: Re: [PATCH RFA] bfd/configure.in, bfd/configure Date: Sat, 01 Apr 2000 00:00:00 -0000 Message-id: <38ABBDF5.DEBB1D35@cygnus.com> References: <1000217090915.ZM4782@ocotillo.lan> X-SW-Source: 2000-q1/msg00218.html Content-length: 416 Kevin Buettner wrote: > > Since I'm not a configury maintainer, I need approval to check the > following in: > > * configure.in: Added corefile support for AIX 4.3. In particular, > AIX_CORE_DUMPX_CORE will be defined in addition to AIX_CORE when > compiling rs6000-core.c. > * configure: Regenerated. Kevin, FYI, BFD patches go to the binutils@sourceware mailing list. Andrew >From kevinb@cygnus.com Sat Apr 01 00:00:00 2000 From: Kevin Buettner To: gdb-patches@sourceware.cygnus.com Subject: [PATCH] skip_prologue() revisions in rs6000-tdep.c Date: Sat, 01 Apr 2000 00:00:00 -0000 Message-id: <1000226100500.ZM2908@ocotillo.lan> X-SW-Source: 2000-q1/msg00369.html Content-length: 12055 I've just committed the patch below. It consists of two parts. The first is just a big comment explaining the issues that lead to my recent addition of the function ppc_linux_memory_remove_breakpoint() to ppc-linux-tdep.c. (Andrew's suggestion.) The second consists of a number of revisions to skip_prologue(). The most important revision (and the reason I was in this code this time around) is the introduction of the notion of instructions which may appear in the prologue, but which must not be the last instruction in the prologue. A good example of this is the nop instruction. Consider the disassembly of loop_count() from testsuite/gdb.base/call-ar-st.c: (gdb) x/20i loop_count 0x100009b0 : stwu r1,-32(r1) 0x100009b4 : stw r31,28(r1) 0x100009b8 : mr r31,r1 0x100009bc : nop 0x100009c0 : li r0,0 0x100009c4 : stw r0,8(r31) 0x100009c8 : lwz r0,8(r31) 0x100009cc : cmpwi r0,3 0x100009d0 : ble 0x100009d8 0x100009d4 : b 0x100009e8 0x100009d8 : lwz r9,8(r31) 0x100009dc : addi r0,r9,1 0x100009e0 : stw r0,8(r31) 0x100009e4 : b 0x100009c8 0x100009e8 : lwz r11,0(r1) 0x100009ec : lwz r31,-4(r11) 0x100009f0 : mr r1,r11 0x100009f4 : blr Here's the source: void loop_count () { int index; for (index=0; index<4; index++); } Prior to my change, the nop at loop_count+12 was considered part of the prologue. Yet, according to the line number info, this instruction is the first instruction in the body. On the powerpc, it is sometimes necessary to use two instructions to load a constant into a register, so the nop is actually just appart of the "index=0" assignment. Anyway... since gdb was erroneously concluding that the nop was part of the prologue, skip_prologue would end up returning the address of the "li r0,0" which is part way through the first statement of the function. When you attempt to place a breakpoint on loop_count, gdb will attempt to find the first executable statement after the prologue. If the prologue scanning code returns an address that is part way through a statement, the first address of the next statement is used. As a result, the breakpoint was being placed on loop_count+56 which is the epilogue of the function. This is definitely wrong. The solution is to allow certain instructions like nop to appear in the prologue, but not permit them to form the final instruction in the prologue. (I think there are certain situations where a nop can validly appear in the prologue.) I expect that over time we'll find other instructions that should be classified the same way. Here are the ChangeLog entries and the diffs... * ppc-linux-tdep.c (ppc_linux_memory_remove_breakpoint): Add comment explaining motivation behind this function and why the generic facilities won't work for this platform. * rs6000-tdep.c (skip_prologue): Always test to make sure that an instruction is read successfully from the target's memory. Introduce notion of instructions which may appear in the prologue, but must not terminate the prologue. Added explicit check for nop instruction. Use memset() to zero the frame data instead of assignment from a statically allocated, uninitialized structure. Index: ppc-linux-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v retrieving revision 1.3 diff -u -p -r1.3 ppc-linux-tdep.c --- ppc-linux-tdep.c 2000/02/24 23:06:48 1.3 +++ ppc-linux-tdep.c 2000/02/26 09:10:28 @@ -615,8 +615,130 @@ ppc_sysv_abi_push_arguments (nargs, args return sp; } -/* This version of ppc_linux_memory_remove_breakpoints handles the - case of self modifying code */ +/* ppc_linux_memory_remove_breakpoints attempts to remove a breakpoint + in much the same fashion as memory_remove_breakpoint in mem-break.c, + but is careful not to write back the previous contents if the code + in question has changed in between inserting the breakpoint and + removing it. + + Here is the problem that we're trying to solve... + + Once upon a time, before introducing this function to remove + breakpoints from the inferior, setting a breakpoint on a shared + library function prior to running the program would not work + properly. In order to understand the problem, it is first + necessary to understand a little bit about dynamic linking on + this platform. + + A call to a shared library function is accomplished via a bl + (branch-and-link) instruction whose branch target is an entry + in the procedure linkage table (PLT). The PLT in the object + file is uninitialized. To gdb, prior to running the program, the + entries in the PLT are all zeros. + + Once the program starts running, the shared libraries are loaded + and the procedure linkage table is initialized, but the entries in + the table are not (necessarily) resolved. Once a function is + actually called, the code in the PLT is hit and the function is + resolved. In order to better illustrate this, an example is in + order; the following example is from the gdb testsuite. + + We start the program shmain. + + [kev@arroyo testsuite]$ ../gdb gdb.base/shmain + [...] + + We place two breakpoints, one on shr1 and the other on main. + + (gdb) b shr1 + Breakpoint 1 at 0x100409d4 + (gdb) b main + Breakpoint 2 at 0x100006a0: file gdb.base/shmain.c, line 44. + + Examine the instruction (and the immediatly following instruction) + upon which the breakpoint was placed. Note that the PLT entry + for shr1 contains zeros. + + (gdb) x/2i 0x100409d4 + 0x100409d4 : .long 0x0 + 0x100409d8 : .long 0x0 + + Now run 'til main. + + (gdb) r + Starting program: gdb.base/shmain + Breakpoint 1 at 0xffaf790: file gdb.base/shr1.c, line 19. + + Breakpoint 2, main () + at gdb.base/shmain.c:44 + 44 g = 1; + + Examine the PLT again. Note that the loading of the shared + library has initialized the PLT to code which loads a constant + (which I think is an index into the GOT) into r11 and then + branchs a short distance to the code which actually does the + resolving. + + (gdb) x/2i 0x100409d4 + 0x100409d4 : li r11,4 + 0x100409d8 : b 0x10040984 + (gdb) c + Continuing. + + Breakpoint 1, shr1 (x=1) + at gdb.base/shr1.c:19 + 19 l = 1; + + Now we've hit the breakpoint at shr1. (The breakpoint was + reset from the PLT entry to the actual shr1 function after the + shared library was loaded.) Note that the PLT entry has been + resolved to contain a branch that takes us directly to shr1. + (The real one, not the PLT entry.) + + (gdb) x/2i 0x100409d4 + 0x100409d4 : b 0xffaf76c + 0x100409d8 : b 0x10040984 + + The thing to note here is that the PLT entry for shr1 has been + changed twice. + + Now the problem should be obvious. GDB places a breakpoint (a + trap instruction) on the zero value of the PLT entry for shr1. + Later on, after the shared library had been loaded and the PLT + initialized, GDB gets a signal indicating this fact and attempts + (as it always does when it stops) to remove all the breakpoints. + + The breakpoint removal was causing the former contents (a zero + word) to be written back to the now initialized PLT entry thus + destroying a portion of the initialization that had occurred only a + short time ago. When execution continued, the zero word would be + executed as an instruction an an illegal instruction trap was + generated instead. (0 is not a legal instruction.) + + The fix for this problem was fairly straightforward. The function + memory_remove_breakpoint from mem-break.c was copied to this file, + modified slightly, and renamed to ppc_linux_memory_remove_breakpoint. + In tm-linux.h, MEMORY_REMOVE_BREAKPOINT is defined to call this new + function. + + The differences between ppc_linux_memory_remove_breakpoint () and + memory_remove_breakpoint () are minor. All that the former does + that the latter does not is check to make sure that the breakpoint + location actually contains a breakpoint (trap instruction) prior + to attempting to write back the old contents. If it does contain + a trap instruction, we allow the old contents to be written back. + Otherwise, we silently do nothing. + + The big question is whether memory_remove_breakpoint () should be + changed to have the same functionality. The downside is that more + traffic is generated for remote targets since we'll have an extra + fetch of a memory word each time a breakpoint is removed. + + For the time being, we'll leave this self-modifying-code-friendly + version in ppc-linux-tdep.c, but it ought to be migrated somewhere + else in the event that some other platform has similar needs with + regard to removing breakpoints in some potentially self modifying + code. */ int ppc_linux_memory_remove_breakpoint (CORE_ADDR addr, char *contents_cache) { Index: rs6000-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v retrieving revision 1.4 diff -u -p -r1.4 rs6000-tdep.c --- rs6000-tdep.c 2000/02/24 23:06:48 1.4 +++ rs6000-tdep.c 2000/02/26 09:10:31 @@ -305,11 +305,10 @@ rs6000_software_single_step (signal, ins #define GET_SRC_REG(x) (((x) >> 21) & 0x1f) CORE_ADDR -skip_prologue (pc, fdata) - CORE_ADDR pc; - struct rs6000_framedata *fdata; +skip_prologue (CORE_ADDR pc, struct rs6000_framedata *fdata) { CORE_ADDR orig_pc = pc; + CORE_ADDR last_prologue_pc; char buf[4]; unsigned long op; long offset = 0; @@ -318,24 +317,31 @@ skip_prologue (pc, fdata) int reg; int framep = 0; int minimal_toc_loaded = 0; - static struct rs6000_framedata zero_frame; + int prev_insn_was_prologue_insn = 1; - *fdata = zero_frame; + memset (fdata, 0, sizeof (struct rs6000_framedata)); fdata->saved_gpr = -1; fdata->saved_fpr = -1; fdata->alloca_reg = -1; fdata->frameless = 1; fdata->nosavedpc = 1; - if (target_read_memory (pc, buf, 4)) - return pc; /* Can't access it -- assume no prologue. */ - - /* Assume that subsequent fetches can fail with low probability. */ pc -= 4; for (;;) { pc += 4; - op = read_memory_integer (pc, 4); + + /* Sometimes it isn't clear if an instruction is a prologue + instruction or not. When we encounter one of these ambiguous + cases, we'll set prev_insn_was_prologue_insn to 0 (false). + Otherwise, we'll assume that it really is a prologue instruction. */ + if (prev_insn_was_prologue_insn) + last_prologue_pc = pc; + prev_insn_was_prologue_insn = 1; + + if (target_read_memory (pc, buf, 4)) + break; + op = extract_signed_integer (buf, 4); if ((op & 0xfc1fffff) == 0x7c0802a6) { /* mflr Rx */ @@ -375,6 +381,16 @@ skip_prologue (pc, fdata) continue; } + else if ((op & 0xffff0000) == 0x60000000) + { + /* nop */ + /* Allow nops in the prologue, but do not consider them to + be part of the prologue unless followed by other prologue + instructions. */ + prev_insn_was_prologue_insn = 0; + continue; + + } else if ((op & 0xffff0000) == 0x3c000000) { /* addis 0,0,NUM, used for >= 32k frames */ @@ -564,7 +580,7 @@ skip_prologue (pc, fdata) #endif /* 0 */ fdata->offset = -fdata->offset; - return pc; + return last_prologue_pc; }