| Message ID | 529CF8B3.60906@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
On 3 December 2013 02:16, Omair Javaid <omair.javaid@linaro.org> wrote: > > On 10/01/2013 08:34 PM, Pedro Alves wrote: >> On 10/01/2013 09:32 AM, Omair Javaid wrote: >>> On 19 September 2013 20:53, Pedro Alves <palves@redhat.com> wrote: >>>> Please don't top post. >>>> >>>> On 09/19/2013 04:23 PM, Omair Javaid wrote: >>>>> Thanks everyone for the feedback. >>>>> >>>>> I am getting following problem with 1byte text section in the dw2-dos-drive.exp >>>>> >>>>> (gdb) PASS: gdb.dwarf2/dw2-dos-drive.exp: set breakpoint pending off >>>>> break 'z:file.c':func >>>>> Cannot access memory at address 0x0 >>>>> >>>>> When I change this to 4bytes the problem gets fixed. That is why I >>>>> thought this could be an unaligned illegal memory access but I accept >>>>> that the above comments verify that its not a alignment issue. >>>>> >>>>> Can anyone help me figure out what could be the cause of this problem? >>>> >>>> Breakpoint instructions on ARM are 4-byte wide. It sounds like >>>> GDB is trying to read the memory at the breakpoint's address, and >>>> that fails (that error message comes from GDB, not the program). >>>> AFAICS, the test doesn't execute the compiled object's code, so >>>> GDB will try to read memory from the binary's sections. As the >>>> section is only 1 byte long, and probably no other section is allocated >>>> contiguously, that'll fail... To confirm, debug GDB under GDB, >>>> and put a break on throw_it or some such. Then work up the stack >>>> to see where that is thrown, and why. >>>> >>>> -- >>>> Pedro Alves >>>> >>> >>> I have verified the error is being thrown by gdb while its unable to >>> read the 4byte breakpoint address. >>> Heres the call stack: >>> Thread [1] (Suspended: Breakpoint hit.) >>> 38 throw_error() exceptions.c:444 0x0016728c >>> 37 memory_error() corefile.c:204 0x001d1fcc >>> 36 read_memory() corefile.c:223 0x001d201a >>> 35 read_memory_unsigned_integer() corefile.c:312 0x001d2166 >>> 34 arm_skip_prologue() arm-tdep.c:1452 0x00054270 >> >> Right, though this is actually parsing the prologue: >> >> static CORE_ADDR >> arm_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc) >> { >> ... >> for (skip_pc = pc; skip_pc < limit_pc; skip_pc += 4) >> { >> inst = read_memory_unsigned_integer (skip_pc, 4, byte_order_for_code); >> >> Some ports detect errors and instead return the PC as far >> as it was managed to be skip. >> E.g. rs6000-tdep.c:skip_prologue (rs6000==PowerPC): >> >> /* Fetch the instruction and convert it to an integer. */ >> if (target_read_memory (pc, buf, 4)) >> break; >> op = extract_unsigned_integer (buf, 4, byte_order); >> >> But not all do that. SPARC also doesn't throw. But others do throw >> an error like ARM. I tried SH and that throws error like ARM; MIPS >> and xtensa, from inspection, look like they'll throw but I haven't >> tried it. AAarch64 throws like ARM, but that's not surprising. >> Anyway, there's no standard. >> >>> 33 gdbarch_skip_prologue() gdbarch.c:2603 0x00176e5c >>> 32 skip_prologue_sal() symtab.c:2869 0x0013dad2 >>> 31 find_function_start_sal() symtab.c:2782 0x0013d9aa >>> 30 symbol_to_sal() linespec.c:3622 0x0014f722 >>> 29 convert_linespec_to_sals() linespec.c:2028 0x0014d6fa >>> 28 parse_linespec() linespec.c:2319 0x0014dc04 >>> 27 decode_line_full() linespec.c:2430 0x0014df44 >>> 26 parse_breakpoint_sals() breakpoint.c:9323 0x00108560 >> ... >> >>> I guess only way to address it is to either use the patch I have >>> posted or may be disable the test for arm? Any suggestions? >> >> Another other way to handle this would be to make the prologue >> scanner cope with this, and not error out, like some ports do. But >> it's not clear at all to me that's a useful behavior. Even if we >> pretended we found the end of the prologue in this case, the address >> we would find in this particular case would never be a valid address >> to put a breakpoint at (the function's first address). If we tried >> setting a breakpoint there, who knows what is it would be overwritten >> by the bytes that fall off the section (we can be 99.99% sure >> the next section would be aligned, and the gap wouldn't be used >> for anything, but still... So, I think it might be better to leave >> the scanner as is, throwing the error while it has context about >> it, and let the user (or higher-level code) decide what to do. >> >> Another way to tackle this could be to actually disable prologue >> skipping, by setting the breakpoint at exactly the func's first >> instruction, with the '*'/address operator: >> >> -gdb_test "break 'z:file.c':func" {Breakpoint [0-9]+ at .*} >> +gdb_test "break *'z:file.c'::func" {Breakpoint [0-9]+ at .*} >> >> This doesn't actually work, though I think that's a bug. I'll >> file a PR. >> >> But, even if it did, that converts a linespec to an expression, >> which may not be a universal solution, as tests with this issue >> might need to use a "real" linespec... >> >> So, in the end, it'd be fine with me to just go in the >> direction of your original patch then. But I think it deserves >> a comment: >> >> pc_start: >> /* Enough space to fit one instruction. */ >> - .byte 0 >> + .4byte 0 >> pc_end: >> >> Could you resend your patch, with that change, a fixed commit >> log description and fixed ChangeLog? >> >> Thanks, >> > > Sorry about responding late to this. I have attached the patch along with commit message and a ChangeLog. > > Commit Log Message: > > Avoid test failure due to error thrown from skip prologue code by > an illegal memory access in case of single byte text section > > gdb/testsuite/ChangeLog: > > 2013-12-02 Omair Javaid <Omair.Javaid@linaro.org> > > * gdb.dwarf2/dw2-dos-drive.S: Changed text section size to 4 bytes > > --- > gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S b/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S > index 682ba4e..f226912 100644 > --- a/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S > +++ b/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S > @@ -15,7 +15,7 @@ > > .text > pc_start: > - .byte 0 > + .4byte 0 > pc_end: > > .section .debug_info > -- Ping! OK to commit?
On Thu 16 Jan 2014 03:25:34 PM PKT, Pedro Alves wrote: > On 12/02/2013 09:16 PM, Omair Javaid wrote: >> On 10/01/2013 08:34 PM, Pedro Alves wrote: >>> So, in the end, it'd be fine with me to just go in the >>> direction of your original patch then. But I think it deserves >>> a comment: >>> >>> pc_start: >>> /* Enough space to fit one instruction. */ >>> - .byte 0 >>> + .4byte 0 >>> pc_end: >>> >>> Could you resend your patch, with that change, a fixed commit >>> log description and fixed ChangeLog? > >> Sorry about responding late to this. I have attached the patch along with commit message and a ChangeLog. >> >> Commit Log Message: >> >> Avoid test failure due to error thrown from skip prologue code by >> an illegal memory access in case of single byte text section > > Period at end of sentences. That log if very incomplete though. > See below for what I suggest. (Note I tweaked the subject too) > > The best/easiest is to write that in the git commit log in your > local tree, and send out the commit exactly as you propose > to push. > >> gdb/testsuite/ChangeLog: >> >> 2013-12-02 Omair Javaid <Omair.Javaid@linaro.org> >> >> * gdb.dwarf2/dw2-dos-drive.S: Changed text section size to 4 bytes > > Period at end of sentence. > >> >> --- >> gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S b/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S >> index 682ba4e..f226912 100644 >> --- a/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S >> +++ b/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S >> @@ -15,7 +15,7 @@ >> >> .text >> pc_start: >> - .byte 0 >> + .4byte 0 > > Still misses comment. The version below adds one similar to > what I suggested before. > > If this looks good to you, I'll push it. > > Thanks. > > -- >8 -- > From: Omair Javaid <omair.javaid@linaro.org> > Subject: Fix testsuite/gdb.dwarf2/dw2-dos-drive.exp on ARM. > > This test currently fails on ARM: > > (gdb) PASS: gdb.dwarf2/dw2-dos-drive.exp: set breakpoint pending off > break 'z:file.c':func > Cannot access memory at address 0x0 > > The error is GDB trying to read the prologue at the breakpoint's > address, and failing: > > 38 throw_error() exceptions.c:444 0x0016728c > 37 memory_error() corefile.c:204 0x001d1fcc > 36 read_memory() corefile.c:223 0x001d201a > 35 read_memory_unsigned_integer() corefile.c:312 0x001d2166 > 34 arm_skip_prologue() arm-tdep.c:1452 0x00054270 > > static CORE_ADDR > arm_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc) > { > ... > for (skip_pc = pc; skip_pc < limit_pc; skip_pc += 4) > { > inst = read_memory_unsigned_integer (skip_pc, 4, byte_order_for_code); > > > The test doesn't execute the compiled object's code, so GDB will try > to read memory from the binary's sections. Instructions on ARM are > 4-byte wide, and thus ARM's prologue scanner reads in 4-byte chunks. > As the section 'func' is put at is only 1 byte long, and no other > section is allocated contiguously: > > ... > Sections: > Idx Name Size VMA LMA File off Algn > 0 .text 00000001 00000000 00000000 00000034 2**0 > CONTENTS, ALLOC, LOAD, READONLY, CODE > ... > > ... the exec target fails the read the 4 bytes. > > Fix this by increasing the function's size. > > gdb/testsuite/ChangeLog: > 2014-01-16 Omair Javaid <Omair.Javaid@linaro.org> > > * gdb.dwarf2/dw2-dos-drive.S: Increase text section size to 4 > bytes. > --- > gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S b/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S > index 6e9d360..84006f7 100644 > --- a/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S > +++ b/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S > @@ -15,7 +15,8 @@ > > .text > pc_start: > - .byte 0 > + /* Enough space to fit at least one instruction. */ > + .4byte 0 > pc_end: > > .section .debug_info Changes look good. Thanks. Kindly go ahead and push.
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S b/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S index 682ba4e..f226912 100644 --- a/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S +++ b/gdb/testsuite/gdb.dwarf2/dw2-dos-drive.S @@ -15,7 +15,7 @@ .text pc_start: - .byte 0 + .4byte 0 pc_end: .section .debug_info