Implementation of the CGRecordLayoutBuilder for Microsoft ABI.

Hi all!

I am writing CGRecordLayoutBuilder for Microsoft ABI and I have a problem.
I have the following code for testing test code - Pastebin.com .

When generated layout for a class C, I have break assert in CGRecordLayoutBuilder.cpp line 968
assert(TypeSizeInBits == getTargetData().getTypeAllocSizeInBits(Ty) &&
          "Type size mismatch!");.
During debugging, I discovered that TypeSizeInBits smaller getTargetData().getTypeAllocSizeInBits(Ty) 4 bytes.
After investigation, I discovered that the alignment is added in StructLayout constructor (TargetData.cpp line 73)
// Add padding to the end of the struct so that it could be put in an array
  // and all array elements would be aligned correctly.
   if ((StructSize & (StructAlignment-1)) != 0)
     StructSize = TargetData::RoundUpAlignment(StructSize, StructAlignment);

But this alignment is not necessary for class C.
MSVC doesn't add to the end of the class alignment, if it has a virtual base classes.
I see two solutions to this problem:
  1. Rewrite assert like this
      (pseudocode)
      if ABI == Microsoft && Class has virtual basses
        assert(TypeSizeInBits == (getTargetData().getTypeAllocSizeInBits(Ty) - 32) &&
                     "Type size mismatch!");
       else
         old version;
   2. Provide to StructLyout information about ABI.

I think the first way is more simple.

What is the best way to solve this problem?
Maybe you able to offer a better approach?

- Dmitry.

No. A *lot* of downstream code will be broken if LLVM doesn't lay
out the IR type the way we think it's laid out, and that includes the
presence or absence of tail padding.

I don't see how this is possible, though. Can you run through an
example? What's the size and layout of class B here?

class A {
  char c;
};

class B : public virtual A {
  void *p;
};

I would expect that sizeof(B) is 12, and that it's laid out like this:
  [0-3] virtual base pointer
  [4-7] B.p
    [8] A.c
[9-11] tail padding

Or are we talking about the base-subobject struct type? It shouldn't
matter whether that type gets tail padding or not, because it's never
separately allocated.

John.

I am writing CGRecordLayoutBuilder for Microsoft ABI and I have a problem.
I have the following code for testing test code - Pastebin.com .

When generated layout for a class C, I have break assert in
CGRecordLayoutBuilder.cpp line 968
assert(TypeSizeInBits == getTargetData().getTypeAllocSizeInBits(Ty)&&
          "Type size mismatch!");.
During debugging, I discovered that TypeSizeInBits smaller
getTargetData().getTypeAllocSizeInBits(Ty) 4 bytes.
After investigation, I discovered that the alignment is added in
StructLayout constructor (TargetData.cpp line 73)
// Add padding to the end of the struct so that it could be put in an array
  // and all array elements would be aligned correctly.
   if ((StructSize& (StructAlignment-1)) != 0)
     StructSize = TargetData::RoundUpAlignment(StructSize, StructAlignment);

But this alignment is not necessary for class C.
MSVC doesn't add to the end of the class alignment, if it has a virtual
base classes.
I see two solutions to this problem:
  1. Rewrite assert like this
      (pseudocode)
      if ABI == Microsoft&& Class has virtual basses
        assert(TypeSizeInBits ==
(getTargetData().getTypeAllocSizeInBits(Ty) - 32)&&
                     "Type size mismatch!");
       else
         old version;
   2. Provide to StructLyout information about ABI.

I think the first way is more simple.

No. A *lot* of downstream code will be broken if LLVM doesn't lay
out the IR type the way we think it's laid out, and that includes the
presence or absence of tail padding.

I don't see how this is possible, though. Can you run through an
example? What's the size and layout of class B here?

class A {
   char c;
};

class B : public virtual A {
   void *p;
};

I would expect that sizeof(B) is 12, and that it's laid out like this:
   [0-3] virtual base pointer
   [4-7] B.p
     [8] A.c
  [9-11] tail padding

MSVS with pragma pack 8 generates such layout:
1> class A size(1):
1> +---
1> 0 | c
1> +---

1> class B size(9):
1> +---
1> 0 | {vbptr}
1> 4 | p
1> +---
1> +--- (virtual base A)
1> 8 | c
1> +---

My code generates same layout (i32* in B is vbtable pointer):
*** Dumping AST Record Layout
    0 | class A
    0 | char c
   sizeof=1, dsize=1, align=1
   nvsize=1, nvalign=1

LLVMType:%class.A = type { i8 }
NonVirtualBaseLLVMType:%class.A = type { i8 }
IsZeroInitializable:1

