From e6cb4b659af9f9c1a4c179093b187e7ad7cc5770 Mon Sep 17 00:00:00 2001
From: Lucas Prates <lucas.prates@arm.com>
Date: Fri, 27 Mar 2020 11:53:49 +0000
Subject: [PATCH] [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
---
 clang/lib/CodeGen/CGExprConstant.cpp      | 12 ++++---
 clang/test/CodeGenCXX/bitfield-layout.cpp | 55 +++++++++++++++++++++++++++----
 2 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp
index e17c1c5f..da5d778 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 46f4111..d8f8c87 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;
 }
-- 
2.7.4