Re: [PATCH v2] Fix gdb crash when trying to print the address of a synthetic pointer.

[Martin Galvan <martin dot galvan at tallertechnologies dot com>]

On Mon, Jun 15, 2015 at 8:17 AM, Pedro Alves <palves@redhat.com> wrote:
> Also, we can't/shouldn't assume that gcc -O3 will always generate
> the expected dwarf info.  That's why tests under gdb.dwarf2/
> either include the compiled .S file, or preferably, use
> the Dwarf::assemble machinery.

Understood.

>> +# If we tried to set a breakpoint and then 'continue' up to it, the program
>> +# would exit without us hitting it because of the optimizations performed
>> +# by gcc. We must advance using 'next' and 'step' instead.
>> +
>
> "optimizations performed by gcc" is a bit vague.  Is it that "function"
> ends up inlined?  What exactly doesn't work?

Yes, 'function' ends up inlined. If we try to set a breakpoint at a
line inside 'function', the breakpoint will be set at an address we'll
never hit, thus we need to advance using 'step' and 'next' instead.

>> +  /* The TYPE_CODE_REF path below causes gdb to crash for synthetic pointers
>> +     created from C++ references. Catch those cases here. */
>
> This "causes gdb to crash" really isn't that helpful for future
> readers.  The immediate question readers will ask is:
>
> "Why would it crash?  Because of a GDB bug?  Or because it doesn't
> make sense because $some_reason?"
>
> So the comment should instead be in terms of $some_reason.  Also,
> double space after periods.

Ok.

> I wonder whether the error message should really talk about pointers,
> when the user tried to look at a reference?  That looks like a bit
> of implementation detail escaping out.

What about "Attempt to take address of a synthetic pointer generated
from a C++ reference"? The users can see it's a synthetic pointer
anyways, so we could be as explicit as possible.

> Also, gdb.dwarf2/implptr.exp has:
>
> # Test various pointer depths in bar.
> proc implptr_test_bar {} {
>     global csrcfile
>     set line [gdb_get_line_number "bar breakpoint" $csrcfile]
>     gdb_test "break implptr.c:$line" "Breakpoint 2.*" \
>        "set bar breakpoint for implptr"
>     gdb_continue_to_breakpoint "continue to bar breakpoint for implptr"
>     gdb_test "print j" " = \\(intp\\) <synthetic pointer>" "print j in implptr:bar"
>
> Sounds like a "print &j" test should be added here too.

I haven't been able to test this so far because the test requires an
x86-like target, which I don't have at hand. I'll keep you posted,
though.

Assuming gdb crashes there too, couldn't I just add a "print &j" test
there and discard the synthetic-pointer-reference.exp test altogether?

>  (gdb) p arg
>  $2 = (int &) <synthetic pointer>
>
> Without optimization we would get:
>
>  (gdb) p arg
>  $1 = (int &) @0x7fffffffd71c: 42
>
> So I'd expect to get something like:
>
>  (gdb) p arg
>  $1 = (int &) @<synthetic pointer>: 42
>
> Because as is it seems like there's no way to get the
> referenced value, even thought it's right there in the
> debug info.  (If it weren't there, there'd be no point
> on describing it as an implicit/synthetic pointer.)
>
> I noticed more breakage while trying to get at the
> reference's value, knowing it's an integer:
>
>  (gdb) p arg + 0
>  Cannot access memory at address 0x0
>
> These issues don't have to be handled by this patch, thought
> thinking through them may point at fixing the crash at hand
> differently.  Or not, I really haven't thought about it much.
> But I thought I'd point it out regardless.

I might take a look into these issues in a later patch :)

-- 

Martin Galvan

Software Engineer

Taller Technologies Argentina

San Lorenzo 47, 3rd Floor, Office 5

CÃrdoba, Argentina

Phone: 54 351 4217888 / +54 351 4218211