*** Dumping AST Record Layout
    0 | class B
    0 | (B vtable pointer)
    0 | (B vbtable pointer)
    4 | void * p
    8 | class A (virtual base)
    8 | char c
   sizeof=9, dsize=9, align=4
   nvsize=8, nvalign=4

LLVMType:%class.B = type { i32*, i8*, %class.A }
NonVirtualBaseLLVMType:%class.B.base = type { i32*, i8* }
IsZeroInitializable:1

Or are we talking about the base-subobject struct type? It shouldn't
matter whether that type gets tail padding or not, because it's never
separately allocated.

John.

MSVC doesn't add additional alignment for virtual base classes, so class size can not be a multiple of alignment(in this example it's 9, but pragma is 8).
But StructLayout ctor adds trailing alignment.

  - Dmitry.

That's can't be what MSVC actually does: the size of B must be a
multiple of its alignment. If you're representation of what MSVC
computes is accurate, you must be miscomputing the alignment.

-Eli

I am writing CGRecordLayoutBuilder for Microsoft ABI and I have a problem.
I have the following code for testing test code - Pastebin.com .

When generated layout for a class C, I have break assert in
CGRecordLayoutBuilder.cpp line 968
assert(TypeSizeInBits == getTargetData().getTypeAllocSizeInBits(Ty)&&
           "Type size mismatch!");.
During debugging, I discovered that TypeSizeInBits smaller
getTargetData().getTypeAllocSizeInBits(Ty) 4 bytes.
After investigation, I discovered that the alignment is added in
StructLayout constructor (TargetData.cpp line 73)
// Add padding to the end of the struct so that it could be put in an array
   // and all array elements would be aligned correctly.
    if ((StructSize& (StructAlignment-1)) != 0)
      StructSize = TargetData::RoundUpAlignment(StructSize, StructAlignment);

But this alignment is not necessary for class C.
MSVC doesn't add to the end of the class alignment, if it has a virtual
base classes.
I see two solutions to this problem:
   1. Rewrite assert like this
       (pseudocode)
       if ABI == Microsoft&& Class has virtual basses
         assert(TypeSizeInBits ==
(getTargetData().getTypeAllocSizeInBits(Ty) - 32)&&
                      "Type size mismatch!");
        else
          old version;
    2. Provide to StructLyout information about ABI.

I think the first way is more simple.

No. A *lot* of downstream code will be broken if LLVM doesn't lay
out the IR type the way we think it's laid out, and that includes the
presence or absence of tail padding.

I don't see how this is possible, though. Can you run through an
example? What's the size and layout of class B here?

class A {
    char c;
};

class B : public virtual A {
    void *p;
};

I would expect that sizeof(B) is 12, and that it's laid out like this:
    [0-3] virtual base pointer
    [4-7] B.p
      [8] A.c
   [9-11] tail padding

MSVS with pragma pack 8 generates such layout:
1> class A size(1):
1> +---
1> 0 | c
1> +---

1> class B size(9):
1> +---
1> 0 | {vbptr}
1> 4 | p
1> +---
1> +--- (virtual base A)
1> 8 | c
1> +---

My code generates same layout (i32* in B is vbtable pointer):
*** Dumping AST Record Layout
    0 | class A
    0 | char c
   sizeof=1, dsize=1, align=1
   nvsize=1, nvalign=1

LLVMType:%class.A = type { i8 }
NonVirtualBaseLLVMType:%class.A = type { i8 }
IsZeroInitializable:1

*** Dumping AST Record Layout
    0 | class B
    0 | (B vtable pointer)
    0 | (B vbtable pointer)
    4 | void * p
    8 | class A (virtual base)
    8 | char c
   sizeof=9, dsize=9, align=4
   nvsize=8, nvalign=4

That's can't be what MSVC actually does: the size of B must be a
multiple of its alignment.

I check this layout twice. I get this layout from MSVC compiler output. And I check it in a memory while debugging.

  If you're representation of what MSVC
computes is accurate, you must be miscomputing the alignment.

Can you clarify?
  - Dmitry.

What is the value of (long)(((B*)0)+1)? From what you are saying, it
should be 9; in that case, the "align=4" from your dump is not
correct.

-Eli

I am writing CGRecordLayoutBuilder for Microsoft ABI and I have a
problem.
I have the following code for testing test code - Pastebin.com .

When generated layout for a class C, I have break assert in
CGRecordLayoutBuilder.cpp line 968
assert(TypeSizeInBits == getTargetData().getTypeAllocSizeInBits(Ty)&&
           "Type size mismatch!");.
