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

[Zachary Turner via lldb-dev]

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/20150929/660c9910/attachment.html>