Re: [PATCH] testsuite/gdb.dwarf2: Fix for dw2-dos-drive failure on ARM

[Pedro Alves <palves@redhat.com>]

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,
-- 
Pedro Alves