During debugging, I discovered that TypeSizeInBits smaller
getTargetData().getTypeAllocSizeInBits(Ty) 4 bytes.
After investigation, I discovered that the alignment is added in
StructLayout constructor (TargetData.cpp line 73)
// Add padding to the end of the struct so that it could be put in an
array
   // and all array elements would be aligned correctly.
    if ((StructSize& (StructAlignment-1)) != 0)
      StructSize = TargetData::RoundUpAlignment(StructSize,
StructAlignment);

But this alignment is not necessary for class C.
MSVC doesn't add to the end of the class alignment, if it has a virtual
base classes.
I see two solutions to this problem:
   1. Rewrite assert like this
       (pseudocode)
       if ABI == Microsoft&& Class has virtual basses
         assert(TypeSizeInBits ==
(getTargetData().getTypeAllocSizeInBits(Ty) - 32)&&
                      "Type size mismatch!");
        else
          old version;
    2. Provide to StructLyout information about ABI.

I think the first way is more simple.

No. A *lot* of downstream code will be broken if LLVM doesn't lay
out the IR type the way we think it's laid out, and that includes the
presence or absence of tail padding.

I don't see how this is possible, though. Can you run through an
example? What's the size and layout of class B here?

class A {
    char c;
};

class B : public virtual A {
    void *p;
};

I would expect that sizeof(B) is 12, and that it's laid out like this:
    [0-3] virtual base pointer
    [4-7] B.p
      [8] A.c
   [9-11] tail padding

MSVS with pragma pack 8 generates such layout:
1> class A size(1):
1> +---
1> 0 | c
1> +---

1> class B size(9):
1> +---
1> 0 | {vbptr}
1> 4 | p
1> +---
1> +--- (virtual base A)
1> 8 | c
1> +---

My code generates same layout (i32* in B is vbtable pointer):
*** Dumping AST Record Layout
    0 | class A
    0 | char c
   sizeof=1, dsize=1, align=1
   nvsize=1, nvalign=1

LLVMType:%class.A = type { i8 }
NonVirtualBaseLLVMType:%class.A = type { i8 }
IsZeroInitializable:1

*** Dumping AST Record Layout
    0 | class B
    0 | (B vtable pointer)
    0 | (B vbtable pointer)
    4 | void * p
    8 | class A (virtual base)
    8 | char c
   sizeof=9, dsize=9, align=4
   nvsize=8, nvalign=4

That's can't be what MSVC actually does: the size of B must be a
multiple of its alignment.

I check this layout twice. I get this layout from MSVC compiler output. And
I check it in a memory while debugging.

  If you're representation of what MSVC
computes is accurate, you must be miscomputing the alignment.

Can you clarify?

What is the value of (long)(((B*)0)+1)? From what you are saying, it
should be 9;

Yes, that right.

  in that case, the "align=4" from your dump is not
correct.

How I can modify align from command line options?

- Dmitry.

Err, what? The align=4 is getting computed by the RecordLayoutBuilder.

-Eli

Oh sorry, it`s really stupid question :slight_smile:

  - Dmitry.

I suppose that align must be 8, am I right?

- Dmitry.

I really don't understand why align=4 isn't correct.
I always thought that #pragma pack only affects members of the structure.
And __declspec(align(n)) specifies the alignment for objects of type.
In MSVS align for B is 4 and for A is 1.

The only answer I can come up with is the model MSVC uses is simply insane.

One more attempt to get some sanity: does the following compile with
MSVC? If so, what is the output?

#include <cstdio>
class A {
char c;
};
class B : public virtual A {
public:
void *p;
};
B* x = new B[10];
int main() { std::printf("%d\n", (int)(x[1].p)); }

-Eli

VS 2010, Win7, 32-bit build:
Debug mode prints: -842150451
Release mode prints: 230338032

Seriously.

~Aaron

it compiles and the output is 0.
not sure what you are trying to test here.

Err, I meant to write the following:
int main() { std::printf("%d\n", (int)&(x[1].p)); }

-Eli

I’ll guess that Eli meant to print &x[1].p (to see whether the void* pointer was aligned or not), perhaps. I modified the program to print:

std::printf(“%d\n”, (int)(&x[0].p));
std::printf(“%d\n”, (int)(&x[1].p));

& the result I got (in debug) was:

1077836
1077845

x86:
Debug: 1395565
Release: 1715501

x64:
Debug:3242496
Release: 1320128

~Aaron

Right... so the alignment is in fact 1. So to be compliant with the
C++ standard on x86-32 Windows, we can't assume any alignment on loads
and stores. Fun.

-Eli

Without a specify __declspec(align(#)) is true.
So, what approach is the best to resolve this problem?
Patch StructLayout or smth else?

  - Dmitry.

Just fixing the RecordLayoutBuilder to set the alignment correctly
should be enough to start with. It has wider implications, though,
which might be a bit tricky to model: it means that we can't assume
*any* struct is actually aligned.

-Eli