Skip to content

Commit

Permalink
Implement explicit field initialization checks in RecordDecl and make…
Browse files Browse the repository at this point in the history
… it work in both C and C++
  • Loading branch information
higher-performance committed Oct 2, 2024
1 parent 7ea9d3d commit 93bbb04
Show file tree
Hide file tree
Showing 15 changed files with 196 additions and 66 deletions.
9 changes: 0 additions & 9 deletions clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,6 @@ FIELD(HasInitMethod, 1, NO_MERGE)
/// within anonymous unions or structs.
FIELD(HasInClassInitializer, 1, NO_MERGE)

/// Custom attribute that is True if any field is marked as requiring explicit
/// initialization with [[clang::requires_explicit_initialization]] in a type
/// without a user-provided default constructor, or if this is the case for any
/// base classes and/or member variables whose types are aggregates.
///
/// In this case, default-construction is diagnosed, as it would not explicitly
/// initialize the field.
FIELD(HasUninitializedExplicitInitFields, 1, NO_MERGE)

/// True if any field is of reference type, and does not have an
/// in-class initializer.
///
Expand Down
8 changes: 8 additions & 0 deletions clang/include/clang/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -4269,6 +4269,14 @@ class RecordDecl : public TagDecl {
RecordDeclBits.HasNonTrivialToPrimitiveCopyCUnion = V;
}

bool hasUninitializedExplicitInitFields() const {
return RecordDeclBits.HasUninitializedExplicitInitFields;
}

void setHasUninitializedExplicitInitFields(bool V) {
RecordDeclBits.HasUninitializedExplicitInitFields = V;
}

