Skip to content

Commit 50e628d

Browse files
committed
[BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C
Previously the attribute was only allowed on flexible array members. This patch patch changes this to also allow the attribute on pointer fields in structs and also allows late parsing of the attribute in some contexts. For example this previously wasn't allowed: ``` struct BufferTypeDeclAttributePosition { size_t count; char* buffer __counted_by(count); // Now allowed } ``` Note the attribute is prevented on pointee types where the size isn't known at compile time. In particular pointee types that are: * Incomplete (e.g. `void`) and sizeless types * Function types (e.g. the pointee of a function pointer) * Struct types with a flexible array member This patch also introduces late parsing of the attribute when used in the declaration attribute position. For example ``` struct BufferTypeDeclAttributePosition { char* buffer __counted_by(count); // Now allowed size_t count; } ``` is now allowed but **only** when passing `-fexperimental-late-parse-attributes`. The motivation for using late parsing here is to avoid breaking the data layout of structs in existing code that want to use the `counted_by` attribute. This patch is the first use of `LateAttrParseExperimentalExt` in `Attr.td` that was introduced in a previous patch. Note by allowing the attribute on struct member pointers this now allows the possiblity of writing the attribute in the type attribute position. For example: ``` struct BufferTypeAttributePosition { size_t count; char *__counted_by(count) buffer; // Now allowed } ``` However, the attribute in this position is still currently parsed immediately rather than late parsed. So this will not parse currently: ``` struct BufferTypeAttributePosition { char *__counted_by(count) buffer; // Fails to parse size_t count; } ``` The intention is to lift this restriction in future patches. It has not been done in this patch to keep this size of this commit small. Note this patch only allows parsing of semantic analysis of the attribute on pointers. It does not make use of this on the codegen side (e.g. for _builtin_dynamic_object_size and UBSan's array-bounds checks). Making use of the attribute on pointers will be handled in separate patches. This work is heavily based on a patch originally written by Yeoul Na. rdar://125400257
1 parent b1867e1 commit 50e628d

18 files changed

+626
-38
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,8 @@ New Compiler Flags
258258

259259
- ``-fexperimental-late-parse-attributes`` enables an experimental feature to
260260
allow late parsing certain attributes in specific contexts where they would
261-
not normally be late parsed.
261+
not normally be late parsed. Currently this allows late parsing the
262+
`counted_by` attribute in C. See `Attribute Changes in Clang`_.
262263

263264
Deprecated Compiler Flags
264265
-------------------------
@@ -335,6 +336,22 @@ Attribute Changes in Clang
335336
- Clang now warns that the ``exclude_from_explicit_instantiation`` attribute
336337
is ignored when applied to a local class or a member thereof.
337338

339+
- The ``counted_by`` attribute can now be late parsed in C when
340+
``-fexperimental-late-parse-attributes`` is passed but only when attribute is
341+
used in the declaration attribute position. This allows using the
342+
attribute on existing code where it previously impossible to do so without
343+
re-ordering struct field declarations would break ABI as shown below.
344+
345+
.. code-block:: c
346+
347+
struct BufferTy {
348+
/* Refering to `count` requires late parsing */
349+
char* buffer __counted_by(count);
350+
/* Swapping `buffer` and `count` to avoid late parsing would break ABI */
351+
size_t count;
352+
};
353+
354+
338355
Improvements to Clang's diagnostics
339356
-----------------------------------
340357
- Clang now applies syntax highlighting to the code snippets it

clang/include/clang/AST/Type.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2515,6 +2515,7 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
25152515
bool isRecordType() const;
25162516
bool isClassType() const;
25172517
bool isStructureType() const;
2518+
bool isStructureTypeWithFlexibleArrayMember() const;
25182519
bool isObjCBoxableRecordType() const;
25192520
bool isInterfaceType() const;
25202521
bool isStructureOrClassType() const;

clang/include/clang/Basic/Attr.td

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2229,7 +2229,8 @@ def TypeNullUnspecified : TypeAttr {
22292229
def CountedBy : DeclOrTypeAttr {
22302230
let Spellings = [Clang<"counted_by">];
22312231
let Subjects = SubjectList<[Field], ErrorDiag>;
2232-
let Args = [ExprArgument<"Count">, IntArgument<"NestedLevel">];
2232+
let Args = [ExprArgument<"Count">, IntArgument<"NestedLevel", 1>];
2233+
let LateParsed = LateAttrParseExperimentalExt;
22332234
let ParseArgumentsAsUnevaluated = 1;
22342235
let Documentation = [CountedByDocs];
22352236
let LangOpts = [COnly];

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6518,8 +6518,10 @@ def warn_superclass_variable_sized_type_not_at_end : Warning<
65186518

65196519
def err_flexible_array_count_not_in_same_struct : Error<
65206520
"'counted_by' field %0 isn't within the same struct as the flexible array">;
6521-
def err_counted_by_attr_not_on_flexible_array_member : Error<
6522-
"'counted_by' only applies to C99 flexible array members">;
6521+
def err_counted_by_attr_not_on_ptr_or_flexible_array_member : Error<
6522+
"'counted_by' only applies to pointers or C99 flexible array members">;
6523+
def err_counted_by_attr_on_array_not_flexible_array_member : Error<
6524+
"'counted_by' on arrays only applies to C99 flexible array members">;
65236525
def err_counted_by_attr_refer_to_itself : Error<
65246526
"'counted_by' cannot refer to the flexible array member %0">;
65256527
def err_counted_by_must_be_in_structure : Error<
@@ -6534,6 +6536,15 @@ def err_counted_by_attr_refer_to_union : Error<
65346536
"'counted_by' argument cannot refer to a union member">;
65356537
def note_flexible_array_counted_by_attr_field : Note<
65366538
"field %0 declared here">;
6539+
def err_counted_by_attr_pointee_unknown_size : Error<
6540+
"'counted_by' cannot be applied a pointer with pointee with unknown size "
6541+
"because %0 is %select{"
6542+
"an incomplete type|" // CountedByInvalidPointeeTypeKind::INCOMPLETE
6543+
"a sizeless type|" // CountedByInvalidPointeeTypeKind::SIZELESS
6544+
"a function type|" // CountedByInvalidPointeeTypeKind::FUNCTION
6545+
// CountedByInvalidPointeeTypeKind::FLEXIBLE_ARRAY_MEMBER
6546+
"a struct type with a flexible array member"
6547+
"}1">;
65376548

65386549
let CategoryName = "ARC Semantic Issue" in {
65396550

clang/include/clang/Parse/Parser.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1643,6 +1643,8 @@ class Parser : public CodeCompletionHandler {
16431643
bool EnterScope, bool OnDefinition);
16441644
void ParseLexedAttribute(LateParsedAttribute &LA,
16451645
bool EnterScope, bool OnDefinition);
1646+
void ParseLexedCAttribute(LateParsedAttribute &LA,
1647+
ParsedAttributes *OutAttrs = nullptr);
16461648
void ParseLexedMethodDeclarations(ParsingClass &Class);
16471649
void ParseLexedMethodDeclaration(LateParsedMethodDeclaration &LM);
16481650
void ParseLexedMethodDefs(ParsingClass &Class);
@@ -2530,7 +2532,8 @@ class Parser : public CodeCompletionHandler {
25302532

25312533
void ParseStructDeclaration(
25322534
ParsingDeclSpec &DS,
2533-
llvm::function_ref<void(ParsingFieldDeclarator &)> FieldsCallback);
2535+
llvm::function_ref<Decl *(ParsingFieldDeclarator &)> FieldsCallback,
2536+
LateParsedAttrList *LateFieldAttrs = nullptr);
25342537

25352538
DeclGroupPtrTy ParseTopLevelStmtDecl();
25362539

@@ -3107,6 +3110,8 @@ class Parser : public CodeCompletionHandler {
31073110
SourceLocation ScopeLoc,
31083111
ParsedAttr::Form Form);
31093112

3113+
void DistributeCLateParsedAttrs(Decl *Dcl, LateParsedAttrList *LateAttrs);
3114+
31103115
void ParseBoundsAttribute(IdentifierInfo &AttrName,
31113116
SourceLocation AttrNameLoc, ParsedAttributes &Attrs,
31123117
IdentifierInfo *ScopeName, SourceLocation ScopeLoc,

clang/include/clang/Sema/Sema.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11509,7 +11509,8 @@ class Sema final : public SemaBase {
1150911509
QualType BuildMatrixType(QualType T, Expr *NumRows, Expr *NumColumns,
1151011510
SourceLocation AttrLoc);
1151111511

11512-
QualType BuildCountAttributedArrayType(QualType WrappedTy, Expr *CountExpr);
11512+
QualType BuildCountAttributedArrayOrPointerType(QualType WrappedTy,
11513+
Expr *CountExpr);
1151311514

1151411515
QualType BuildAddressSpaceAttr(QualType &T, LangAS ASIdx, Expr *AddrSpace,
1151511516
SourceLocation AttrLoc);

clang/lib/AST/Type.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,18 @@ bool Type::isStructureType() const {
631631
return false;
632632
}
633633

634+
bool Type::isStructureTypeWithFlexibleArrayMember() const {
635+
const auto *RT = getAs<RecordType>();
636+
if (!RT)
637+
return false;
638+
const auto *Decl = RT->getDecl();
639+
if (!Decl->isStruct())
640+
return false;
641+
if (!Decl->hasFlexibleArrayMember())
642+
return false;
643+
return true;
644+
}
645+
634646
bool Type::isObjCBoxableRecordType() const {
635647
if (const auto *RT = getAs<RecordType>())
636648
return RT->getDecl()->hasAttr<ObjCBoxableAttr>();

clang/lib/Parse/ParseDecl.cpp

Lines changed: 97 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3282,6 +3282,19 @@ void Parser::ParseAlignmentSpecifier(ParsedAttributes &Attrs,
32823282
}
32833283
}
32843284

3285+
void Parser::DistributeCLateParsedAttrs(Decl *Dcl,
3286+
LateParsedAttrList *LateAttrs) {
3287+
assert(Dcl);
3288+
3289+
if (!LateAttrs)
3290+
return;
3291+
3292+
for (auto *LateAttr : *LateAttrs) {
3293+
if (LateAttr->Decls.empty())
3294+
LateAttr->addDecl(Dcl);
3295+
}
3296+
}
3297+
32853298
/// Bounds attributes (e.g., counted_by):
32863299
/// AttrName '(' expression ')'
32873300
void Parser::ParseBoundsAttribute(IdentifierInfo &AttrName,
@@ -4815,13 +4828,14 @@ static void DiagnoseCountAttributedTypeInUnnamedAnon(ParsingDeclSpec &DS,
48154828
///
48164829
void Parser::ParseStructDeclaration(
48174830
ParsingDeclSpec &DS,
4818-
llvm::function_ref<void(ParsingFieldDeclarator &)> FieldsCallback) {
4831+
llvm::function_ref<Decl *(ParsingFieldDeclarator &)> FieldsCallback,
4832+
LateParsedAttrList *LateFieldAttrs) {
48194833

48204834
if (Tok.is(tok::kw___extension__)) {
48214835
// __extension__ silences extension warnings in the subexpression.
48224836
ExtensionRAIIObject O(Diags); // Use RAII to do this.
48234837
ConsumeToken();
4824-
return ParseStructDeclaration(DS, FieldsCallback);
4838+
return ParseStructDeclaration(DS, FieldsCallback, LateFieldAttrs);
48254839
}
48264840

48274841
// Parse leading attributes.
@@ -4886,10 +4900,12 @@ void Parser::ParseStructDeclaration(
48864900
}
48874901

48884902
// If attributes exist after the declarator, parse them.
4889-
MaybeParseGNUAttributes(DeclaratorInfo.D);
4903+
MaybeParseGNUAttributes(DeclaratorInfo.D, LateFieldAttrs);
48904904

48914905
// We're done with this declarator; invoke the callback.
4892-
FieldsCallback(DeclaratorInfo);
4906+
Decl *Field = FieldsCallback(DeclaratorInfo);
4907+
if (Field)
4908+
DistributeCLateParsedAttrs(Field, LateFieldAttrs);
48934909

48944910
// If we don't have a comma, it is either the end of the list (a ';')
48954911
// or an error, bail out.
@@ -4900,6 +4916,69 @@ void Parser::ParseStructDeclaration(
49004916
}
49014917
}
49024918

4919+
/// Finish parsing an attribute for which parsing was delayed.
4920+
/// This will be called at the end of parsing a class declaration
4921+
/// for each LateParsedAttribute. We consume the saved tokens and
4922+
/// create an attribute with the arguments filled in. We add this
4923+
/// to the Attribute list for the decl.
4924+
void Parser::ParseLexedCAttribute(LateParsedAttribute &LA,
4925+
ParsedAttributes *OutAttrs) {
4926+
// Create a fake EOF so that attribute parsing won't go off the end of the
4927+
// attribute.
4928+
Token AttrEnd;
4929+
AttrEnd.startToken();
4930+
AttrEnd.setKind(tok::eof);
4931+
AttrEnd.setLocation(Tok.getLocation());
4932+
AttrEnd.setEofData(LA.Toks.data());
4933+
LA.Toks.push_back(AttrEnd);
4934+
4935+
// Append the current token at the end of the new token stream so that it
4936+
// doesn't get lost.
4937+
LA.Toks.push_back(Tok);
4938+
PP.EnterTokenStream(LA.Toks, /*DisableMacroExpansion=*/true,
4939+
/*IsReinject=*/true);
4940+
// Drop the current token and bring the first cached one. It's the same token
4941+
// as when we entered this function.
4942+
ConsumeAnyToken(/*ConsumeCodeCompletionTok=*/true);
4943+
4944+
ParsedAttributes Attrs(AttrFactory);
4945+
4946+
assert(LA.Decls.size() <= 1 &&
4947+
"late field attribute expects to have at most one declaration.");
4948+
4949+
// Dispatch based on the attribute and parse it
4950+
const AttributeCommonInfo::Form ParsedForm = ParsedAttr::Form::GNU();
4951+
IdentifierInfo *ScopeName = nullptr;
4952+
const ParsedAttr::Kind AttrKind =
4953+
ParsedAttr::getParsedKind(&LA.AttrName, /*ScopeName=*/ScopeName,
4954+
/*SyntaxUsed=*/ParsedForm.getSyntax());
4955+
switch (AttrKind) {
4956+
case ParsedAttr::Kind::AT_CountedBy:
4957+
ParseBoundsAttribute(LA.AttrName, LA.AttrNameLoc, Attrs,
4958+
/*ScopeName=*/ScopeName, SourceLocation(),
4959+
/*Form=*/ParsedForm);
4960+
break;
4961+
default:
4962+
llvm_unreachable("Unhandled late parsed attribute");
4963+
}
4964+
4965+
for (auto *D : LA.Decls)
4966+
Actions.ActOnFinishDelayedAttribute(getCurScope(), D, Attrs);
4967+
4968+
// Due to a parsing error, we either went over the cached tokens or
4969+
// there are still cached tokens left, so we skip the leftover tokens.
4970+
while (Tok.isNot(tok::eof))
4971+
ConsumeAnyToken();
4972+
4973+
// Consume the fake EOF token if it's there
4974+
if (Tok.is(tok::eof) && Tok.getEofData() == AttrEnd.getEofData())
4975+
ConsumeAnyToken();
4976+
4977+
if (OutAttrs) {
4978+
OutAttrs->takeAllFrom(Attrs);
4979+
}
4980+
}
4981+
49034982
/// ParseStructUnionBody
49044983
/// struct-contents:
49054984
/// struct-declaration-list
@@ -4923,6 +5002,11 @@ void Parser::ParseStructUnionBody(SourceLocation RecordLoc,
49235002
ParseScope StructScope(this, Scope::ClassScope|Scope::DeclScope);
49245003
Actions.ActOnTagStartDefinition(getCurScope(), TagDecl);
49255004

5005+
// `LateAttrParseExperimentalExtOnly=true` requests that only attributes
5006+
// marked with `LateAttrParseExperimentalExt` are late parsed.
5007+
LateParsedAttrList LateFieldAttrs(/*PSoon=*/false,
5008+
/*LateAttrParseExperimentalExtOnly=*/true);
5009+
49265010
// While we still have something to read, read the declarations in the struct.
49275011
while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&
49285012
Tok.isNot(tok::eof)) {
@@ -4973,18 +5057,19 @@ void Parser::ParseStructUnionBody(SourceLocation RecordLoc,
49735057
}
49745058

49755059
if (!Tok.is(tok::at)) {
4976-
auto CFieldCallback = [&](ParsingFieldDeclarator &FD) {
5060+
auto CFieldCallback = [&](ParsingFieldDeclarator &FD) -> Decl * {
49775061
// Install the declarator into the current TagDecl.
49785062
Decl *Field =
49795063
Actions.ActOnField(getCurScope(), TagDecl,
49805064
FD.D.getDeclSpec().getSourceRange().getBegin(),
49815065
FD.D, FD.BitfieldSize);
49825066
FD.complete(Field);
5067+
return Field;
49835068
};
49845069

49855070
// Parse all the comma separated declarators.
49865071
ParsingDeclSpec DS(*this);
4987-
ParseStructDeclaration(DS, CFieldCallback);
5072+
ParseStructDeclaration(DS, CFieldCallback, &LateFieldAttrs);
49885073
} else { // Handle @defs
49895074
ConsumeToken();
49905075
if (!Tok.isObjCAtKeyword(tok::objc_defs)) {
@@ -5025,7 +5110,12 @@ void Parser::ParseStructUnionBody(SourceLocation RecordLoc,
50255110

50265111
ParsedAttributes attrs(AttrFactory);
50275112
// If attributes exist after struct contents, parse them.
5028-
MaybeParseGNUAttributes(attrs);
5113+
MaybeParseGNUAttributes(attrs, &LateFieldAttrs);
5114+
5115+
// Late parse field attributes if necessary.
5116+
assert(!getLangOpts().CPlusPlus);
5117+
for (auto *LateAttr : LateFieldAttrs)
5118+
ParseLexedCAttribute(*LateAttr);
50295119

50305120
SmallVector<Decl *, 32> FieldDecls(TagDecl->fields());
50315121

clang/lib/Parse/ParseObjc.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -778,16 +778,16 @@ void Parser::ParseObjCInterfaceDeclList(tok::ObjCKeywordKind contextKey,
778778
}
779779

780780
bool addedToDeclSpec = false;
781-
auto ObjCPropertyCallback = [&](ParsingFieldDeclarator &FD) {
781+
auto ObjCPropertyCallback = [&](ParsingFieldDeclarator &FD) -> Decl * {
782782
if (FD.D.getIdentifier() == nullptr) {
783783
Diag(AtLoc, diag::err_objc_property_requires_field_name)
784784
<< FD.D.getSourceRange();
785-
return;
785+
return nullptr;
786786
}
787787
if (FD.BitfieldSize) {
788788
Diag(AtLoc, diag::err_objc_property_bitfield)
789789
<< FD.D.getSourceRange();
790-
return;
790+
return nullptr;
791791
}
792792

793793
// Map a nullability property attribute to a context-sensitive keyword
@@ -816,6 +816,7 @@ void Parser::ParseObjCInterfaceDeclList(tok::ObjCKeywordKind contextKey,
816816
MethodImplKind);
817817

818818
FD.complete(Property);
819+
return Property;
819820
};
820821

821822
// Parse all the comma separated declarators.
@@ -2024,7 +2025,7 @@ void Parser::ParseObjCClassInstanceVariables(ObjCContainerDecl *interfaceDecl,
20242025
continue;
20252026
}
20262027

2027-
auto ObjCIvarCallback = [&](ParsingFieldDeclarator &FD) {
2028+
auto ObjCIvarCallback = [&](ParsingFieldDeclarator &FD) -> Decl * {
20282029
assert(getObjCDeclContext() == interfaceDecl &&
20292030
"Ivar should have interfaceDecl as its decl context");
20302031
// Install the declarator into the interface decl.
@@ -2035,6 +2036,7 @@ void Parser::ParseObjCClassInstanceVariables(ObjCContainerDecl *interfaceDecl,
20352036
if (Field)
20362037
AllIvarDecls.push_back(Field);
20372038
FD.complete(Field);
2039+
return Field;
20382040
};
20392041

20402042
// Parse all the comma separated declarators.

0 commit comments

Comments
 (0)