This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On 06/20/2015 04:25 AM, Martin Galvan wrote:
> 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.

Then it sounds like you were putting a breakpoint at a line that was
optimized out.  If you put the breakpoint at the same line that you
"step" and "next" to, then it should work, I think.  You'll still
need a "step" to go from real stack frame to inline frame, as GDB always
presents you the stack frame on breakpoint hits.

> 
>>> +  /* 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.

Hmm, that looks too long to me and still escaping out a detail
that isn't really that interesting to the user.  I was just thinking:

if (is reference)
 "Attempt to take address of a synthetic reference."
else
 "Attempt to take address of a synthetic pointer."

> 
>> 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.

Do you have an x86-64 box?  If so, you can just run the test with:

 make check RUNTESTFLAGS="--target_board=unix/-m32 gdb.dwarf2/implptr.exp"

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

Actually from your description, I assume it _doesn't_ crash, because it's
not a reference.  But it's not tested presently, so who know.  I think it's
worth it to have both pointers and references covered.

Thanks,
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]