/// Determine whether this class can be passed in registers. In C++ mode,
/// it must have at least one trivial, non-deleted copy or move constructor.
/// FIXME: This should be set as part of completeDefinition.
Expand Down
13 changes: 12 additions & 1 deletion clang/include/clang/AST/DeclBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -1447,6 +1447,9 @@ class DeclContext {
/// hasLazyLocalLexicalLookups, hasLazyExternalLexicalLookups
friend class ASTWriter;

protected:
enum { NumOdrHashBits = 25 };

// We use uint64_t in the bit-fields below since some bit-fields
// cross the unsigned boundary and this breaks the packing.

Expand Down Expand Up @@ -1668,6 +1671,14 @@ class DeclContext {
LLVM_PREFERRED_TYPE(bool)
uint64_t HasNonTrivialToPrimitiveCopyCUnion : 1;

/// True if any field is marked as requiring explicit initialization with
/// [[clang::requires_explicit_initialization]].
/// In C++, this is also set for types without a user-provided default
/// constructor, and is propagated from any base classes and/or member
/// variables whose types are aggregates.
LLVM_PREFERRED_TYPE(bool)
uint64_t HasUninitializedExplicitInitFields : 1;

/// Indicates whether this struct is destroyed in the callee.
LLVM_PREFERRED_TYPE(bool)
uint64_t ParamDestroyedInCallee : 1;
Expand All @@ -1682,7 +1693,7 @@ class DeclContext {

/// True if a valid hash is stored in ODRHash. This should shave off some
/// extra storage and prevent CXXRecordDecl to store unused bits.
uint64_t ODRHash : 26;
uint64_t ODRHash : NumOdrHashBits;
};

/// Number of inherited and non-inherited bits in RecordDeclBitfields.
Expand Down
5 changes: 0 additions & 5 deletions clang/include/clang/AST/DeclCXX.h
Original file line number Diff line number Diff line change
Expand Up @@ -1152,11 +1152,6 @@ class CXXRecordDecl : public RecordDecl {
/// structs).
bool hasInClassInitializer() const { return data().HasInClassInitializer; }

bool hasUninitializedExplicitInitFields() const {
return !isUnion() && !hasUserProvidedDefaultConstructor() &&
data().HasUninitializedExplicitInitFields;
}

/// Whether this class or any of its subobjects has any members of
/// reference type which would make value-initialization ill-formed.
///
Expand Down
3 changes: 1 addition & 2 deletions clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -1862,10 +1862,9 @@ def Leaf : InheritableAttr {
}

def ExplicitInit : InheritableAttr {
let Spellings = [Clang<"requires_explicit_initialization", 0>];
let Spellings = [Clang<"requires_explicit_initialization", 1>];
let Subjects = SubjectList<[Field], ErrorDiag>;
let Documentation = [ExplicitInitDocs];
let LangOpts = [CPlusPlus];
let SimpleHandler = 1;
}

Expand Down
31 changes: 19 additions & 12 deletions clang/include/clang/Basic/AttrDocs.td
Original file line number Diff line number Diff line change
Expand Up @@ -1426,24 +1426,31 @@ The ``clang::requires_explicit_initialization`` attribute indicates that the
field of an aggregate must be initialized explicitly by users when the class
is constructed. Its usage is invalid on non-aggregates.

Example usage:
Note that this attribute is *not* a memory safety feature, and is *not* intended
to guard against use of uninitialized memory.

Rather, it is intended for use in "parameter-objects", used to simulate the
passing of named parameters.
The attribute generates a warning when explicit initializers for such
"named parameters" are not provided:

.. code-block:: c++

struct some_aggregate {
int x;
int y [[clang::requires_explicit_initialization]];
struct ArrayIOParams {
size_t count [[clang::requires_explicit_initialization]];
size_t element_size [[clang::requires_explicit_initialization]];
int flags = 0;
};

some_aggregate create() {
return {.x = 1}; // error: y is not initialized explicitly
}
size_t ReadArray(FILE *file, void *buffer, ArrayIOParams params);

This attribute is *not* a memory safety feature, and is *not* intended to guard
against use of uninitialized memory.
Rather, its intended use is in structs that represent "parameter objects", to
allow extending them while ensuring that callers do not forget to specify
values for newly added fields ("parameters").
int main() {
unsigned int buf[512];
ReadArray(stdin, buf, {
.count = sizeof(buf) / sizeof(*buf),
// warning: field 'element_size' is not explicitly initialized
});
}

}];
}
Expand Down
8 changes: 6 additions & 2 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -2336,8 +2336,8 @@ def err_init_reference_member_uninitialized : Error<
def note_uninit_reference_member : Note<
"uninitialized reference member is here">;
def warn_field_requires_explicit_init : Warning<
"field %0 is not explicitly initialized, but was marked as requiring "
"explicit initialization">, InGroup<UninitializedExplicitInit>;
"field %select{%1|in %1}0 is not explicitly initialized, but was marked as "
"requiring explicit initialization">, InGroup<UninitializedExplicitInit>;
def warn_field_is_uninit : Warning<"field %0 is uninitialized when used here">,
InGroup<Uninitialized>;
def warn_base_class_is_uninit : Warning<
Expand Down Expand Up @@ -3148,6 +3148,10 @@ def warn_attribute_ignored_no_calls_in_stmt: Warning<
"statement">,
InGroup<IgnoredAttributes>;

def warn_attribute_needs_aggregate : Warning<
"%0 attribute is ignored in non-aggregate type %1">,
InGroup<IgnoredAttributes>;

def warn_attribute_ignored_non_function_pointer: Warning<
"%0 attribute is ignored because %1 is not a function pointer">,
InGroup<IgnoredAttributes>;
Expand Down
8 changes: 5 additions & 3 deletions clang/lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5014,6 +5014,7 @@ RecordDecl::RecordDecl(Kind DK, TagKind TK, const ASTContext &C,
setHasNonTrivialToPrimitiveDefaultInitializeCUnion(false);
setHasNonTrivialToPrimitiveDestructCUnion(false);
setHasNonTrivialToPrimitiveCopyCUnion(false);
setHasUninitializedExplicitInitFields(false);
setParamDestroyedInCallee(false);
setArgPassingRestrictions(RecordArgPassingKind::CanPassInRegs);
setIsRandomized(false);
Expand Down Expand Up @@ -5214,9 +5215,10 @@ unsigned RecordDecl::getODRHash() {
// Only calculate hash on first call of getODRHash per record.
ODRHash Hash;
Hash.AddRecordDecl(this);
// For RecordDecl the ODRHash is stored in the remaining 26
// bit of RecordDeclBits, adjust the hash to accomodate.
setODRHash(Hash.CalculateHash() >> 6);
// For RecordDecl the ODRHash is stored in the remaining
// bits of RecordDeclBits, adjust the hash to accommodate.
static_assert(sizeof(Hash.CalculateHash()) * CHAR_BIT == 32);
setODRHash(Hash.CalculateHash() >> (32 - NumOdrHashBits));
return RecordDeclBits.ODRHash;
}

Expand Down
32 changes: 25 additions & 7 deletions clang/lib/AST/DeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D)
HasPrivateFields(false), HasProtectedFields(false),
HasPublicFields(false), HasMutableFields(false), HasVariantMembers(false),
HasOnlyCMembers(true), HasInitMethod(false), HasInClassInitializer(false),
HasUninitializedExplicitInitFields(false),
HasUninitializedReferenceMember(false), HasUninitializedFields(false),
HasInheritedConstructor(false), HasInheritedDefaultConstructor(false),
HasInheritedAssignment(false),
Expand Down Expand Up @@ -460,6 +459,10 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases,
if (BaseClassDecl->hasMutableFields())
data().HasMutableFields = true;

if (BaseClassDecl->hasUninitializedExplicitInitFields() &&
BaseClassDecl->isAggregate())
setHasUninitializedExplicitInitFields(true);

if (BaseClassDecl->hasUninitializedReferenceMember())
data().HasUninitializedReferenceMember = true;

Expand Down Expand Up @@ -1114,7 +1117,7 @@ void CXXRecordDecl::addedMember(Decl *D) {
data().PlainOldData = false;

if (Field->hasAttr<ExplicitInitAttr>() && !Field->hasInClassInitializer()) {
data().HasUninitializedExplicitInitFields = true;
setHasUninitializedExplicitInitFields(true);
}

if (T->isReferenceType()) {
Expand Down Expand Up @@ -1370,7 +1373,7 @@ void CXXRecordDecl::addedMember(Decl *D) {

if (FieldRec->hasUninitializedExplicitInitFields() &&
FieldRec->isAggregate() && !Field->hasInClassInitializer())
data().HasUninitializedExplicitInitFields = true;
setHasUninitializedExplicitInitFields(true);

if (FieldRec->hasUninitializedReferenceMember() &&
!Field->hasInClassInitializer())
Expand Down Expand Up @@ -2160,17 +2163,32 @@ void CXXRecordDecl::completeDefinition(CXXFinalOverriderMap *FinalOverriders) {
I.setAccess((*I)->getAccess());

ASTContext &Context = getASTContext();
if (!Context.getLangOpts().CPlusPlus20 && isAggregate() &&
hasUserDeclaredConstructor()) {

if (isAggregate() && hasUserDeclaredConstructor() &&
!Context.getLangOpts().CPlusPlus20) {
// Diagnose any aggregate behavior changes in C++20
for (field_iterator I = field_begin(), E = field_end(); I != E; ++I) {
if (const auto *attr = I->getAttr<ExplicitInitAttr>()) {
Context.getDiagnostics().Report(
getLocation(),
attr->getLocation(),
diag::warn_cxx20_compat_requires_explicit_init_non_aggregate)
<< attr->getRange() << Context.getRecordType(this);
<< attr << Context.getRecordType(this);
}
}
}

if (!isAggregate() && hasUninitializedExplicitInitFields()) {
// Diagnose any fields that required explicit initialization in a
// non-aggregate type. (Note that the fields may not be directly in this
// type, but in a subobject. In such cases we don't emit diagnoses here.)
for (field_iterator I = field_begin(), E = field_end(); I != E; ++I) {
if (const auto *attr = I->getAttr<ExplicitInitAttr>()) {
Context.getDiagnostics().Report(attr->getLocation(),
diag::warn_attribute_needs_aggregate)
<< attr << Context.getRecordType(this);
}
}
setHasUninitializedExplicitInitFields(false);
}
}

Expand Down
3 changes: 3 additions & 0 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19086,6 +19086,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
if (FT.hasNonTrivialToPrimitiveCopyCUnion() || Record->isUnion())
Record->setHasNonTrivialToPrimitiveCopyCUnion(true);
}
if (FD->hasAttr<ExplicitInitAttr>()) {
Record->setHasUninitializedExplicitInitFields(true);
}
if (FT.isDestructedType()) {
Record->setNonTrivialToPrimitiveDestroy(true);
Record->setParamDestroyedInCallee(true);
Expand Down
26 changes: 24 additions & 2 deletions clang/lib/Sema/SemaInit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,14 @@ static void CheckStringInit(Expr *Str, QualType &DeclT, const ArrayType *AT,
updateStringLiteralType(Str, DeclT);
}

void emitUninitializedExplicitInitFields(Sema &S, const RecordDecl *R) {
for (const FieldDecl *Field : R->fields()) {
if (Field->hasAttr<ExplicitInitAttr>()) {
S.Diag(Field->getLocation(), diag::note_entity_declared_at) << Field;
}
}
}

//===----------------------------------------------------------------------===//
// Semantic checking for initializer lists.
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -746,7 +754,7 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field,

if (!VerifyOnly && Field->hasAttr<ExplicitInitAttr>()) {
SemaRef.Diag(ILE->getExprLoc(), diag::warn_field_requires_explicit_init)
<< Field;
<< /* Var-in-Record */ 0 << Field;
SemaRef.Diag(Field->getLocation(), diag::note_entity_declared_at)
<< Field;
}
Expand Down Expand Up @@ -4573,7 +4581,8 @@ static void TryConstructorInitialization(Sema &S,
DestRecordDecl != nullptr && DestRecordDecl->isAggregate() &&
DestRecordDecl->hasUninitializedExplicitInitFields()) {
S.Diag(Kind.getLocation(), diag::warn_field_requires_explicit_init)
<< "in class";
<< /* Var-in-Record */ 1 << DestRecordDecl;
emitUninitializedExplicitInitFields(S, DestRecordDecl);
}

// C++11 [dcl.init]p6:
Expand Down Expand Up @@ -6472,6 +6481,19 @@ void InitializationSequence::InitializeFrom(Sema &S,
}
}

if (!S.getLangOpts().CPlusPlus &&
Kind.getKind() == InitializationKind::IK_Default) {
RecordDecl *Rec = DestType->getAsRecordDecl();
if (Rec && Rec->hasUninitializedExplicitInitFields()) {
VarDecl *Var = dyn_cast_or_null<VarDecl>(Entity.getDecl());
if (Var && !Initializer) {
S.Diag(Var->getLocation(), diag::warn_field_requires_explicit_init)
<< /* Var-in-Record */ 1 << Rec;
emitUninitializedExplicitInitFields(S, Rec);
}
}
}

// - If the destination type is a reference type, see 8.5.3.
if (DestType->isReferenceType()) {
// C++0x [dcl.init.ref]p1:
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Serialization/ASTReaderDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,7 @@ RedeclarableResult ASTDeclReader::VisitRecordDeclImpl(RecordDecl *RD) {
RecordDeclBits.getNextBit());
RD->setHasNonTrivialToPrimitiveDestructCUnion(RecordDeclBits.getNextBit());
RD->setHasNonTrivialToPrimitiveCopyCUnion(RecordDeclBits.getNextBit());
RD->setHasUninitializedExplicitInitFields(RecordDeclBits.getNextBit());
RD->setParamDestroyedInCallee(RecordDeclBits.getNextBit());
RD->setArgPassingRestrictions(
(RecordArgPassingKind)RecordDeclBits.getNextBits(/*Width=*/2));
Expand Down
4 changes: 3 additions & 1 deletion clang/lib/Serialization/ASTWriterDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,7 @@ void ASTDeclWriter::VisitRecordDecl(RecordDecl *D) {
RecordDeclBits.addBit(D->hasNonTrivialToPrimitiveDefaultInitializeCUnion());
RecordDeclBits.addBit(D->hasNonTrivialToPrimitiveDestructCUnion());
RecordDeclBits.addBit(D->hasNonTrivialToPrimitiveCopyCUnion());
RecordDeclBits.addBit(D->hasUninitializedExplicitInitFields());
RecordDeclBits.addBit(D->isParamDestroyedInCallee());
RecordDeclBits.addBits(llvm::to_underlying(D->getArgPassingRestrictions()), 2);
Record.push_back(RecordDeclBits);
Expand Down Expand Up @@ -2407,7 +2408,8 @@ void ASTWriter::WriteDeclAbbrevs() {
// isNonTrivialToPrimitiveCopy, isNonTrivialToPrimitiveDestroy,
// hasNonTrivialToPrimitiveDefaultInitializeCUnion,
// hasNonTrivialToPrimitiveDestructCUnion,
// hasNonTrivialToPrimitiveCopyCUnion, isParamDestroyedInCallee,
// hasNonTrivialToPrimitiveCopyCUnion,
// hasUninitializedExplicitInitFields, isParamDestroyedInCallee,
// getArgPassingRestrictions
// ODRHash
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 26));
Expand Down
11 changes: 11 additions & 0 deletions clang/test/Sema/uninit-variables.c
Original file line number Diff line number Diff line change
Expand Up @@ -551,3 +551,14 @@ struct full_of_empty empty_test_2(void) {
struct full_of_empty e;
return e; // no-warning
}

struct with_explicit_field {
int x;
int y [[clang::requires_explicit_initialization]]; // #FIELD_Y
};

void aggregate() {
struct with_explicit_field a; // expected-warning {{field in 'with_explicit_field' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_Y {{'y' declared here}}
struct with_explicit_field b = {1}; // expected-warning {{field 'y' is not explicitly initialized, but was marked as requiring explicit initialization}} expected-note@#FIELD_Y {{'y' declared here}}
(void)(&a != &b);
}
Loading

0 comments on commit 93bbb04

Please sign in to comment.