[Clang][Codegen] Truncate initializers of union bitfield members

[tmatheson]

Summary

If an initial value is given for a bitfield that does not fit in the
bitfield, the value should be truncated. Constant folding for
expressions did not account for this truncation in the case of union
member functions, despite a warning being emitted. In some contexts,
evaluation of expressions was not enabled unless C++11, ROPI or RWPI
was enabled.

[tmatheson]

Rebased

[tmatheson]

Ping

[joechrisellis]

clang/lib/AST/ExprConstant.cpp

9803–9804

nit: I would prefer this:

if (Field->isBitField() && truncateBitfieldValue(Info, InitExpr, Result.getUnionValue(), Field))
    return true;

return EvaluateInPlace(Result.getUnionValue(), Info, Subobject, InitExpr);

It feels more in-line with the rest of the function. But it is okay if you want to ignore this too. 😄

clang/test/CodeGenCXX/bitfield-layout.cpp

88–95

I'd like to see some more tests that check the truncation behaviour. My understanding is that this is trucating to -1 because of two's complement? How about something like:

int test_trunc() {
    union {
        int i : 4;
    } U = {80};
    return U.i;
    // CHECK: ret i32 0
}

Am I understanding the behaviour correctly?

Some comments about what is actually happening on the bit-level to get this result would also be nice.

[rsmith]

clang/lib/AST/ExprConstant.cpp

9801–9804

Shouldn't this be && not ||? These functions return true if they succeed (unlike the convention in Sema where true means an error diagnostic was produced).

[tmatheson]

Added more test cases and explanatory comments

[tmatheson]

Add test RUN line that checks C++11 behaviour

[tmatheson]

Make unions in test cases const, and clarify logic

[tmatheson]

clang/lib/AST/ExprConstant.cpp

9801–9804

You are correct. I have updated the logic to be simpler to follow as well.

clang/test/CodeGenCXX/bitfield-layout.cpp

88–95

Yes that is correct. I have added some new test cases and comments to make it clear what they are testing for.

[tmatheson]

Ping

[pratlucas]

The truncate conditions look a lot better and the test coverage seems reasonable now.
LGTM.

clang/lib/AST/ExprConstant.cpp

if (RD->isUnion()) {
     LValue Subobject = This;
     if (!HandleLValueMember(Info, InitExpr, Subobject, Field, &Layout))
       return false;
 
     // Temporarily override This, in case there's a CXXDefaultInitExpr in here.
     ThisOverrideRAII ThisOverride(*Info.CurrentCall, &This,
                                   isa<CXXDefaultInitExpr>(InitExpr));
 
-    return EvaluateInPlace(Result.getUnionValue(), Info, Subobject, InitExpr);
+    return EvaluateInPlace(Result.getUnionValue(), Info, Subobject, InitExpr) ||
+           (Field->isBitField() &&
+            truncateBitfieldValue(Info, InitExpr, Result.getUnionValue(),
+                                  Field));
   }
 
   if (!Result.hasValue())
     Result = APValue(APValue::UninitStruct(), CXXRD ? CXXRD->getNumBases() : 0,
                      std::distance(RD->field_begin(), RD->field_end()));
   unsigned ElementNo = 0;
   bool Success = true;
 

clang/test/CodeGenCXX/bitfield-layout.cpp

int test_init() {
 
   CHECK(u1.a == 1);
   CHECK(u1.b == 1);
   CHECK(u2.val == 42);
 
   // CHECK: ret i32 0
   return 0;
 }
+
+// CHECK: define i32 @_Z10test_truncv()
+int test_trunc() {
+  union {
+    int i : 4;
+  } U = {15};
+  return U.i;
+  // CHECK: ret i32 -1
+}