[cfe-dev] Implementation of MSRecordLayoutBuilder.
Don Williamson
don.williamson at yahoo.com
Thu Sep 15 04:14:45 PDT 2011
Sure, I started a previous thread about this:
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-September/016941.html
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-September/016942.html
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-September/016944.html
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-September/016970.html
It boils down to two problems I've had:
* If there is a double anywhere in a struct, MSVC pads the vftbl pointer.
* The IsPODForThePurposeOfLayout check is invalid for MS targets and so you get a different layout depending upon whether your type has a struct or not.
I suggested a simple patch in my last post that fixes these. My test cases so far are here:
https://bitbucket.org/dwilliamson/clreflect/src/tip/src/clReflectTest/TestOffsets.cpp
my patch works for all of them but unfortunately I haven't had time to sync to the lastest build of clang so I'm not in a position to submit a patch.
It would be nice if we could figure something out that solves the biggest number of cases :)
Cheers,
- Don
________________________________
From: r4start <r4start at gmail.com>
To: Don Williamson <don.williamson at yahoo.com>
Cc: cfe-dev at cs.uiuc.edu
Sent: Thursday, September 15, 2011 12:06 PM
Subject: Re: [cfe-dev] Implementation of MSRecordLayoutBuilder.
On 15/09/2011 14:52, Don Williamson wrote:
Hi Dmitry,
>
>
>I'm really interested in the original test cases you have that failed with the original implementation of the record layout builder - could you share them?
>
>
>How does your current patch work with the following test:
>
>
>#pragma pack(push, 8)
>class B {
>public:
> virtual void b(){}
> int a;
> double b;
>};
>#pragma pack(pop)
>
>
>Have you also tried test cases with and without constructors?
>
>
>Thanks,
>- Don
>
>
>
>________________________________
>From: r4start <r4start at gmail.com>
>To: John McCall <rjmccall at apple.com>
>Cc: cfe-dev at cs.uiuc.edu
>Sent: Thursday, September 15, 2011 11:20 AM
>Subject: Re: [cfe-dev] Implementation of MSRecordLayoutBuilder.
>
>On 15/09/2011 00:39, r4start wrote:
>> On 15/09/2011 00:36, John McCall wrote:
>>> On Sep 14, 2011, at 1:33 PM, r4start wrote:
>>>
>>>> On 15/09/2011 00:24, John McCall wrote:
>>>>> On Sep 14, 2011, at 1:23 PM, r4start wrote:
>>>>>> On 14/09/2011 21:53, John McCall wrote:
>>>>>>> On Sep 14, 2011, at 4:05 AM,
r4start wrote:
>>>>>>>
>>>>>>>> On 14/09/2011 00:42, r4start
wrote:
>>>>>>>>> On 14/09/2011 00:37, Eli
Friedman wrote:
>>>>>>>>>> On Mon, Sep 12, 2011 at
11:45 PM, r4start<r4start at gmail.com> wrote:
>>>>>>>>>>> On 12/09/2011
21:57, r4start wrote:
>>>>>>>>>>>> On 12/09/2011
21:37, Eli Friedman wrote:
>>>>>>>>>>>>> On Mon, Sep
12, 2011 at 8:56 AM, r4start<r4start at gmail.com> wrote:
>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>> I have
some prototype code for MSRecordLayoutBuilder. I test
>>>>>>>>>>>>>> this
code
>>>>>>>>>>>>>> with
MSVS 2010 and \Zp8 compiler option. On simple examples it
>>>>>>>>>>>>>> seems
to
>>>>>>>>>>>>>> work
properly.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Is it
interesting to the clang project?
>>>>>>>>>>>>> We
certainly want to fix any cases where clang does not do
struct
>>>>>>>>>>>>> layout
consistently with MSVC on Windows. It's hard to comment on
>>>>>>>>>>>>> your patch
without seeing it.
>>>>>>>>>>>>>
>>>>>>>>>>>>> -Eli
>>>>>>>>>>>> Now I'm not at
work. When I come to work I'll send a patch.
>>>>>>>>>>>>
>>>>>>>>>>>> - Dmitry
>>>>>>>>>>> Here is patch for
MSRecordLayoutBuilder.
>>>>>>>>>> I think it would be
better to integrate the checks into the main
>>>>>>>>>> RecordLayoutBuilder
class rather than subclassing it.
>>>>>>>>>>
>>>>>>>>>> Please include tests in
your patch. (See test/CodeGen/ms_struct.c for
>>>>>>>>>> an example.)
>>>>>>>>>>
>>>>>>>>>> I would like someone
more familiar with MSVC to review the actual
>>>>>>>>>> logic here (ping me if
nobody does within a few days, though, and I'll
>>>>>>>>>> try to review anyway).
>>>>>>>>>>
>>>>>>>>>> -Eli
>>>>>>>>> Ok, thanks for hint.
>>>>>>>>>
>>>>>>>>> -Dmitry.
>>>>>>>> I have a problem when writing a
test. Clang doesn't implement Microsoft
>>>>>>>> ABI and he crashes at code
generation stage.
>>>>>>>> Clang have dump-record-layouts
option but he prints AST layouts and
>>>>>>>> CodeGen layouts.
>>>>>>>>
>>>>>>>> Is there a way to save only AST
record layout to file and then check
>>>>>>>> or maybe my patch must include
CGRecordLayoutBuilder for Microsoft?
>>>>>>> It would be easy enough to add a
new testing switch if you'd like to get this in.
>>>>>> Can you give me some hint how can I do
this?
>>>>> Follow the code for -dump-record-layouts.
>>>> You propose to duplicate the output to a file
and then run FileCheck to check output, am I right?
>>> It would probably be better to run FileCheck
directly on the output, but
>>> I think you get the general idea.
>>>
>>> Whenever you get CGRecordLayoutBuilder working
adequately, you can rip
>>> out the new option and just use
-dump-record-layouts.
>>>
>>> John.
>> Ok, thanks. Tomorrow I'll do it.
>>
>Here is a test and patch.
>
>- Dmitry.
>
>
>_______________________________________________
>cfe-dev mailing list
>cfe-dev at cs.uiuc.edu
>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
>
Hi,
what I get from MSVS output :
class B size(24):
1> +---
1> 0 | {vfptr}
1> 8 | a
1> | <alignment member> (size=4)
1> 16 | c
1> +---
And this I get from clang with my patch:
0 | class B
0 | (B vtable pointer)
4 | int a
8 | double c
sizeof=16, dsize=16, align=8
nvsize=16, nvalign=8
I'll try to fix this.
What do you mean when you say "Have you also tried test cases with and without constructors?", can you give me some examples and I check them.
- Dmitry.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110915/c7f86422/attachment.html>
More information about the cfe-dev
mailing list