[clang] e6cb4b6 - [Clang][CodeGen] Fixing mismatch between memory layout and const expressions for oversized bitfields

[Lucas Prates via cfe-commits]

Author: Lucas Prates
Date: 2020-04-02T11:55:20+01:00
New Revision: e6cb4b659af9f9c1a4c179093b187e7ad7cc5770

URL: https://github.com/llvm/llvm-project/commit/e6cb4b659af9f9c1a4c179093b187e7ad7cc5770
DIFF: https://github.com/llvm/llvm-project/commit/e6cb4b659af9f9c1a4c179093b187e7ad7cc5770.diff

LOG: [Clang][CodeGen] Fixing mismatch between memory layout and const expressions for oversized bitfields

Summary:
The construction of constants for structs/unions was conflicting the
expected memory layout for over-sized bit-fields. When building the
necessary bits for those fields, clang was ignoring the size information
computed for the struct/union memory layout and using the original data
from the AST's FieldDecl information. This caused an issue in big-endian
targets, where the field's contant was incorrectly misplaced due to
endian calculations.

This patch aims to separate the constant value from the necessary
padding bits, using the proper size information for each one of them.
With this, the layout of constants for over-sized bit-fields matches the
ABI requirements.

Reviewers: rsmith, eli.friedman, efriedma

Reviewed By: efriedma

Subscribers: efriedma, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D77048

Added: 
    

Modified: 
    clang/lib/CodeGen/CGExprConstant.cpp
    clang/test/CodeGenCXX/bitfield-layout.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp
index e17c1c5f7ac4..da5d778a4922 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -589,19 +589,21 @@ bool ConstStructBuilder::AppendBytes(CharUnits FieldOffsetInChars,
 bool ConstStructBuilder::AppendBitField(
     const FieldDecl *Field, uint64_t FieldOffset, llvm::ConstantInt *CI,
     bool AllowOverwrite) {
-  uint64_t FieldSize = Field->getBitWidthValue(CGM.getContext());
+  const CGRecordLayout &RL =
+      CGM.getTypes().getCGRecordLayout(Field->getParent());
+  const CGBitFieldInfo &Info = RL.getBitFieldInfo(Field);
   llvm::APInt FieldValue = CI->getValue();
 
   // Promote the size of FieldValue if necessary
   // FIXME: This should never occur, but currently it can because initializer
   // constants are cast to bool, and because clang is not enforcing bitfield
   // width limits.
-  if (FieldSize > FieldValue.getBitWidth())
-    FieldValue = FieldValue.zext(FieldSize);
+  if (Info.Size > FieldValue.getBitWidth())
+    FieldValue = FieldValue.zext(Info.Size);
 
   // Truncate the size of FieldValue to the bit field size.
-  if (FieldSize < FieldValue.getBitWidth())
-    FieldValue = FieldValue.trunc(FieldSize);
+  if (Info.Size < FieldValue.getBitWidth())
+    FieldValue = FieldValue.trunc(Info.Size);
 
   return Builder.addBits(FieldValue,
                          CGM.getContext().toBits(StartOffset) + FieldOffset,

diff  --git a/clang/test/CodeGenCXX/bitfield-layout.cpp b/clang/test/CodeGenCXX/bitfield-layout.cpp
index 46f41111a1de..d8f8c87eb28b 100644
--- a/clang/test/CodeGenCXX/bitfield-layout.cpp
+++ b/clang/test/CodeGenCXX/bitfield-layout.cpp
@@ -1,11 +1,14 @@
-// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -o - -O3 | FileCheck -check-prefix CHECK-LP64 %s
-// RUN: %clang_cc1 %s -triple=i386-apple-darwin10 -emit-llvm -o - -O3 | FileCheck -check-prefix CHECK-LP32 %s
+// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -o - -O3 | FileCheck -check-prefix=CHECK-LP64 -check-prefix=CHECK %s
+// RUN: %clang_cc1 %s -triple=i386-apple-darwin10 -emit-llvm -o - -O3 | FileCheck -check-prefix CHECK-LP32 -check-prefix=CHECK %s
+// RUN: %clang_cc1 %s -triple=aarch64_be-none-eabi -emit-llvm -o - -O3 | FileCheck -check-prefix CHECK-A64BE -check-prefix=CHECK %s
+// RUN: %clang_cc1 %s -triple=thumbv7_be-none-eabi -emit-llvm -o - -O3 | FileCheck -check-prefix CHECK-A32BE -check-prefix=CHECK %s
 
 // CHECK-LP64: %union.Test1 = type { i32, [4 x i8] }
 union Test1 {
   int a;
   int b: 39;
-} t1;
+};
+Test1 t1;
 
 // CHECK-LP64: %union.Test2 = type { i8 }
 union Test2 {
@@ -17,10 +20,16 @@ union Test3 {
   int : 9;
 } t3;
 
+// CHECK: %union.Test4 = type { i8, i8 }
+union Test4 {
+  char val : 16;
+};
+Test4 t4;
 
 #define CHECK(x) if (!(x)) return __LINE__
 
-int f() {
+// CHECK: define i32 @_Z11test_assignv()
+int test_assign() {
   struct {
     int a;
 
@@ -37,7 +46,41 @@ int f() {
   CHECK(c.b == (unsigned long long)-1);
   CHECK(c.c == 0);
 
-// CHECK-LP64: ret i32 0
-// CHECK-LP32: ret i32 0
+  Test1 u1;
+  Test4 u2;
+
+  u1.b = 1;
+  u2.val = 42;
+
+  CHECK(u1.b == 1);
+  CHECK(u2.val == 42);
+
+  // CHECK: ret i32 0
+  return 0;
+}
+
+// CHECK: define i32 @_Z9test_initv()
+int test_init() {
+  struct S {
+    int a;
+
+    unsigned long long b : 65;
+
+    int c;
+  };
+  S s1 = {1, 42, 0};
+
+  CHECK(s1.a == 1);
+  CHECK(s1.b == (unsigned long long)42);
+  CHECK(s1.c == 0);
+
+  Test1 u1 = {1};
+  Test4 u2 = {42};
+
+  CHECK(u1.a == 1);
+  CHECK(u1.b == 1);
+  CHECK(u2.val == 42);
+
+  // CHECK: ret i32 0
   return 0;
 }