From 9124a4b7f4e310c9f4aa305f42ce48fd352eaaf3 Mon Sep 17 00:00:00 2001 From: Yunzhong Gao Date: Wed, 29 Jan 2014 00:09:16 +0000 Subject: [PATCH] Fixing PR18430 by checking that the size of bitfields plus padding does not grow into the following virtual base. Differential Revision: http://llvm-reviews.chandlerc.com/D2560 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@200359 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/CGRecordLayoutBuilder.cpp | 7 +++- test/CodeGenCXX/bitfield.cpp | 52 +++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/lib/CodeGen/CGRecordLayoutBuilder.cpp b/lib/CodeGen/CGRecordLayoutBuilder.cpp index a1cc2aec50..3c1e7431eb 100644 --- a/lib/CodeGen/CGRecordLayoutBuilder.cpp +++ b/lib/CodeGen/CGRecordLayoutBuilder.cpp @@ -344,7 +344,12 @@ bool CGRecordLayoutBuilder::LayoutBitfields(const ASTRecordLayout &Layout, // maximum latitude the language provides, and rely on the backend to lower // these in conjunction with shifts and masks to narrower operations where // beneficial. - uint64_t EndOffset = Types.getContext().toBits(Layout.getDataSize()); + uint64_t EndOffset; + if (Types.getContext().getLangOpts().CPlusPlus) + // Do not grow the bitfield storage into the following virtual base. + EndOffset = Types.getContext().toBits(Layout.getNonVirtualSize()); + else + EndOffset = Types.getContext().toBits(Layout.getDataSize()); if (BFE != FE) // If there are more fields to be laid out, the offset at the end of the // bitfield is the offset of the next field in the record. diff --git a/test/CodeGenCXX/bitfield.cpp b/test/CodeGenCXX/bitfield.cpp index 2c454b0c07..820af16b1c 100644 --- a/test/CodeGenCXX/bitfield.cpp +++ b/test/CodeGenCXX/bitfield.cpp @@ -426,3 +426,55 @@ namespace N6 { s->b2 = x; } } + +namespace N7 { + // Similar to N4 except that this adds a virtual base to the picture. (PR18430) + // Do NOT widen loads and stores to bitfields into padding at the end of + // a class which might end up with members inside of it when inside a derived + // class. + struct B1 { + virtual void f(); + unsigned b1 : 24; + }; + struct B2 : virtual B1 { + virtual ~B2(); + unsigned b : 24; + }; + // Imagine some other translation unit introduces: +#if 0 + struct Derived : public B2 { + char c; + }; +#endif + unsigned read(B2* s) { + // FIXME: We should widen this load as long as the function isn't being + // instrumented by thread-sanitizer. + // + // CHECK-X86-64-LABEL: define i32 @_ZN2N74read + // CHECK-X86-64: %[[gep:.*]] = getelementptr inbounds {{.*}}* %{{.*}}, i32 0, i32 1 + // CHECK-X86-64: %[[ptr:.*]] = bitcast [3 x i8]* %[[gep]] to i24* + // CHECK-X86-64: %[[val:.*]] = load i24* %[[ptr]] + // CHECK-X86-64: %[[ext:.*]] = zext i24 %[[val]] to i32 + // CHECK-X86-64: ret i32 %[[ext]] + // CHECK-PPC64-LABEL: define zeroext i32 @_ZN2N74read + // CHECK-PPC64: %[[gep:.*]] = getelementptr inbounds {{.*}}* %{{.*}}, i32 0, i32 1 + // CHECK-PPC64: %[[ptr:.*]] = bitcast [3 x i8]* %[[gep]] to i24* + // CHECK-PPC64: %[[val:.*]] = load i24* %[[ptr]] + // CHECK-PPC64: %[[ext:.*]] = zext i24 %[[val]] to i32 + // CHECK-PPC64: ret i32 %[[ext]] + return s->b; + } + void write(B2* s, unsigned x) { + // CHECK-X86-64-LABEL: define void @_ZN2N75write + // CHECK-X86-64: %[[gep:.*]] = getelementptr inbounds {{.*}}* %{{.*}}, i32 0, i32 1 + // CHECK-X86-64: %[[ptr:.*]] = bitcast [3 x i8]* %[[gep]] to i24* + // CHECK-X86-64: %[[new:.*]] = trunc i32 %{{.*}} to i24 + // CHECK-X86-64: store i24 %[[new]], i24* %[[ptr]] + // CHECK-PPC64-LABEL: define void @_ZN2N75write + // CHECK-PPC64: %[[gep:.*]] = getelementptr inbounds {{.*}}* %{{.*}}, i32 0, i32 1 + // CHECK-PPC64: %[[ptr:.*]] = bitcast [3 x i8]* %[[gep]] to i24* + // CHECK-PPC64: %[[new:.*]] = trunc i32 %{{.*}} to i24 + // CHECK-PPC64: store i24 %[[new]], i24* %[[ptr]] + s->b = x; + } +} -- 2.40.0