[PATCH] Fixing a struct layout bug involving a packed derived class with an unpacked non-virtual base

[Yunzhong Gao]

ygao added you to the CC list for the revision "Fixing a struct layout bug involving a packed derived class with an unpacked non-virtual base".

Hi,

The following patch fixes PR18510 by checking whether the non-virtual base of the derived class might
have a smaller size as compared to the stand-alone type of the base class. This is possible when the
derived class is packed and hence might have smaller alignment requirement than the base class. Without
this fix, clang would hit an assertion failure for the following test case:

```
/* test.cpp */
struct ClassName0 {
  char : 3;
  virtual void ClassName0Method() {}
};
struct ClassName7 : public ClassName0
{ };

#pragma pack(push, 2)
struct ClassName9 : public ClassName7, public virtual ClassName0
{ };
#pragma pack(pop)

ClassName9 x;
/* end test.cpp */
```

When the derived class is packed, CGRecordLayoutBuilder::Packed is set to true. As a result,
getTypeAlignment(baseLayout.getBaseSubobjectLLVMType()) returns one and changes in r146443
do not catch this bug.

- Gao

http://llvm-reviews.chandlerc.com/D2599

Files:
  test/CodeGenCXX/pragma-pack-3.cpp
  lib/CodeGen/CGRecordLayoutBuilder.cpp

Index: test/CodeGenCXX/pragma-pack-3.cpp
===================================================================
--- test/CodeGenCXX/pragma-pack-3.cpp
+++ test/CodeGenCXX/pragma-pack-3.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -triple=i686-apple-darwin10 -emit-llvm -o - | FileCheck %s
+
+struct Base {
+  char a;
+};
+
+struct Derived_1 : virtual Base
+{
+  char b;
+};
+
+#pragma pack(1)
+struct Derived_2 : Derived_1 {
+  // CHECK: %struct.Derived_2 = type <{ [5 x i8], %struct.Base }>
+};
+
+Derived_2 x;
Index: lib/CodeGen/CGRecordLayoutBuilder.cpp
===================================================================
--- lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -558,7 +558,12 @@
   if (getTypeAlignment(subobjectType) > Alignment)
     return false;
 
-  AppendField(baseOffset, subobjectType);
+  if (LastLaidOutBase.NonVirtualSize < CharUnits::fromQuantity(
+      Types.getDataLayout().getStructLayout(subobjectType)->getSizeInBytes()))
+    AppendBytes(LastLaidOutBase.NonVirtualSize);
+  else
+    AppendField(baseOffset, subobjectType);
+
   return true;
 }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2599.2.patch
Type: text/x-patch
Size: 1112 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140123/5e81623c/attachment.bin>