delcypher wrote:
I'll put up a new version of this PR with the memory leak fixed soon.
https://github.com/llvm/llvm-project/pull/90786
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
delcypher wrote:
Ok. Now I see what's happening.
These lines here are basically giving ownership of `LateParsedAttribute` to the
`LateParsedAttrList`
```
// Handle attributes with arguments that require late parsing.
LateParsedAttribute *LA =
new
delcypher wrote:
The leak via `clang::Parser::ParseLexedCAttribute` is
```c++
LA.Toks.push_back(AttrEnd);
```
and the leak via `clang::Parser::ParseGNUAttributes`
is
```
LateParsedAttribute *LA =
new LateParsedAttribute(this, *AttrName, AttrNameLoc);
```
which is really
delcypher wrote:
Hmm. Apparently there's a memory leak.
https://lab.llvm.org/buildbot/#/builders/239/builds/7043
```
-- Testing: 79948 of 79949 tests, 48 workers --
Testing:
FAIL: Clang :: AST/attr-counted-by-late-parsed-struct-ptrs.c (480 of 79948)
TEST 'Clang ::
bwendling wrote:
Thank you. I wrote to the author. I hope he'll be able to come up with a change
on his end. Or at least an explanation that makes sense :-)
https://github.com/llvm/llvm-project/pull/90786
___
cfe-commits mailing list
delcypher wrote:
@kees @bwendling @rapidsna The workaround to downgrade this error to a warning
has landed
https://github.com/llvm/llvm-project/commit/cef6387e52578366c2332275dad88b9953b55336
https://github.com/llvm/llvm-project/pull/90786
___
bwendling wrote:
Ah! I see what you mean. I'll bring this up with the developer. (Actually, that
construct makes me nervous about their code in general...)
https://github.com/llvm/llvm-project/pull/90786
___
cfe-commits mailing list
delcypher wrote:
@bwendling This is unfortunate
```
drivers/gpu/drm/radeon/pptable.h:442:5: error: 'counted_by' cannot be applied
to an array with element of unknown size because 'ATOM_PPLIB_STATE_V2' (aka
'struct _ATOM_PPLIB_STATE_V2') is a struct type with a flexible array member
442 |
rapidsna wrote:
@bwendling @kees Likely, we should not put `__counted_by` in that case. Could
we fix the source?
https://github.com/llvm/llvm-project/pull/90786
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
rapidsna wrote:
@bwendling @kees Wait. `ATOM_PPLIB_STATE_V2` is also a struct with flexible
array member? This is concerning because `ucNumEntries *
sizeof(ATOM_PPLIB_STATE_V2)` is not the correct size anyway. Do you know the
semantics of this structure?
rapidsna wrote:
@bwendling Thanks for reporting. We will relax the restrictions for arrays to
not break the existing users.
https://github.com/llvm/llvm-project/pull/90786
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
bwendling wrote:
This seems to have broken the Linux build:
https://github.com/llvm/llvm-project/commit/0ec3b972e58bcbcdc1bebe1696ea37f2931287c3
breaks the build for Linux, added by
https://git.kernel.org/linus/781d41fed19caf900c8405064676813dc9921d32:
delcypher wrote:
Test fixed by `112eadd55f06bee15caadff688ea0b45acbfa804`.
https://github.com/llvm/llvm-project/pull/90786
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
delcypher wrote:
Looks like I broke the
`clang/test/Misc/pragma-attribute-supported-attributes-list.test` test. I'll
push a follow up fix to that test once I've confirmed I've fixed it.
https://github.com/llvm/llvm-project/pull/90786
___
cfe-commits
https://github.com/delcypher closed
https://github.com/llvm/llvm-project/pull/90786
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/delcypher updated
https://github.com/llvm/llvm-project/pull/90786
>From 80dbab4c4b43eb78f29b7b8fa577f04772a7f52c Mon Sep 17 00:00:00 2001
From: Dan Liew
Date: Wed, 1 May 2024 13:56:52 -0700
Subject: [PATCH] [BoundsSafety] Allow 'counted_by' attribute on pointers in
structs
delcypher wrote:
@kees Thanks for approving.
I'm going to resolve the merge conflict in `clang/docs/ReleaseNotes.rst` and
then merge. I'll start looking at supporting `__counted_by()` on incomplete
pointee types next. @hnrklssn is going to start working on upstreaming the
`__sized_by`
https://github.com/delcypher updated
https://github.com/llvm/llvm-project/pull/90786
>From e6fb7a3374ada3d02b4c89263ffd14a037a7e56a Mon Sep 17 00:00:00 2001
From: Dan Liew
Date: Wed, 1 May 2024 13:56:52 -0700
Subject: [PATCH] [BoundsSafety] Allow 'counted_by' attribute on pointers in
structs
https://github.com/kees approved this pull request.
Thanks for the updates! Let's get this in and continue with the rest of the
support. :)
https://github.com/llvm/llvm-project/pull/90786
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
delcypher wrote:
@bwendling @kees Any further feedback? If not, can you approve?
As @rapidsna said we'll follow up this PR with additional PRs to address the
two major concerns you had.
https://github.com/llvm/llvm-project/pull/90786
___
cfe-commits
@@ -0,0 +1,187 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#define __counted_by(f) __attribute__((counted_by(f)))
+
+struct bar;
+
+struct not_found {
+ int count;
+ struct bar *fam[] __counted_by(bork); // expected-error {{use of undeclared
identifier 'bork'}}
+};
+
https://github.com/delcypher updated
https://github.com/llvm/llvm-project/pull/90786
>From 1f4d924768409d6bc61d160c6161e6acebf62b60 Mon Sep 17 00:00:00 2001
From: Dan Liew
Date: Wed, 1 May 2024 13:56:52 -0700
Subject: [PATCH 1/4] [BoundsSafety] Allow 'counted_by' attribute on pointers
in
bwendling wrote:
> > It's not a lie, because the contents of a pointer don't contribute to the
> > size of the struct containing that pointer.
>
> Consider this example. It tries to illustrate why putting `__counted_by()` on
> a pointer to a structs containing flexible array members doesn't
rapidsna wrote:
> The main concern I have with delaying support for this is that header files
> could find themselves in a state where they could not be refactored without
> removing counted_by attributes that refer to now-incomplete structs.
@kees Agreed. We will work on a follow up patch to
@@ -0,0 +1,187 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#define __counted_by(f) __attribute__((counted_by(f)))
+
+struct bar;
+
+struct not_found {
+ int count;
+ struct bar *fam[] __counted_by(bork); // expected-error {{use of undeclared
identifier 'bork'}}
+};
+
kees wrote:
> Consider this example. It tries to illustrate why putting `__counted_by()` on
> a pointer to a structs containing flexible array members doesn't make sense.
>
> ```c
> struct HasFAM {
> int count;
> char buffer[] __counted_by(count); // This is OK
> };
>
> struct
delcypher wrote:
> It's not a lie, because the contents of a pointer don't contribute to the
> size of the struct containing that pointer.
Consider this example. It tries to illustrate why putting `__counted_by()` on a
pointer to a structs containing flexible array members doesn't make sense.
kees wrote:
> As @apple-fcloutier @rapidsna noted this is how `-fbounds-safety` is
> currently implemented (because its much simpler) but it is a restriction that
> could be lifted in future by only requiring `struct bar` to be defined at the
> point that `foo::bar` is used rather than when
delcypher wrote:
> @rapidsna @delcypher @apple-fcloutier @kees:
>
> Okay, I think I see what the complication is. Are you trying to prevent the
> use case of someone writing something like:
>
> ```c
> struct bar;
>
> struct foo {
> size_t count;
> struct bar *ptr __counted_by(count);
>
bwendling wrote:
@rapidsna @delcypher @apple-fcloutier @kees:
Okay, I think I see what the complication is. Are you trying to prevent the use
case of someone writing something like:
```c
struct bar;
struct foo {
size_t count;
struct bar *ptr __counted_by(count);
};
```
where `ptr` is a
bwendling wrote:
@apple-fcloutier:
> think that there's room to allow `__counted_by` on incomplete types so that a
> TU where it's complete could use it (and we have use cases where that would
> be handy), but our implementation doesn't support it at this time. This can
> be added without
rapidsna wrote:
> I've been thinking about this restriction. Why is this necessary? My
> assumption was that applying counted_by to a pointer causes a bounds check on
> an index into the pointer rather than its underlying type.
@bwendling It's because these types are not indexable really.
delcypher wrote:
@bwendling
> I've been thinking about this restriction. Why is this necessary? My
> assumption was that applying counted_by to a pointer causes a bounds check on
> an index into the pointer rather than its underlying type.
@rapidsna Please add additional points if I
apple-fcloutier wrote:
I think that there's room to allow `__counted_by` on incomplete types so that a
TU where it's complete could use it (and we have use cases where that would be
handy), but our implementation doesn't support it at this time. This can be
added without disruptions at a
bwendling wrote:
> 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
@@ -335,6 +336,22 @@ Attribute Changes in Clang
- Clang now warns that the ``exclude_from_explicit_instantiation`` attribute
is ignored when applied to a local class or a member thereof.
+- The ``counted_by`` attribute can now be late parsed in C when
@@ -335,6 +336,22 @@ Attribute Changes in Clang
- Clang now warns that the ``exclude_from_explicit_instantiation`` attribute
is ignored when applied to a local class or a member thereof.
+- The ``counted_by`` attribute can now be late parsed in C when
@@ -6534,6 +6536,15 @@ def err_counted_by_attr_refer_to_union : Error<
"'counted_by' argument cannot refer to a union member">;
def note_flexible_array_counted_by_attr_field : Note<
"field %0 declared here">;
+def err_counted_by_attr_pointee_unknown_size : Error<
+
@@ -631,6 +631,18 @@ bool Type::isStructureType() const {
return false;
}
+bool Type::isStructureTypeWithFlexibleArrayMember() const {
+ const auto *RT = getAs();
+ if (!RT)
+return false;
+ const auto *Decl = RT->getDecl();
+ if (!Decl->isStruct())
+return
@@ -6534,6 +6536,15 @@ def err_counted_by_attr_refer_to_union : Error<
"'counted_by' argument cannot refer to a union member">;
def note_flexible_array_counted_by_attr_field : Note<
"field %0 declared here">;
+def err_counted_by_attr_pointee_unknown_size : Error<
+
@@ -6534,6 +6536,15 @@ def err_counted_by_attr_refer_to_union : Error<
"'counted_by' argument cannot refer to a union member">;
def note_flexible_array_counted_by_attr_field : Note<
"field %0 declared here">;
+def err_counted_by_attr_pointee_unknown_size : Error<
+
@@ -335,6 +336,22 @@ Attribute Changes in Clang
- Clang now warns that the ``exclude_from_explicit_instantiation`` attribute
is ignored when applied to a local class or a member thereof.
+- The ``counted_by`` attribute can now be late parsed in C when
https://github.com/delcypher updated
https://github.com/llvm/llvm-project/pull/90786
>From 1f4d924768409d6bc61d160c6161e6acebf62b60 Mon Sep 17 00:00:00 2001
From: Dan Liew
Date: Wed, 1 May 2024 13:56:52 -0700
Subject: [PATCH 1/3] [BoundsSafety] Allow 'counted_by' attribute on pointers
in
@@ -631,6 +631,18 @@ bool Type::isStructureType() const {
return false;
}
+bool Type::isStructureTypeWithFlexibleArrayMember() const {
+ const auto *RT = getAs();
+ if (!RT)
+return false;
+ const auto *Decl = RT->getDecl();
+ if (!Decl->isStruct())
+return
@@ -6534,6 +6536,15 @@ def err_counted_by_attr_refer_to_union : Error<
"'counted_by' argument cannot refer to a union member">;
def note_flexible_array_counted_by_attr_field : Note<
"field %0 declared here">;
+def err_counted_by_attr_pointee_unknown_size : Error<
+
@@ -335,6 +336,22 @@ Attribute Changes in Clang
- Clang now warns that the ``exclude_from_explicit_instantiation`` attribute
is ignored when applied to a local class or a member thereof.
+- The ``counted_by`` attribute can now be late parsed in C when
https://github.com/delcypher updated
https://github.com/llvm/llvm-project/pull/90786
>From 1f4d924768409d6bc61d160c6161e6acebf62b60 Mon Sep 17 00:00:00 2001
From: Dan Liew
Date: Wed, 1 May 2024 13:56:52 -0700
Subject: [PATCH 1/2] [BoundsSafety] Allow 'counted_by' attribute on pointers
in
https://github.com/bwendling edited
https://github.com/llvm/llvm-project/pull/90786
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -3282,6 +3282,19 @@ void Parser::ParseAlignmentSpecifier(ParsedAttributes
,
}
}
+void Parser::DistributeCLateParsedAttrs(Decl *Dcl,
+LateParsedAttrList *LateAttrs) {
+ assert(Dcl);
bwendling wrote:
Could you add
@@ -8588,31 +8588,71 @@ static const RecordDecl
*GetEnclosingNamedOrTopAnonRecord(const FieldDecl *FD) {
return RD;
}
-static bool
-CheckCountExpr(Sema , FieldDecl *FD, Expr *E,
- llvm::SmallVectorImpl ) {
+enum class CountedByInvalidPointeeTypeKind {
+
@@ -631,6 +631,18 @@ bool Type::isStructureType() const {
return false;
}
+bool Type::isStructureTypeWithFlexibleArrayMember() const {
+ const auto *RT = getAs();
+ if (!RT)
+return false;
+ const auto *Decl = RT->getDecl();
+ if (!Decl->isStruct())
+return
@@ -6534,6 +6536,15 @@ def err_counted_by_attr_refer_to_union : Error<
"'counted_by' argument cannot refer to a union member">;
def note_flexible_array_counted_by_attr_field : Note<
"field %0 declared here">;
+def err_counted_by_attr_pointee_unknown_size : Error<
+
@@ -335,6 +336,22 @@ Attribute Changes in Clang
- Clang now warns that the ``exclude_from_explicit_instantiation`` attribute
is ignored when applied to a local class or a member thereof.
+- The ``counted_by`` attribute can now be late parsed in C when
@@ -8588,31 +8588,71 @@ static const RecordDecl
*GetEnclosingNamedOrTopAnonRecord(const FieldDecl *FD) {
return RD;
}
-static bool
-CheckCountExpr(Sema , FieldDecl *FD, Expr *E,
- llvm::SmallVectorImpl ) {
+enum class CountedByInvalidPointeeTypeKind {
+
@@ -8588,31 +8588,71 @@ static const RecordDecl
*GetEnclosingNamedOrTopAnonRecord(const FieldDecl *FD) {
return RD;
}
-static bool
-CheckCountExpr(Sema , FieldDecl *FD, Expr *E,
- llvm::SmallVectorImpl ) {
+enum class CountedByInvalidPointeeTypeKind {
+
https://github.com/Endilll commented:
`Sema.h` changes look good to me.
https://github.com/llvm/llvm-project/pull/90786
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Dan Liew (delcypher)
Changes
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
https://github.com/delcypher created
https://github.com/llvm/llvm-project/pull/90786
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
58 matches
Mail list logo
