Diagnose union tail padding

[jfb]

Summary

From a discussion in D68115, it seems like union tail padding is often surprising to folks, compiler implementors included. This new diagnostic, -Wunion-tail-padding, emits a diagnostic when a union's tail is left undefined when develoeprs might have reasonably expected that it would not be. I've included -Wunion-tail-padding under the umbrella of a new diagnostic group, -Wpadding, since it often seems like padding in general is a source of pain. In particular, padding can leak secret information to malicious attackers. Here we focus on the places where padding is *unitialized*, not where a structure or union has padding.

[jfb]

clang/test/CodeGen/union-tail-padding.c

9

This test, while it might be surprising, is testing the current behavior of clang. I haven't validated that it's correct for each standard.

[dexonsmith]

clang/test/CodeGen/union-tail-padding.c

28–36

Are these warnings actionable? What should users do when they see this warning?

[jfb]

clang/test/CodeGen/union-tail-padding.c

28–36

Good point!

I was thinking about this, and was wondering if I should add a fixit which suggests using the first wider member of the union. The problem is to offer the same object representation... that's tricky on its own (there isn't always an exact match), and then we have to deal with type punning (in C++, not in C).

So I'd love ideas, because I'm not sure what to do. That being said, I wrote this partly because D68115 was surprising to folks, and partly because developers would like to opt-in to this diagnostic to find places where initialization isn't doing what they think.

Maybe instead we should suggest to leave uninitialized, and use an assignment, or memset?

[jfb]

clang/include/clang/Basic/DiagnosticGroups.td

825

I'd like to hear what other diagnostics folks think should fit under the Padding group. I've focused not on declarations which have padding, but rather on initializations which leave uninitialized padding behind. It seems like internal padding for structs and between array elements would be desirable.

[jfb]

clang/include/clang/Basic/DiagnosticFrontendKinds.td

264

Something which I'm not sure matters:

union Unnamed {
  union {
    int i;
    float f;
  };
  char bytes[8];
};

union Unnamed unnamed = { 42 };

Will generate the following diagnostic:

warning: Initializing union 'Unnamed' field '' only initializes the first 4 of 8 bytes, leaving the remaining 4 bytes undefined [-Wunion-tail-padding]
union Unnamed unnamed = { 42 };
                        ^

I think that's generally how we handle unnamed members anyways?

[dexonsmith]

clang/test/CodeGen/union-tail-padding.c

28–36

Maybe instead we should suggest to leave uninitialized, and use an assignment, or memset?

It's not clear to me that those are better. For example, memset doesn't seem right for non-PODs in C++. I'm not sure what to suggest though.

[jfb]

clang/test/CodeGen/union-tail-padding.c

28–36

I'd argue that non-PODs in unions don't seem right either ;-)
It would be easy to diagnose non-trivially-copyable types differently in this circumstance.

[dexonsmith]

clang/test/CodeGen/union-tail-padding.c

28–36

It would be easy to diagnose non-trivially-copyable types differently in this circumstance.

Good point.

[rsmith]

I'm not convinced that this is an especially useful diagnostic (much like with -Wpadded), but I'm also not opposed if you are aware of cases where it would be used.

It seems worth pointing out that, at least in C, merely assigning to a union member results in all padding becoming unspecified (6.2.6.1/7). So the suggestion to memset the union to zero and then assign to a specific union member does not work.

clang/include/clang/Basic/DiagnosticFrontendKinds.td

264

Please start diagnostic messages with a lowercase letter.

In the case you identify, it'd be nice to say "initializing union Unnamed field i only initializes [...]". (We should probably special-case formatting a declaration with an empty name and print out something like '<unnamed>' instead, but that's not specific to this diagnostic.)

Also, I think this would read more naturally as "initializing field i of union Unnamed only initializes [...]".

clang/include/clang/Basic/DiagnosticGroups.td

825

