Skip to content

Commit 67b675e

Browse files
authored
Revert "[Clang] Implement the 'counted_by' attribute" (#68603)
This reverts commit 9a954c6, which causes clang crashes when compiling with `-fsanitize=bounds`. See 9a954c6#commitcomment-129529574 for details.
1 parent 08b20d8 commit 67b675e

18 files changed

+77
-770
lines changed

clang/docs/ReleaseNotes.rst

-5
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,6 @@ C Language Changes
139139
- ``structs``, ``unions``, and ``arrays`` that are const may now be used as
140140
constant expressions. This change is more consistent with the behavior of
141141
GCC.
142-
- Clang now supports the C-only attribute ``counted_by``. When applied to a
143-
struct's flexible array member, it points to the struct field that holds the
144-
number of elements in the flexible array member. This information can improve
145-
the results of the array bound sanitizer and the
146-
``__builtin_dynamic_object_size`` builtin.
147142

148143
C23 Feature Support
149144
^^^^^^^^^^^^^^^^^^^

clang/include/clang/AST/Decl.h

-24
Original file line numberDiff line numberDiff line change
@@ -4302,30 +4302,6 @@ class RecordDecl : public TagDecl {
43024302
return field_begin() == field_end();
43034303
}
43044304

4305-
FieldDecl *getLastField() {
4306-
FieldDecl *FD = nullptr;
4307-
for (FieldDecl *Field : fields())
4308-
FD = Field;
4309-
return FD;
4310-
}
4311-
const FieldDecl *getLastField() const {
4312-
return const_cast<RecordDecl *>(this)->getLastField();
4313-
}
4314-
4315-
template <typename Functor>
4316-
const FieldDecl *findFieldIf(Functor &Pred) const {
4317-
for (const Decl *D : decls()) {
4318-
if (const auto *FD = dyn_cast<FieldDecl>(D); FD && Pred(FD))
4319-
return FD;
4320-
4321-
if (const auto *RD = dyn_cast<RecordDecl>(D))
4322-
if (const FieldDecl *FD = RD->findFieldIf(Pred))
4323-
return FD;
4324-
}
4325-
4326-
return nullptr;
4327-
}
4328-
43294305
/// Note that the definition of this type is now complete.
43304306
virtual void completeDefinition();
43314307

clang/include/clang/AST/DeclBase.h

-10
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
#include "clang/AST/DeclarationName.h"
1919
#include "clang/Basic/IdentifierTable.h"
2020
#include "clang/Basic/LLVM.h"
21-
#include "clang/Basic/LangOptions.h"
2221
#include "clang/Basic/SourceLocation.h"
2322
#include "clang/Basic/Specifiers.h"
2423
#include "llvm/ADT/ArrayRef.h"
@@ -478,15 +477,6 @@ class alignas(8) Decl {
478477
// Return true if this is a FileContext Decl.
479478
bool isFileContextDecl() const;
480479

481-
/// Whether it resembles a flexible array member. This is a static member
482-
/// because we want to be able to call it with a nullptr. That allows us to
483-
/// perform non-Decl specific checks based on the object's type and strict
484-
/// flex array level.
485-
static bool isFlexibleArrayMemberLike(
486-
ASTContext &Context, const Decl *D, QualType Ty,
487-
LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel,
488-
bool IgnoreTemplateOrMacroSubstitution);
489-
490480
ASTContext &getASTContext() const LLVM_READONLY;
491481

492482
/// Helper to get the language options from the ASTContext.

clang/include/clang/Basic/Attr.td

-18
Original file line numberDiff line numberDiff line change
@@ -4246,21 +4246,3 @@ def AvailableOnlyInDefaultEvalMethod : InheritableAttr {
42464246
let Subjects = SubjectList<[TypedefName], ErrorDiag>;
42474247
let Documentation = [Undocumented];
42484248
}
4249-
4250-
def CountedBy : InheritableAttr {
4251-
let Spellings = [Clang<"counted_by">];
4252-
let Subjects = SubjectList<[Field]>;
4253-
let Args = [IdentifierArgument<"CountedByField">];
4254-
let Documentation = [CountedByDocs];
4255-
let LangOpts = [COnly];
4256-
// FIXME: This is ugly. Let using a DeclArgument would be nice, but a Decl
4257-
// isn't yet available due to the fact that we're still parsing the
4258-
// structure. Maybe that code could be changed sometime in the future.
4259-
code AdditionalMembers = [{
4260-
private:
4261-
SourceRange CountedByFieldLoc;
4262-
public:
4263-
SourceRange getCountedByFieldLoc() const { return CountedByFieldLoc; }
4264-
void setCountedByFieldLoc(SourceRange Loc) { CountedByFieldLoc = Loc; }
4265-
}];
4266-
}

clang/include/clang/Basic/AttrDocs.td

-66
Original file line numberDiff line numberDiff line change
@@ -7275,69 +7275,3 @@ relative ordering of values is important. For example:
72757275
attribute, they default to the value ``65535``.
72767276
}];
72777277
}
7278-
7279-
def CountedByDocs : Documentation {
7280-
let Category = DocCatField;
7281-
let Content = [{
7282-
Clang supports the ``counted_by`` attribute on the flexible array member of a
7283-
structure in C. The argument for the attribute is the name of a field member in
7284-
the same structure holding the count of elements in the flexible array. This
7285-
information can be used to improve the results of the array bound sanitizer and
7286-
the ``__builtin_dynamic_object_size`` builtin.
7287-
7288-
For example, the following code:
7289-
7290-
.. code-block:: c
7291-
7292-
struct bar;
7293-
7294-
struct foo {
7295-
size_t count;
7296-
char other;
7297-
struct bar *array[] __attribute__((counted_by(count)));
7298-
};
7299-
7300-
specifies that the flexible array member ``array`` has the number of elements
7301-
allocated for it stored in ``count``. This establishes a relationship between
7302-
``array`` and ``count``. Specifically, ``p->array`` must have at least
7303-
``p->count`` number of elements available. It's the user's responsibility to
7304-
ensure that this relationship is maintained through changes to the structure.
7305-
7306-
In the following example, the allocated array erroneously has fewer elements
7307-
than what's specified by ``p->count``. This would result in an out-of-bounds
7308-
access not being detected.
7309-
7310-
.. code-block:: c
7311-
7312-
#define SIZE_INCR 42
7313-
7314-
struct foo *p;
7315-
7316-
void foo_alloc(size_t count) {
7317-
p = malloc(MAX(sizeof(struct foo),
7318-
offsetof(struct foo, array[0]) + count * sizeof(struct bar *)));
7319-
p->count = count + SIZE_INCR;
7320-
}
7321-
7322-
The next example updates ``p->count``, breaking the relationship requirement
7323-
that ``p->array`` must have at least ``p->count`` number of elements available:
7324-
7325-
.. code-block:: c
7326-
7327-
#define SIZE_INCR 42
7328-
7329-
struct foo *p;
7330-
7331-
void foo_alloc(size_t count) {
7332-
p = malloc(MAX(sizeof(struct foo),
7333-
offsetof(struct foo, array[0]) + count * sizeof(struct bar *)));
7334-
p->count = count;
7335-
}
7336-
7337-
void use_foo(int index) {
7338-
p->count += SIZE_INCR + 1; /* 'count' is now larger than the number of elements of 'array'. */
7339-
p->array[index] = 0; /* the sanitizer can't properly check if this is an out-of-bounds access. */
7340-
}
7341-
7342-
}];
7343-
}

