[lldb-dev] r247953 - TypeSystem is now a plugin interface

[Zachary Turner via lldb-dev]

I was able to track this down to a bug in Clang.  I have a patch up for
review.

http://reviews.llvm.org/D13276

I'm not sure what part of the TypeSystem refactor caused this to finally be
exposed, but in any case I expect that patch will go through and this bug
will be fixed.



On Tue, Sep 29, 2015 at 3:40 PM Zachary Turner <zturner at google.com> wrote:

> Resending since the wrong list was CC'ed.
>
>
> On Tue, Sep 29, 2015 at 3:39 PM Zachary Turner <zturner at google.com> wrote:
>
>> Hi Greg.  I'm responding on this thread since you said lldb-dev is a
>> better place for questions.
>>
>> I had a chance to look at this some more. It looks like the way we're
>> constructing the clang::RecordDecl is incorrect on Windows.  I'm not sure
>> what record layout is like on non-Windows platforms, but if we have this
>> code:
>>
>> class A {
>>     virtual ~A() {}
>>     int a;
>> };
>>
>> class B : public A {
>>     virtual ~B() {}
>>     int b;
>> };
>>
>> A * pa = new B ();
>>
>> then using the MS ABI, offset of a is 4 and offset of B is 8.  I verified
>> this by modifying the source of sbvalue-cast.cpp to print the offsets.
>>
>> d:\src\llvm\tools\lldb\test\lang\cpp\dynamic-value>a.out
>> DerivedA::ctor()->
>> m_base_val=20
>> m_a_val=10
>> *offset of DerivedA::m_a_val is 8*
>> *offset of DerivedA::m_base_val is 4*
>>
>> Clang agrees with me as well.  Compiling with -Xclang -emit-llvm-only
>> -Xclang -fdump-record-layouts gives me the following AST Record Layout
>>
>> *** Dumping AST Record Layout
>>    0 | class DerivedA
>>    0 |   class Base (primary base)
>>    0 |     (Base vftable pointer)
>>  *  4 |     int m_base_val*
>>    8 |   int m_a_val
>>      | [sizeof=12, align=4
>>      |  nvsize=12, nvalign=4]
>>
>> On the LLDB side, we have ClangASTContext::GetChildCompilerTypeAtIndex
>> which computes the byte offset, field name, etc for the ValueObject.  On
>> line 5754 it begins querying the name and byte offset of the field.  The
>> name returns `m_base_val`, but the field_offset returns 8 bytes, which we
>> can see above is wrong.
>>
>> Any suggestions where I might look to dig further?
>>
>> On Wed, Sep 23, 2015 at 10:34 AM Greg Clayton <clayborg at gmail.com> wrote:
>>
>>> I responded in another e-mail with follow stuff you will need to send
>>> me. Always send questions to lldb-dev and not lldb-commits as there is so
>>> much traffic in lldb-commmits it might get lost. I keep up with lldb-dev
>>> daily.
>>>
>>> > On Sep 22, 2015, at 10:12 AM, Zachary Turner <zturner at google.com>
>>> wrote:
>>> >
>>> > In regards to this CL:
>>> http://llvm.org/viewvc/llvm-project?rev=247953&view=rev
>>> >
>>> > It has broken TestCppValueCast on Windows and fixed TestCxxWcharT on
>>> Windows as well.  Although these appear to be platform specific behavioral
>>> changes, as I don't see anyone else reporting failures or fixes, the
>>> description of the CL leads me to infer that this change was supposed to
>>> have no functional change.  I haven't gotten any response pinging the
>>> thread so I'll pose it to the wider list for visibility in case someone can
>>> shed some light on this.  In particular, is this supposed to come with a
>>> functional change, and if so what?
>>> >
>>> > Unfortunately I don't have buildbots running tests on Windows yet, so
>>> I can't justify reverting the CL (and it's too old to revert now anyway
>>> since other things depend on it), but it would greatly appreciate if
>>> someone can take a look at this and see if they can identify the source of
>>> the behavioral change
>>> >
>>> > I do plan to have a buildbot running tests within a week or two, so
>>> hopefully we can catch this kind of thing much sooner next time.
>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20150930/cef32978/attachment.html>