It seems to me that including this in the existing -Wpadded warning group would be reasonable (I prefer the name -Wpadding over -Wpadded for what it's worth, but it seems problematic to include both with different meanings.)

-Wpadded currently warns whenever struct layout inserts interior or tail padding. Also warning on union fields that, when active, would result in tail padding seems reasonable to me too.

(The new warning seems like something that would be hard to enable on any sizeable codebase, or with any headers that aren't under the end-user's control, which I find somewhat concerning. But the old -Wpadded warning has the same issues, so this seems like it's not making things worse.)

clang/lib/CodeGen/CGExprConstant.cpp

608–612

I don't think it's acceptable to produce a warning such as this from IR generation, and especially not from here -- which we may or may not reach depending on a diverse set of factors (in particular, whether the initializer happens to be a constant).

I think either you should warn from record layout, like we do for -Wpadded, or from SemaInit (if you only want this to apply if the narrower field is actually initialized).

clang/test/CodeGen/union-tail-padding.c

28–36

Regarding whether this is actionable, I think one useful test is: would it be reasonable for someone to want to promote this warning to an error? If the answer is no, then we should consider making this diagnostic be a remark instead of a warning.

[jfb]

In D80055#2042630, @rsmith wrote:

I'm not convinced that this is an especially useful diagnostic (much like with -Wpadded), but I'm also not opposed if you are aware of cases where it would be used.

I wrote it to help with issues a codebase was having adopting variable auto-init. They were surprised at what clang did, and wish they could have known. This is just one datapoint, and I agree that generally this isn't super useful.

It seems worth pointing out that, at least in C, merely assigning to a union member results in all padding becoming unspecified (6.2.6.1/7). So the suggestion to memset the union to zero and then assign to a specific union member does not work.

Indeed, this isn't something I'm trying to address here but it would be useful to do so.

I was wondering if any of the tests were surprising to you, or if the behavior described was as expected? It's certainly not tractable for most users of C and C++ (which might be fine, it shouldn't be relied on).

[rsmith]

In D80055#2055151, @jfb wrote:

I was wondering if any of the tests were surprising to you, or if the behavior described was as expected?

I've highlighted one case where the test expectation doesn't match the standard rules.

clang/test/CodeGen/union-tail-padding.c

42

This warning appears to be incorrect. Value-initialization of a union with a trivial default constructor performs zero-initialization, which does zero out padding bits.

[jfb]

I've gotten what I wanted out of this (diagnosed a particular codebase), and am not sure it's worth pursuing further. If anyone is interested, please let me know.

clang/include/clang/Basic/DiagnosticFrontendKinds.td

def warn_atomic_op_misaligned : Warning<
   "%select{large|misaligned}0 atomic operation may incur "
   "significant performance penalty">, InGroup<DiagGroup<"atomic-alignment">>;
 
 def warn_alias_with_section : Warning<
   "%select{alias|ifunc}1 will not be in section '%0' but in the same section "
   "as the %select{aliasee|resolver}2">,
   InGroup<IgnoredAttributes>;
 
+def warn_union_tail_padding_uninitialized : Warning<
+  "Initializing union %0 field %1 only initializes the first %2 of %3 bytes, leaving the remaining %4 bytes undefined">,
+  InGroup<UnionTailPadding>;
+
 let CategoryName = "Instrumentation Issue" in {
 def warn_profile_data_out_of_date : Warning<
   "profile data may be out of date: of %0 function%s0, %1 %plural{1:has|:have}1"
   " mismatched data that will be ignored">,
   InGroup<ProfileInstrOutOfDate>;
 def warn_profile_data_missing : Warning<
   "profile data may be incomplete: of %0 function%s0, %1 %plural{1:has|:have}1"
   " no data">,

clang/include/clang/Basic/DiagnosticGroups.td

def Unused : DiagGroup<"unused",
                         // UnusedParameter, (matches GCC's behavior)
                         // UnusedTemplate, (clean-up libc++ before enabling)
                         // UnusedMemberFunction, (clean-up llvm before enabling)
                         UnusedPrivateField, UnusedLambdaCapture,
                         UnusedLocalTypedef, UnusedValue, UnusedVariable,
                         UnusedPropertyIvar]>,
                         DiagCategory<"Unused Entity Issue">;
 
+def UnionTailPadding : DiagGroup<"union-tail-padding">;
+def Padding : DiagGroup<"padding", [UnionTailPadding]>,
+              DiagCategory<"Padding Issue">;
+
 // Format settings.
 def FormatInvalidSpecifier : DiagGroup<"format-invalid-specifier">;
 def FormatSecurity : DiagGroup<"format-security">;
 def FormatNonStandard : DiagGroup<"format-non-iso">;
 def FormatY2K : DiagGroup<"format-y2k">;
 def FormatPedantic : DiagGroup<"format-pedantic">;
 def FormatTypeConfusion : DiagGroup<"format-type-confusion">;
 def Format : DiagGroup<"format",

clang/lib/CodeGen/CGExprConstant.cpp

 #include "ConstantEmitter.h"
 #include "TargetInfo.h"
 #include "clang/AST/APValue.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/Builtins.h"
+#include "clang/Frontend/FrontendDiagnostic.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Sequence.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalVariable.h"
 using namespace clang;
 using namespace CodeGen;
struct ConstantAggregateBuilderUtils {
   llvm::Constant *getZeroes(CharUnits ZeroSize) const {
     llvm::Type *Ty = llvm::ArrayType::get(CGM.Int8Ty, ZeroSize.getQuantity());
     return llvm::ConstantAggregateZero::get(Ty);
   }
 };
 
 /// Incremental builder for an llvm::Constant* holding a struct or array
 /// constant.
-class ConstantAggregateBuilder : private ConstantAggregateBuilderUtils {
+class ConstantAggregateBuilder : public ConstantAggregateBuilderUtils {
   /// The elements of the constant. These two arrays must have the same size;
   /// Offsets[i] describes the offset of Elems[i] within the constant. The
   /// elements are kept in increasing offset order, and we ensure that there
   /// is no overlap: Offsets[i+1] >= Offsets[i] + getSize(Elemes[i]).
   ///
   /// This may contain explicit padding elements (in order to create a
   /// natural layout), but need not. Gaps between elements are implicitly
   /// considered to be filled with undef.
class ConstStructBuilder {
   ConstantEmitter &Emitter;
   ConstantAggregateBuilder &Builder;
   CharUnits StartOffset;
 
 public:
   static llvm::Constant *BuildStruct(ConstantEmitter &Emitter,
                                      InitListExpr *ILE, QualType StructTy);
   static llvm::Constant *BuildStruct(ConstantEmitter &Emitter,
+                                     const SourceLocation loc,
                                      const APValue &Value, QualType ValTy);
   static bool UpdateStruct(ConstantEmitter &Emitter,
                            ConstantAggregateBuilder &Const, CharUnits Offset,
                            InitListExpr *Updater);
 
 private:
   ConstStructBuilder(ConstantEmitter &Emitter,
                      ConstantAggregateBuilder &Builder, CharUnits StartOffset)
       : CGM(Emitter.CGM), Emitter(Emitter), Builder(Builder),
         StartOffset(StartOffset) {}
 
   bool AppendField(const FieldDecl *Field, uint64_t FieldOffset,
-                   llvm::Constant *InitExpr, bool AllowOverwrite = false);
+                   llvm::Constant *InitExpr, bool AllowOverwrite,
+                   const SourceLocation loc);
 
   bool AppendBytes(CharUnits FieldOffsetInChars, llvm::Constant *InitCst,
                    bool AllowOverwrite = false);
 
   bool AppendBitField(const FieldDecl *Field, uint64_t FieldOffset,
                       llvm::ConstantInt *InitExpr, bool AllowOverwrite = false);
 
   bool Build(InitListExpr *ILE, bool AllowOverwrite);
-  bool Build(const APValue &Val, const RecordDecl *RD, bool IsPrimaryBase,
-             const CXXRecordDecl *VTableClass, CharUnits BaseOffset);
+  bool Build(const SourceLocation loc, const APValue &Val, const RecordDecl *RD,
+             bool IsPrimaryBase, const CXXRecordDecl *VTableClass,
+             CharUnits BaseOffset);
   llvm::Constant *Finalize(QualType Ty);
 };
 
-bool ConstStructBuilder::AppendField(
-    const FieldDecl *Field, uint64_t FieldOffset, llvm::Constant *InitCst,
-    bool AllowOverwrite) {
+bool ConstStructBuilder::AppendField(const FieldDecl *Field,
+                                     uint64_t FieldOffset,
+                                     llvm::Constant *InitCst,
+                                     bool AllowOverwrite,
+                                     const SourceLocation loc) {
   const ASTContext &Context = CGM.getContext();
 
   CharUnits FieldOffsetInChars = Context.toCharUnitsFromBits(FieldOffset);
 
-  return AppendBytes(FieldOffsetInChars, InitCst, AllowOverwrite);
+  if (!AppendBytes(FieldOffsetInChars, InitCst, AllowOverwrite))
+    return false;
+
+  if (!Field->getParent()->isUnion())
+    return true;
+
+  const ASTRecordLayout &Layout =
+      CGM.getContext().getASTRecordLayout(Field->getParent());
+  CharUnits UnionSize = Layout.getSize();
+  CharUnits InitSize = Builder.getSize(InitCst);
+  if (UnionSize <= InitSize)
+    return true;
+
+  // The tail padding of the union isn't being initialized.
+  CGM.getDiags().Report(loc, diag::warn_union_tail_padding_uninitialized)
+      << Field->getParent() << Field << (unsigned)InitSize.getQuantity()
+      << (unsigned)UnionSize.getQuantity()
+      << (unsigned)(UnionSize - InitSize).getQuantity();
+
+  return true;
 }
 
 bool ConstStructBuilder::AppendBytes(CharUnits FieldOffsetInChars,
                                      llvm::Constant *InitCst,
                                      bool AllowOverwrite) {
   return Builder.add(InitCst, StartOffset + FieldOffsetInChars, AllowOverwrite);
 }
 
llvm::Constant *EltInit =
         Init ? Emitter.tryEmitPrivateForMemory(Init, Field->getType())
              : Emitter.emitNullForMemory(Field->getType());
     if (!EltInit)
       return false;
 
     if (!Field->isBitField()) {
       // Handle non-bitfield members.
       if (!AppendField(Field, Layout.getFieldOffset(FieldNo), EltInit,
-                       AllowOverwrite))
+                       AllowOverwrite, ILE->getExprLoc()))
         return false;
       // After emitting a non-empty field with [[no_unique_address]], we may
       // need to overwrite its tail padding.
       if (Field->hasAttr<NoUniqueAddressAttr>())
         AllowOverwrite = true;
     } else {
       // Otherwise we have a bitfield.
       if (auto *CI = dyn_cast<llvm::ConstantInt>(EltInit)) {
struct BaseInfo {
   const CXXRecordDecl *Decl;
   CharUnits Offset;
   unsigned Index;
 
   bool operator<(const BaseInfo &O) const { return Offset < O.Offset; }
 };
 }
 
-bool ConstStructBuilder::Build(const APValue &Val, const RecordDecl *RD,
-                               bool IsPrimaryBase,
+bool ConstStructBuilder::Build(const SourceLocation loc, const APValue &Val,
+                               const RecordDecl *RD, bool IsPrimaryBase,
                                const CXXRecordDecl *VTableClass,
                                CharUnits Offset) {
   const ASTRecordLayout &Layout = CGM.getContext().getASTRecordLayout(RD);
 
   if (const CXXRecordDecl *CD = dyn_cast<CXXRecordDecl>(RD)) {
     // Add a vtable pointer, if we need one and it hasn't already been added.
     if (CD->isDynamicClass() && !IsPrimaryBase) {
       llvm::Constant *VTableAddressPoint =
for (CXXRecordDecl::base_class_const_iterator Base = CD->bases_begin(),
       Bases.push_back(BaseInfo(BD, BaseOffset, BaseNo));
     }
     llvm::stable_sort(Bases);
 
     for (unsigned I = 0, N = Bases.size(); I != N; ++I) {
       BaseInfo &Base = Bases[I];
 
       bool IsPrimaryBase = Layout.getPrimaryBase() == Base.Decl;
-      Build(Val.getStructBase(Base.Index), Base.Decl, IsPrimaryBase,
+      Build(loc, Val.getStructBase(Base.Index), Base.Decl, IsPrimaryBase,
             VTableClass, Offset + Base.Offset);
     }
   }
 
   unsigned FieldNo = 0;
   uint64_t OffsetBits = CGM.getContext().toBits(Offset);
 
   bool AllowOverwrite = false;
   for (RecordDecl::field_iterator Field = RD->field_begin(),
        FieldEnd = RD->field_end(); Field != FieldEnd; ++Field, ++FieldNo) {
     // If this is a union, skip all the fields that aren't being initialized.
     if (RD->isUnion() && !declaresSameEntity(Val.getUnionField(), *Field))
       continue;
 
     // Don't emit anonymous bitfields or zero-sized fields.
     if (Field->isUnnamedBitfield() || Field->isZeroSize(CGM.getContext()))
       continue;
 
     // Emit the value of the initializer.
     const APValue &FieldValue =
       RD->isUnion() ? Val.getUnionValue() : Val.getStructField(FieldNo);
     llvm::Constant *EltInit =
-      Emitter.tryEmitPrivateForMemory(FieldValue, Field->getType());
+        Emitter.tryEmitPrivateForMemory(loc, FieldValue, Field->getType());
     if (!EltInit)
       return false;
 
     if (!Field->isBitField()) {
       // Handle non-bitfield members.
       if (!AppendField(*Field, Layout.getFieldOffset(FieldNo) + OffsetBits,
-                       EltInit, AllowOverwrite))
+                       EltInit, AllowOverwrite, loc))
         return false;
       // After emitting a non-empty field with [[no_unique_address]], we may
       // need to overwrite its tail padding.
       if (Field->hasAttr<NoUniqueAddressAttr>())
         AllowOverwrite = true;
     } else {
       // Otherwise we have a bitfield.
       if (!AppendBitField(*Field, Layout.getFieldOffset(FieldNo) + OffsetBits,
llvm::Constant *ConstStructBuilder::BuildStruct(ConstantEmitter &Emitter,
 
   if (!Builder.Build(ILE, /*AllowOverwrite*/false))
     return nullptr;
 
   return Builder.Finalize(ValTy);
 }
 
 llvm::Constant *ConstStructBuilder::BuildStruct(ConstantEmitter &Emitter,
+                                                const SourceLocation loc,
                                                 const APValue &Val,
                                                 QualType ValTy) {
   ConstantAggregateBuilder Const(Emitter.CGM);
   ConstStructBuilder Builder(Emitter, Const, CharUnits::Zero());
 
   const RecordDecl *RD = ValTy->castAs<RecordType>()->getDecl();
   const CXXRecordDecl *CD = dyn_cast<CXXRecordDecl>(RD);
-  if (!Builder.Build(Val, RD, false, CD, CharUnits::Zero()))
+  if (!Builder.Build(loc, Val, RD, false, CD, CharUnits::Zero()))
     return nullptr;
 
   return Builder.Finalize(ValTy);
 }
 
 bool ConstStructBuilder::UpdateStruct(ConstantEmitter &Emitter,
                                       ConstantAggregateBuilder &Const,
                                       CharUnits Offset, InitListExpr *Updater) {

 
 llvm::Constant *
 ConstantEmitter::tryEmitAbstract(const Expr *E, QualType destType) {
   auto state = pushAbstract();
   auto C = tryEmitPrivate(E, destType);
   return validateAndPopAbstract(C, state);
 }
 
-llvm::Constant *
-ConstantEmitter::tryEmitAbstract(const APValue &value, QualType destType) {
+llvm::Constant *ConstantEmitter::tryEmitAbstract(const SourceLocation loc,
+                                                 const APValue &value,
+                                                 QualType destType) {
   auto state = pushAbstract();
-  auto C = tryEmitPrivate(value, destType);
+  auto C = tryEmitPrivate(loc, value, destType);
   return validateAndPopAbstract(C, state);
 }
 
 llvm::Constant *
 ConstantEmitter::emitAbstract(const Expr *E, QualType destType) {
   auto state = pushAbstract();
   auto C = tryEmitPrivate(E, destType);
   C = validateAndPopAbstract(C, state);
   if (!C) {
     CGM.Error(E->getExprLoc(),
               "internal error: could not emit constant value \"abstractly\"");
     C = CGM.EmitNullConstant(destType);
   }
   return C;
 }
 
 llvm::Constant *
 ConstantEmitter::emitAbstract(SourceLocation loc, const APValue &value,
                               QualType destType) {
   auto state = pushAbstract();
-  auto C = tryEmitPrivate(value, destType);
+  auto C = tryEmitPrivate(loc, value, destType);
   C = validateAndPopAbstract(C, state);
   if (!C) {
     CGM.Error(loc,
               "internal error: could not emit constant value \"abstractly\"");
     C = CGM.EmitNullConstant(destType);
   }
   return C;
 }
 
 llvm::Constant *ConstantEmitter::tryEmitForInitializer(const VarDecl &D) {
   initializeNonAbstract(D.getType().getAddressSpace());
   return markIfFailed(tryEmitPrivateForVarInit(D));
 }
 
 llvm::Constant *ConstantEmitter::tryEmitForInitializer(const Expr *E,
                                                        LangAS destAddrSpace,
                                                        QualType destType) {
   initializeNonAbstract(destAddrSpace);
   return markIfFailed(tryEmitPrivateForMemory(E, destType));
 }
 
-llvm::Constant *ConstantEmitter::emitForInitializer(const APValue &value,
+llvm::Constant *ConstantEmitter::emitForInitializer(const SourceLocation loc,
+                                                    const APValue &value,
                                                     LangAS destAddrSpace,
                                                     QualType destType) {
   initializeNonAbstract(destAddrSpace);
-  auto C = tryEmitPrivateForMemory(value, destType);
+  auto C = tryEmitPrivateForMemory(loc, value, destType);
   assert(C && "couldn't emit constant value non-abstractly?");
   return C;
 }
 
 llvm::GlobalValue *ConstantEmitter::getCurrentAddrPrivate() {
   assert(!Abstract && "cannot get current address for abstract constant");
 
 
if (!D.hasLocalStorage()) {
     InConstantContext = true;
   }
 
   QualType destType = D.getType();
 
   // Try to emit the initializer.  Note that this can allow some things that
   // are not allowed by tryEmitPrivateForMemory alone.
   if (auto value = D.evaluateValue()) {
-    return tryEmitPrivateForMemory(*value, destType);
+    return tryEmitPrivateForMemory(D.getInit()->getExprLoc(), *value, destType);
   }
 
   // FIXME: Implement C++11 [basic.start.init]p2: if the initializer of a
   // reference is a constant expression, and the reference binds to a temporary,
   // then constant initialization is performed. ConstExprEmitter will
   // incorrectly emit a prvalue constant in this case, and the calling code
   // interprets that as the (pointer) value of the reference, rather than the
   // desired value of the referee.

 
 llvm::Constant *
 ConstantEmitter::tryEmitAbstractForMemory(const Expr *E, QualType destType) {
   auto nonMemoryDestType = getNonMemoryType(CGM, destType);
   auto C = tryEmitAbstract(E, nonMemoryDestType);
   return (C ? emitForMemory(C, destType) : nullptr);
 }
 
-llvm::Constant *
-ConstantEmitter::tryEmitAbstractForMemory(const APValue &value,
-                                          QualType destType) {
+llvm::Constant *ConstantEmitter::tryEmitAbstractForMemory(
+    const SourceLocation loc, const APValue &value, QualType destType) {
   auto nonMemoryDestType = getNonMemoryType(CGM, destType);
-  auto C = tryEmitAbstract(value, nonMemoryDestType);
+  auto C = tryEmitAbstract(loc, value, nonMemoryDestType);
   return (C ? emitForMemory(C, destType) : nullptr);
 }
 
 llvm::Constant *ConstantEmitter::tryEmitPrivateForMemory(const Expr *E,
                                                          QualType destType) {
   auto nonMemoryDestType = getNonMemoryType(CGM, destType);
   llvm::Constant *C = tryEmitPrivate(E, nonMemoryDestType);
   return (C ? emitForMemory(C, destType) : nullptr);
 }
 
-llvm::Constant *ConstantEmitter::tryEmitPrivateForMemory(const APValue &value,
-                                                         QualType destType) {
+llvm::Constant *ConstantEmitter::tryEmitPrivateForMemory(
+    const SourceLocation loc, const APValue &value, QualType destType) {
   auto nonMemoryDestType = getNonMemoryType(CGM, destType);
-  auto C = tryEmitPrivate(value, nonMemoryDestType);
+  auto C = tryEmitPrivate(loc, value, nonMemoryDestType);
   return (C ? emitForMemory(C, destType) : nullptr);
 }
 
 llvm::Constant *ConstantEmitter::emitForMemory(CodeGenModule &CGM,
                                                llvm::Constant *C,
                                                QualType destType) {
   // For an _Atomic-qualified constant, we may need to add tail padding.
   if (auto AT = destType->getAs<AtomicType>()) {
llvm::Constant *ConstantEmitter::tryEmitPrivate(const Expr *E,
 
   if (destType->isReferenceType())
     Success = E->EvaluateAsLValue(Result, CGM.getContext());
   else
     Success = E->EvaluateAsRValue(Result, CGM.getContext(), InConstantContext);
 
   llvm::Constant *C;
   if (Success && !Result.HasSideEffects)
-    C = tryEmitPrivate(Result.Val, destType);
+    C = tryEmitPrivate(E->getExprLoc(), Result.Val, destType);
   else
     C = ConstExprEmitter(*this).Visit(const_cast<Expr*>(E), destType);
 
   return C;
 }
 
 llvm::Constant *CodeGenModule::getNullPointer(llvm::PointerType *T, QualType QT) {
   return getTargetCodeGenInfo().getNullPointer(*this, T, QT);
ConstantLValueEmitter::VisitMaterializeTemporaryExpr(
   assert(E->getStorageDuration() == SD_Static);
   SmallVector<const Expr *, 2> CommaLHSs;
   SmallVector<SubobjectAdjustment, 2> Adjustments;
   const Expr *Inner =
       E->getSubExpr()->skipRValueSubobjectAdjustments(CommaLHSs, Adjustments);
   return CGM.GetAddrOfGlobalTemporary(E, Inner);
 }
 
-llvm::Constant *ConstantEmitter::tryEmitPrivate(const APValue &Value,
+llvm::Constant *ConstantEmitter::tryEmitPrivate(const SourceLocation loc,
+                                                const APValue &Value,
                                                 QualType DestType) {
   switch (Value.getKind()) {
   case APValue::None:
   case APValue::Indeterminate:
     // Out-of-lifetime and indeterminate values can be modeled as 'undef'.
     return llvm::UndefValue::get(CGM.getTypes().ConvertType(DestType));
   case APValue::LValue:
     return ConstantLValueEmitter(*this, Value, DestType).tryEmit();
case APValue::AddrLabelDiff: {
 
     // LLVM is a bit sensitive about the exact format of the
     // address-of-label difference; make sure to truncate after
     // the subtraction.
     return llvm::ConstantExpr::getTruncOrBitCast(AddrLabelDiff, ResultType);
   }
   case APValue::Struct:
   case APValue::Union:
-    return ConstStructBuilder::BuildStruct(*this, Value, DestType);
+    return ConstStructBuilder::BuildStruct(*this, loc, Value, DestType);
   case APValue::Array: {
     const ConstantArrayType *CAT =
         CGM.getContext().getAsConstantArrayType(DestType);
     unsigned NumElements = Value.getArraySize();
     unsigned NumInitElts = Value.getArrayInitializedElts();
 
     // Emit array filler, if there is one.
     llvm::Constant *Filler = nullptr;
     if (Value.hasArrayFiller()) {
-      Filler = tryEmitAbstractForMemory(Value.getArrayFiller(),
+      Filler = tryEmitAbstractForMemory(loc, Value.getArrayFiller(),
                                         CAT->getElementType());
       if (!Filler)
         return nullptr;
     }
 
     // Emit initializer elements.
     SmallVector<llvm::Constant*, 16> Elts;
     if (Filler && Filler->isNullValue())
       Elts.reserve(NumInitElts + 1);
     else
       Elts.reserve(NumElements);
 
     llvm::Type *CommonElementType = nullptr;
     for (unsigned I = 0; I < NumInitElts; ++I) {
       llvm::Constant *C = tryEmitPrivateForMemory(
-          Value.getArrayInitializedElt(I), CAT->getElementType());
+          loc, Value.getArrayInitializedElt(I), CAT->getElementType());
       if (!C) return nullptr;
 
       if (I == 0)
         CommonElementType = C->getType();
       else if (C->getType() != CommonElementType)
         CommonElementType = nullptr;
       Elts.push_back(C);
     }

clang/lib/CodeGen/CodeGenModule.cpp

ConstantAddress CodeGenModule::GetAddrOfMSGuidDecl(const MSGuidDecl *GD) {
 
   ConstantEmitter Emitter(*this);
   llvm::Constant *Init;
 
   APValue &V = GD->getAsAPValue();
   if (!V.isAbsent()) {
     // If possible, emit the APValue version of the initializer. In particular,
     // this gets the type of the constant right.
-    Init = Emitter.emitForInitializer(
-        GD->getAsAPValue(), GD->getType().getAddressSpace(), GD->getType());
+    Init = Emitter.emitForInitializer(GD->getLocation(), GD->getAsAPValue(),
+                                      GD->getType().getAddressSpace(),
+                                      GD->getType());
   } else {
     // As a fallback, directly construct the constant.
     // FIXME: This may get padding wrong under esoteric struct layout rules.
     // MSVC appears to create a complete type 'struct __s_GUID' that it
     // presumably uses to represent these constants.
     MSGuidDecl::Parts Parts = GD->getParts();
     llvm::Constant *Fields[4] = {
         llvm::ConstantInt::get(Int32Ty, Parts.Part1),
ConstantAddress CodeGenModule::GetAddrOfGlobalTemporary(
 
   Optional<ConstantEmitter> emitter;
   llvm::Constant *InitialValue = nullptr;
   bool Constant = false;
   llvm::Type *Type;
   if (Value) {
     // The temporary has a constant initializer, use it.
     emitter.emplace(*this);
-    InitialValue = emitter->emitForInitializer(*Value, AddrSpace,
-                                               MaterializedType);
+    InitialValue = emitter->emitForInitializer(Init->getExprLoc(), *Value,
+                                               AddrSpace, MaterializedType);
     Constant = isTypeConstant(MaterializedType, /*ExcludeCtor*/Value);
     Type = InitialValue->getType();
   } else {
     // No initializer, the initialization will be provided when we
     // initialize the declaration which performed lifetime extension.
     Type = getTypes().ConvertTypeForMem(MaterializedType);
   }
 

clang/lib/CodeGen/ConstantEmitter.h

public:
 
   ~ConstantEmitter();
 
   /// Is the current emission context abstract?
   bool isAbstract() const {
     return Abstract;
   }
 
-  /// Try to emit the initiaizer of the given declaration as an abstract
-  /// constant.  If this succeeds, the emission must be finalized.
+  /// Try to emit the initializer of the given declaration as an abstract
+  /// constant. If this succeeds, the emission must be finalized.
   llvm::Constant *tryEmitForInitializer(const VarDecl &D);
   llvm::Constant *tryEmitForInitializer(const Expr *E, LangAS destAddrSpace,
                                         QualType destType);
-  llvm::Constant *emitForInitializer(const APValue &value, LangAS destAddrSpace,
+  llvm::Constant *emitForInitializer(const SourceLocation loc,
+                                     const APValue &value, LangAS destAddrSpace,
                                      QualType destType);
 
   void finalize(llvm::GlobalVariable *global);
 
   // All of the "abstract" emission methods below permit the emission to
   // be immediately discarded without finalizing anything.  Therefore, they
   // must also promise not to do anything that will, in the future, require
   // finalization:
public:
   llvm::Constant *emitAbstract(const Expr *E, QualType T);
   llvm::Constant *emitAbstract(SourceLocation loc, const APValue &value,
                                QualType T);
 
   /// Try to emit the result of the given expression as an abstract constant.
   llvm::Constant *tryEmitAbstract(const Expr *E, QualType T);
   llvm::Constant *tryEmitAbstractForMemory(const Expr *E, QualType T);
 
-  llvm::Constant *tryEmitAbstract(const APValue &value, QualType T);
-  llvm::Constant *tryEmitAbstractForMemory(const APValue &value, QualType T);
+  llvm::Constant *tryEmitAbstract(const SourceLocation loc,
+                                  const APValue &value, QualType T);
+  llvm::Constant *tryEmitAbstractForMemory(const SourceLocation loc,
+                                           const APValue &value, QualType T);
 
   llvm::Constant *emitNullForMemory(QualType T) {
     return emitNullForMemory(CGM, T);
   }
   llvm::Constant *emitForMemory(llvm::Constant *C, QualType T) {
     return emitForMemory(CGM, C, T);
   }
 
   static llvm::Constant *emitNullForMemory(CodeGenModule &CGM, QualType T);
   static llvm::Constant *emitForMemory(CodeGenModule &CGM, llvm::Constant *C,
                                        QualType T);
 
   // These are private helper routines of the constant emitter that
   // can't actually be private because things are split out into helper
   // functions and classes.
 
   llvm::Constant *tryEmitPrivateForVarInit(const VarDecl &D);
 
   llvm::Constant *tryEmitPrivate(const Expr *E, QualType T);
   llvm::Constant *tryEmitPrivateForMemory(const Expr *E, QualType T);
 
-  llvm::Constant *tryEmitPrivate(const APValue &value, QualType T);
-  llvm::Constant *tryEmitPrivateForMemory(const APValue &value, QualType T);
+  llvm::Constant *tryEmitPrivate(const SourceLocation loc, const APValue &value,
+                                 QualType T);
+  llvm::Constant *tryEmitPrivateForMemory(const SourceLocation loc,
+                                          const APValue &value, QualType T);
 
   /// Get the address of the current location.  This is a constant
   /// that will resolve, after finalization, to the address of the
   /// 'signal' value that is registered with the emitter later.
   llvm::GlobalValue *getCurrentAddrPrivate();
 
   /// Register a 'signal' value with the emitter to inform it where to
   /// resolve a placeholder.  The signal value must be unique in the

clang/test/CodeGen/union-tail-padding.c

+// RUN: %clang_cc1 -Wunion-tail-padding -std=c99 -triple aarch64-apple-darwin %s -emit-llvm -o /dev/null -verify
+// RUN: %clang_cc1 -Wunion-tail-padding -std=c11 -triple aarch64-apple-darwin %s -emit-llvm -o /dev/null -verify
+// RUN: %clang_cc1 -Wunion-tail-padding -std=c17 -triple aarch64-apple-darwin %s -emit-llvm -o /dev/null -verify
+// RUN: %clang_cc1 -Wunion-tail-padding -xc++ -std=c++98 -triple aarch64-apple-darwin %s -emit-llvm -o /dev/null -verify
+// RUN: %clang_cc1 -Wunion-tail-padding -xc++ -std=c++03 -triple aarch64-apple-darwin %s -emit-llvm -o /dev/null -verify
+// RUN: %clang_cc1 -Wunion-tail-padding -xc++ -std=c++11 -triple aarch64-apple-darwin %s -emit-llvm -o /dev/null -verify
+// RUN: %clang_cc1 -Wunion-tail-padding -xc++ -std=c++14 -triple aarch64-apple-darwin %s -emit-llvm -o /dev/null -verify
+// RUN: %clang_cc1 -Wunion-tail-padding -xc++ -std=c++17 -triple aarch64-apple-darwin %s -emit-llvm -o /dev/null -verify
+// RUN: %clang_cc1 -Wunion-tail-padding -xc++ -std=c++2a -triple aarch64-apple-darwin %s -emit-llvm -o /dev/null -verify
+
+union OK {
+  char c;
+  unsigned char uc;
+};
+
+union OK ok1;
+union OK ok2 = {};
+union OK ok3 = {42};
+union OK ok4 = {.c = 42};
+union OK ok5 = {.uc = 42};
+#if defined(__cplusplus)
+OK ok6 = OK();
+#if __cplusplus >= 201103
+OK ok7 = OK{};
+#endif
+#endif
+
+union Front {
+  int i;
+  long long ll;
+};
+
+union Front front1;
+union Front front2 = {};        // expected-warning {{Initializing union 'Front' field 'i' only initializes the first 4 of 8 bytes, leaving the remaining 4 bytes undefined}}
+union Front front3 = {42};      // expected-warning {{Initializing union 'Front' field 'i' only initializes the first 4 of 8 bytes, leaving the remaining 4 bytes undefined}}
+union Front front4 = {.i = 42}; // expected-warning {{Initializing union 'Front' field 'i' only initializes the first 4 of 8 bytes, leaving the remaining 4 bytes undefined}}
+union Front front5 = {.ll = 42};
+#if defined(__cplusplus)
+#if __cplusplus < 201103
+Front front6 = Front();
+#elif __cplusplus < 201703
+Front front6 = Front(); // expected-warning {{Initializing union 'Front' field 'i' only initializes the first 4 of 8 bytes, leaving the remaining 4 bytes undefined}}
+Front front7 = Front{}; // expected-warning {{Initializing union 'Front' field 'i' only initializes the first 4 of 8 bytes, leaving the remaining 4 bytes undefined}}
+#else
+Front front6 = Front();
+Front front7 = Front{}; // expected-warning {{Initializing union 'Front' field 'i' only initializes the first 4 of 8 bytes, leaving the remaining 4 bytes undefined}}
+#endif
+#endif
+
+union Back {
+  long long ll;
+  int i;
+};
+
+union Back back1;
+union Back back2 = {};
+union Back back3 = {42};
+union Back back4 = {.i = 42}; // expected-warning {{Initializing union 'Back' field 'i' only initializes the first 4 of 8 bytes, leaving the remaining 4 bytes undefined}}
+union Back back5 = {.ll = 42};
+#if defined(__cplusplus)
+Back back6 = Back();
+#if __cplusplus >= 201103
+Back back7 = Back{};
+#endif
+#endif
+
+#if defined(__cplusplus) && __cplusplus >= 201103
+union FrontInit {
+  int i = 42;
+  long long ll;
+};
+FrontInit frontinit1;               // expected-warning {{Initializing union 'FrontInit' field 'i' only initializes the first 4 of 8 bytes, leaving the remaining 4 bytes undefined}}
+FrontInit frontinit2 = {};          // expected-warning {{Initializing union 'FrontInit' field 'i' only initializes the first 4 of 8 bytes, leaving the remaining 4 bytes undefined}}
+FrontInit frontinit3 = FrontInit(); // expected-warning {{Initializing union 'FrontInit' field 'i' only initializes the first 4 of 8 bytes, leaving the remaining 4 bytes undefined}}
+FrontInit frontinit4 = FrontInit{}; // expected-warning {{Initializing union 'FrontInit' field 'i' only initializes the first 4 of 8 bytes, leaving the remaining 4 bytes undefined}}
+#endif
+
+#if defined(__cplusplus) && __cplusplus >= 201103
+union BackInit {
+  int i;
+  long long ll = 42;
+};
+BackInit backinit1;
+BackInit backinit2 = {};
+BackInit backinit3 = BackInit();
+BackInit backinit4 = BackInit{};
+#endif