Re: Patch for PR 9399

[Chris Moller <cmoller@redhat.com>]

On 12/09/09 09:05, Daniel Jacobowitz wrote:
> On Wed, Dec 09, 2009 at 08:32:34AM -0500, Chris Moller wrote:
>
>> The patch file includes the patch to gdb/valops.c, gdb/ChangeLog,
>> gdb/testsuite/ChangeLog, gdb/testsuite/gdb.cp/Makefile.in, and
>> mc-log.diffs.  (The patch to  Makefile.in is to incorporate the
>> testcase for 'make check'; mc-log.diffs are the diffs between before
>> and after runs of 'make check')
>>
>
> Next time, please diff -u gdb.sum files; the rest is just noise.
>

Okay--if I ever do this again. :-)

> Those do vary a little bit but they're more stable.  It looks like
> your only change is mi-nsmoribund.exp, which is sometimes flaky.
>
>
>> I can't do a 'cvs add' so the the
>> testcase expects and .cc file are attached separately as
>> gdb/testsuite/gdb.cp/virtfunc2.cc and
>> gdb/testsuite/gdb.cp/virtfunc2.exp.
>>
>
> Take a look at cvsutils; "cvsdo add" works.
>
> (Or, nowadays, I suppose you could use the git mirror! :-)
>

Didn't know you had a git mirror--I just pulled the CVS version because 
it was pointed to on http://sourceware.org/gdb/

>
>> You can see what the patch does by compiling -g virtfunc.cc, gdb-ing
>> it, breaking in the return stmt at // marker1, and doing things like
>> "print o.do_print()".  Without the patch, gdb tries to access
>> location 0x0; with the patch it does the right thing.  (There are
>> more tests in virtfunc2.exp)
>>
>
> Where does the access to 0x0 come from?  Is it inside
> search_struct_field?

Ultimately, yes.  Without the patch, the thread ultimately gets to

      if (BASETYPE_VIA_VIRTUAL (type, i))

in search_struct_field and then to the memcpy about 30 lines later that 
extracts a new value struct.  That main_type of that value doesn't 
include a field for the virtual function, so it's never found, and 
ultimately returns a null pointer.  I haven't a clue why it works that 
way--for a while I was working on the assumption that the DWARF reader 
was screwing up, but if it is, it's too subtle for me.

>   I wouldn't expect value_cast_structs to do any
> cast in this case,

value_cast_structs only does nothing if both TYPE_NAME()s are null.  I 
was wondering if, back when the code was originally written, if there 
never was a case when both TYPE_NAME()s were non-null--it's the only 
way, other than simple oversight, I can account for that case not being 
covered.

> but it does do a little extra work.
>
>
>> +   if ((TYPE_NAME (t1) != NULL)&&
>> +       (TYPE_NAME (t2) != NULL)&&
>> +       !strcmp (TYPE_NAME (t1), TYPE_NAME (t2)))
>>
>
> &&  on the beginning of the line, please.
>
>