clang/include/clang/Basic/DiagnosticSemaKinds.td

-15
Original file line numberDiff line numberDiff line change
@@ -6389,21 +6389,6 @@ def warn_superclass_variable_sized_type_not_at_end : Warning<
63896389
"field %0 can overwrite instance variable %1 with variable sized type %2"
63906390
" in superclass %3">, InGroup<ObjCFlexibleArray>;
63916391

6392-
def err_counted_by_attr_not_on_flexible_array_member : Error<
6393-
"'counted_by' only applies to flexible array members">;
6394-
def err_flexible_array_counted_by_attr_field_not_found : Error<
6395-
"field %0 in 'counted_by' not found">;
6396-
def err_flexible_array_counted_by_attr_field_not_found_suggest : Error<
6397-
"field %0 in 'counted_by' not found; did you mean %1?">;
6398-
def err_flexible_array_counted_by_attr_field_not_found_in_struct : Error<
6399-
"field %0 in 'counted_by' is not found in struct">;
6400-
def err_flexible_array_counted_by_attr_refers_to_self : Error<
6401-
"field %0 in 'counted_by' cannot refer to the flexible array">;
6402-
def err_flexible_array_counted_by_attr_field_not_integer : Error<
6403-
"field %0 in 'counted_by' is not a non-boolean integer type">;
6404-
def note_flexible_array_counted_by_attr_field : Note<
6405-
"field %0 declared here">;
6406-
64076392
let CategoryName = "ARC Semantic Issue" in {
64086393

64096394
// ARC-mode diagnostics.

clang/include/clang/Sema/Sema.h

-2
Original file line numberDiff line numberDiff line change
@@ -4795,8 +4795,6 @@ class Sema final {
47954795
bool CheckAlwaysInlineAttr(const Stmt *OrigSt, const Stmt *CurSt,
47964796
const AttributeCommonInfo &A);
47974797

4798-
bool CheckCountedByAttr(Scope *Scope, const FieldDecl *FD);
4799-
48004798
/// Adjust the calling convention of a method to be the ABI default if it
48014799
/// wasn't specified explicitly. This handles method types formed from
48024800
/// function type typedefs and typename template arguments.

clang/lib/AST/ASTImporter.cpp

-13
Original file line numberDiff line numberDiff line change
@@ -8978,10 +8978,6 @@ class AttrImporter {
89788978
public:
89798979
AttrImporter(ASTImporter &I) : Importer(I), NImporter(I) {}
89808980

8981-
// Useful for accessing the imported attribute.
8982-
template <typename T> T *castAttrAs() { return cast<T>(ToAttr); }
8983-
template <typename T> const T *castAttrAs() const { return cast<T>(ToAttr); }
8984-
89858981
// Create an "importer" for an attribute parameter.
89868982
// Result of the 'value()' of that object is to be passed to the function
89878983
// 'importAttr', in the order that is expected by the attribute class.
@@ -9188,15 +9184,6 @@ Expected<Attr *> ASTImporter::Import(const Attr *FromAttr) {
91889184
From->args_size());
91899185
break;
91909186
}
9191-
case attr::CountedBy: {
9192-
AI.cloneAttr(FromAttr);
9193-
const auto *CBA = cast<CountedByAttr>(FromAttr);
9194-
Expected<SourceRange> SR = Import(CBA->getCountedByFieldLoc()).get();
9195-
if (!SR)
9196-
return SR.takeError();
9197-
AI.castAttrAs<CountedByAttr>()->setCountedByFieldLoc(SR.get());
9198-
break;
9199-
}
92009187

92019188
default: {
92029189
// The default branch works for attributes that have no arguments to import.

clang/lib/AST/DeclBase.cpp

+1-75
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "clang/AST/Type.h"
3030
#include "clang/Basic/IdentifierTable.h"
3131
#include "clang/Basic/LLVM.h"
32+
#include "clang/Basic/LangOptions.h"
3233
#include "clang/Basic/Module.h"
3334
#include "clang/Basic/ObjCRuntime.h"
3435
#include "clang/Basic/PartialDiagnostic.h"
@@ -410,81 +411,6 @@ bool Decl::isFileContextDecl() const {
410411
return DC && DC->isFileContext();
411412
}
412413

413-
bool Decl::isFlexibleArrayMemberLike(
414-
ASTContext &Ctx, const Decl *D, QualType Ty,
415-
LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel,
416-
bool IgnoreTemplateOrMacroSubstitution) {
417-
// For compatibility with existing code, we treat arrays of length 0 or
418-
// 1 as flexible array members.
419-
const auto *CAT = Ctx.getAsConstantArrayType(Ty);
420-
if (CAT) {
421-
using FAMKind = LangOptions::StrictFlexArraysLevelKind;
422-
423-
llvm::APInt Size = CAT->getSize();
424-
FAMKind StrictFlexArraysLevel =
425-
Ctx.getLangOpts().getStrictFlexArraysLevel();
426-
427-
if (StrictFlexArraysLevel == FAMKind::IncompleteOnly)
428-
return false;
429-
430-
// GCC extension, only allowed to represent a FAM.
431-
if (Size.isZero())
432-
return true;
433-
434-
if (StrictFlexArraysLevel == FAMKind::ZeroOrIncomplete && Size.uge(1))
435-
return false;
436-
437-
if (StrictFlexArraysLevel == FAMKind::OneZeroOrIncomplete && Size.uge(2))
438-
return false;
439-
} else if (!Ctx.getAsIncompleteArrayType(Ty)) {
440-
return false;
441-
}
442-
443-
if (const auto *OID = dyn_cast_if_present<ObjCIvarDecl>(D))
444-
return OID->getNextIvar() == nullptr;
445-
446-
const auto *FD = dyn_cast_if_present<FieldDecl>(D);
447-
if (!FD)
448-
return false;
449-
450-
if (CAT) {
451-
// GCC treats an array memeber of a union as an FAM if the size is one or
452-
// zero.
453-
llvm::APInt Size = CAT->getSize();
454-
if (FD->getParent()->isUnion() && (Size.isZero() || Size.isOne()))
455-
return true;
456-
}
457-
458-
// Don't consider sizes resulting from macro expansions or template argument
459-
// substitution to form C89 tail-padded arrays.
460-
if (IgnoreTemplateOrMacroSubstitution) {
461-
TypeSourceInfo *TInfo = FD->getTypeSourceInfo();
462-
while (TInfo) {
463-
TypeLoc TL = TInfo->getTypeLoc();
464-
465-
// Look through typedefs.
466-
if (TypedefTypeLoc TTL = TL.getAsAdjusted<TypedefTypeLoc>()) {
467-
const TypedefNameDecl *TDL = TTL.getTypedefNameDecl();
468-
TInfo = TDL->getTypeSourceInfo();
469-
continue;
470-
}
471-
472-
if (auto CTL = TL.getAs<ConstantArrayTypeLoc>()) {
473-
const Expr *SizeExpr = dyn_cast<IntegerLiteral>(CTL.getSizeExpr());
474-
if (!SizeExpr || SizeExpr->getExprLoc().isMacroID())
475-
return false;
476-
}
477-
478-
break;
479-
}
480-
}
481-
482-
// Test that the field is the last in the structure.
483-
RecordDecl::field_iterator FI(
484-
DeclContext::decl_iterator(const_cast<FieldDecl *>(FD)));
485-
return ++FI == FD->getParent()->field_end();
486-
}
487-
488414
TranslationUnitDecl *Decl::getTranslationUnitDecl() {
489415
if (auto *TUD = dyn_cast<TranslationUnitDecl>(this))
490416
return TUD;

0 commit comments

Comments
 (0)