[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits

https://github.com/delcypher closed 
https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits

https://github.com/delcypher updated 
https://github.com/llvm/llvm-project/pull/93121

>From c1f0914b1f8ae8416cebdf7f0573b4ff11f6110d Mon Sep 17 00:00:00 2001
From: Dan Liew 
Date: Fri, 17 May 2024 12:07:40 -0700
Subject: [PATCH] [BoundsSafety] Reland #93121 Allow 'counted_by' attribute on
 pointers in structs in C (#93121)

Fixes #92687.

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.

There are also several other follow up changes that will need to be
addressed in future patches:

* Make late parsing working with anonymous structs (see
`on_pointer_anon_buf` in `attr-counted-by-late-parsed-struct-ptrs.c`).
* Allow `counted_by` on more subjects (e.g. parameters, returns types)
when `-fbounds-safety` is enabled.
* Make use of the attribute on pointer types in code gen (e.g. for
`_builtin_dynamic_object_size` and UBSan's array-bounds checks).

This work is heavily based on a patch originally written by Yeoul Na.

** Differences between #93121 and this patch **

* The memory leak that caused #93121 to be reverted (see #92687) should
  now be fixed. See "The Memory Leak".
* The fix to `pragma-attribute-supported-attributes-list.test`
  (originally in cef6387) has been incorporated into this patch.
* A relaxation of counted_by semantics (originally in 112eadd) has been
  incorporated into this patch.
* The assert in `Parser::DistributeCLateParsedAttrs` has been removed
  because that broke downstream code.
* The switch statement in `Parser::ParseLexedCAttribute` has been
  removed in favor of using `Parser::ParseGNUAttributeArgs` which does
  the same thing but is more feature complete.
* The `EnterScope` parameter has been plumbed through
  `Parser::ParseLexedCAttribute` and `Parser::ParseLexedCAttributeList`.
  It currently doesn't do anything but it will be needed in future
  commits.

** The Memory Leak **

The problem was that these lines parsed the attributes but then did nothing to 
free the memory

```
assert(!getLangOpts().CPlusPlus);
for (auto *LateAttr : LateFieldAttrs)
  ParseLexedCAttribute(*LateAttr);
```

To fix this this a new `Parser::ParseLexedCAttributeList` method has been
added (based on `Parser::ParseLexedAttributeList`) which does the
necessary memory management. The intention is to merge these two
methods together so there is just one implementation in a future patch
(#93263).

A more principled fixed here would be to fix the ownership of the
`LateParsedAttribute` objects. In principle `LateParsedAttrList` should own
its pointers exclusively and be responsible for deallocating them.
Unfortunately this is complicated by `LateParsedAttribute` objects also
being stored in another data structure (`LateParsedDeclarations`) as
can be seen below (`LA` gets stored in two places).

```
  // Handle attributes with arguments that require late parsing.
  LateParsedAttribute *LA =
  new LateParsedAttribute(this, *AttrName, AttrNameLoc);
  LateAttrs->push_back(LA);

  // Attributes in a class are parsed at the end of the class, along
  // with other late-parsed declarations.
  if (!ClassStack.empty() && !LateAttrs->parseSoon())
   

[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits

delcypher wrote:

Filed to #93263 to track the potential refactor.

https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits

https://github.com/delcypher updated 
https://github.com/llvm/llvm-project/pull/93121

>From 9e1a9e49b33d4b77d75cee243244dd66a40ea209 Mon Sep 17 00:00:00 2001
From: Dan Liew 
Date: Fri, 17 May 2024 12:07:40 -0700
Subject: [PATCH] [BoundsSafety] Reland #93121 Allow 'counted_by' attribute on
 pointers in structs in C (#93121)

Fixes #92687.

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.

There are also several other follow up changes that will need to be
addressed in future patches:

* Make late parsing working with anonymous structs (see
`on_pointer_anon_buf` in `attr-counted-by-late-parsed-struct-ptrs.c`).
* Allow `counted_by` on more subjects (e.g. parameters, returns types)
when `-fbounds-safety` is enabled.
* Make use of the attribute on pointer types in code gen (e.g. for
`_builtin_dynamic_object_size` and UBSan's array-bounds checks).

This work is heavily based on a patch originally written by Yeoul Na.

** Differences between #93121 and this patch **

* The memory leak that caused #93121 to be reverted (see #92687) should
  now be fixed. See "The Memory Leak".
* The fix to `pragma-attribute-supported-attributes-list.test`
  (originally in cef6387) has been incorporated into this patch.
* A relaxation of counted_by semantics (originally in 112eadd) has been
  incorporated into this patch.
* The assert in `Parser::DistributeCLateParsedAttrs` has been removed
  because that broke downstream code.
* The switch statement in `Parser::ParseLexedCAttribute` has been
  removed in favor of using `Parser::ParseGNUAttributeArgs` which does
  the same thing but is more feature complete.
* The `EnterScope` parameter has been plumbed through
  `Parser::ParseLexedCAttribute` and `Parser::ParseLexedCAttributeList`.
  It currently doesn't do anything but it will be needed in future
  commits.

** The Memory Leak **

The problem was that these lines parsed the attributes but then did nothing to 
free the memory

```
assert(!getLangOpts().CPlusPlus);
for (auto *LateAttr : LateFieldAttrs)
  ParseLexedCAttribute(*LateAttr);
```

To fix this this a new `Parser::ParseLexedCAttributeList` method has been
added (based on `Parser::ParseLexedAttributeList`) which does the
necessary memory management. The intention is to merge these two
methods together so there is just one implementation in a future patch.

A more principled fixed here would be to fix the ownership of the
`LateParsedAttribute` objects. In principle `LateParsedAttrList` should own
its pointers exclusively and be responsible for deallocating them.
Unfortunately this is complicated by `LateParsedAttribute` objects also
being stored in another data structure (`LateParsedDeclarations`) as
can be seen below (`LA` gets stored in two places).

```
  // Handle attributes with arguments that require late parsing.
  LateParsedAttribute *LA =
  new LateParsedAttribute(this, *AttrName, AttrNameLoc);
  LateAttrs->push_back(LA);

  // Attributes in a class are parsed at the end of the class, along
  // with other late-parsed declarations.
  if (!ClassStack.empty() && !LateAttrs->parseSoon())

[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits

https://github.com/delcypher updated 
https://github.com/llvm/llvm-project/pull/93121

>From f9c8abdb243096b6fe0c2ad1a42023039ca7f1f2 Mon Sep 17 00:00:00 2001
From: Dan Liew 
Date: Fri, 17 May 2024 12:07:40 -0700
Subject: [PATCH] [BoundsSafety] Reland #93121 Allow 'counted_by' attribute on
 pointers in structs in C (#93121)

Fixes #92687.

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.

There are also several other follow up changes that will need to be
addressed in future patches:

* Make late parsing working with anonymous structs (see
`on_pointer_anon_buf` in `attr-counted-by-late-parsed-struct-ptrs.c`).
* Allow `counted_by` on more subjects (e.g. parameters, returns types)
when `-fbounds-safety` is enabled.
* Make use of the attribute on pointer types in code gen (e.g. for
`_builtin_dynamic_object_size` and UBSan's array-bounds checks).

This work is heavily based on a patch originally written by Yeoul Na.

** Differences between #93121 and this patch **

* The memory leak that caused #93121 to be reverted (see #92687) should
  now be fixed. See "The Memory Leak".
* The fix to `pragma-attribute-supported-attributes-list.test`
  (originally in cef6387) has been incorporated into this patch.
* A relaxation of counted_by semantics (originally in 112eadd) has been
  incorporated into this patch.
* The assert in `Parser::DistributeCLateParsedAttrs` has been removed
  because that broke downstream code.
* The switch statement in `Parser::ParseLexedCAttribute` has been
  removed in favor of using `Parser::ParseGNUAttributeArgs` which does
  the same thing but is more feature complete.
* The `EnterScope` parameter has been plumbed through
  `Parser::ParseLexedCAttribute` and `Parser::ParseLexedCAttributeList`.
  It currently doesn't do anything but it will be needed in future
  commits.

** The Memory Leak **

The problem was that these lines parsed the attributes but then did nothing to 
free the memory

```
assert(!getLangOpts().CPlusPlus);
for (auto *LateAttr : LateFieldAttrs)
  ParseLexedCAttribute(*LateAttr);
```

To fix this this a new `Parser::ParseLexedCAttributeList` method has been
added (based on Parser::ParseLexedAttributeList) which does the
necessary memory management. The intention is to merge these two
methods together so there is just one implementation in a future patch.

A more principled fixed here would be to fix the ownership of the
LateParsedAttribute objects. In principle LateParsedAttrList should own
its pointers exclusively and be responsible for deallocating them.
Unfortunately this is complicated by LateParsedAttribute objects also
being stored in another data structure (LateParsedDeclarations) as
can be seen below (`LA` gets stored in two places).

```
  // Handle attributes with arguments that require late parsing.
  LateParsedAttribute *LA =
  new LateParsedAttribute(this, *AttrName, AttrNameLoc);
  LateAttrs->push_back(LA);

  // Attributes in a class are parsed at the end of the class, along
  // with other late-parsed declarations.
  if (!ClassStack.empty() && !LateAttrs->parseSoon())

[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits

https://github.com/delcypher updated 
https://github.com/llvm/llvm-project/pull/93121

>From bc5b8bc34b50e2e98d2c6103c7b1c232a3e765b4 Mon Sep 17 00:00:00 2001
From: Dan Liew 
Date: Fri, 17 May 2024 12:07:40 -0700
Subject: [PATCH] [BoundsSafety] Reland #93121 Allow 'counted_by' attribute on
 pointers in structs in C (#93121)

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.

There are also several other follow up changes that will need to be
addressed in future patches:

* Make late parsing working with anonymous structs (see
`on_pointer_anon_buf` in `attr-counted-by-late-parsed-struct-ptrs.c`).
* Allow `counted_by` on more subjects (e.g. parameters, returns types)
when `-fbounds-safety` is enabled.
* Make use of the attribute on pointer types in code gen (e.g. for
`_builtin_dynamic_object_size` and UBSan's array-bounds checks).

This work is heavily based on a patch originally written by Yeoul Na.

** Differences between #93121 and this patch **

* The memory leak that caused #93121 to be reverted (see #92687) should now be 
fixed
* The fix to `pragma-attribute-supported-attributes-list.test` (originally in 
cef6387) has been incorporated into this patch
* A relaxation of counted_by semantics (originally in 112eadd) has been
  incorporated into this patch
* The assert in `Parser::DistributeCLateParsedAttrs` has been removed because 
that broke
  downstream code
* The switch statement in `Parser::ParseLexedCAttribute` has been
  removed in favor of using `Parser::ParseGNUAttributeArgs` which
  does the same thing but is more feature complete.
* The `EnterScope` parameter has been plumbed through
  `Parser::ParseLexedCAttribute` and `Parser::ParseLexedCAttributeList`.
  It currently doesn't do anything but it will be needed in future
  commits.

rdar://125400257
---
 clang/docs/ReleaseNotes.rst   |  21 +-
 clang/include/clang/AST/Type.h|   1 +
 clang/include/clang/Basic/Attr.td |   3 +-
 clang/include/clang/Basic/DiagnosticGroups.td |   4 +
 .../clang/Basic/DiagnosticSemaKinds.td|  23 +-
 clang/include/clang/Parse/Parser.h|   9 +-
 clang/include/clang/Sema/Sema.h   |   3 +-
 clang/lib/AST/Type.cpp|  10 +
 clang/lib/Parse/ParseDecl.cpp | 106 +++-
 clang/lib/Parse/ParseObjc.cpp |  10 +-
 clang/lib/Sema/SemaDeclAttr.cpp   |  95 +--
 clang/lib/Sema/SemaType.cpp   |   6 +-
 clang/lib/Sema/TreeTransform.h|   2 +-
 .../attr-counted-by-late-parsed-struct-ptrs.c |  45 
 clang/test/AST/attr-counted-by-struct-ptrs.c  | 117 
 ...a-attribute-supported-attributes-list.test |   1 -
 .../Sema/attr-counted-by-late-parsed-off.c|  26 ++
 .../attr-counted-by-late-parsed-struct-ptrs.c | 254 ++
 ...tr-counted-by-struct-ptrs-sizeless-types.c |  17 ++
 clang/test/Sema/attr-counted-by-struct-ptrs.c | 224 +++
 .../Sema/attr-counted-by-vla-sizeless-types.c |  11 +
 clang/test/Sema/attr-counted-by-vla.c | 196 ++
 clang/test/Sema/attr-counted-by.c  

[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Henrik G. Olsson via cfe-commits


@@ -4944,6 +4944,26 @@ void Parser::ParseStructDeclaration(
   }
 }
 
+// TODO: All callers of this function should be moved to
+// `Parser::ParseLexedAttributeList`.
+void Parser::ParseLexedCAttributeList(LateParsedAttrList , bool EnterScope,
+  ParsedAttributes *OutAttrs) {
+  assert(LAs.parseSoon() &&
+ "Attribute list should be marked for immediate parsing.");
+#ifndef NDEBUG
+  auto LangStd = getLangOpts().LangStd;
+  if (LangStd != LangStandard::lang_unspecified) {
+auto Lang = LangStandard::getLangStandardForKind(LangStd).getLanguage();
+assert(Lang == Language::C || Lang == Language::OpenCL);

hnrklssn wrote:

>  However, I think I'll remove this assert for now because I intend (provided 
> I can make it to work with our internal code) to remove this code entirely in 
> favor of re-using the late parsing code that's used for C++

Ah so that's the same thing I suggested then. I thought you meant to just 
remove the assert. Overall I think this is a good idea  

https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits


@@ -4944,6 +4944,26 @@ void Parser::ParseStructDeclaration(
   }
 }
 
+// TODO: All callers of this function should be moved to
+// `Parser::ParseLexedAttributeList`.
+void Parser::ParseLexedCAttributeList(LateParsedAttrList , bool EnterScope,
+  ParsedAttributes *OutAttrs) {
+  assert(LAs.parseSoon() &&
+ "Attribute list should be marked for immediate parsing.");
+#ifndef NDEBUG
+  auto LangStd = getLangOpts().LangStd;
+  if (LangStd != LangStandard::lang_unspecified) {
+auto Lang = LangStandard::getLangStandardForKind(LangStd).getLanguage();
+assert(Lang == Language::C || Lang == Language::OpenCL);

delcypher wrote:

So parsing a `.m` file without specifying a language seems to result in

```
(lldb) p LangOpts.LangStd
(clang::LangStandard::Kind) lang_gnu11
(lldb) p LangOpts.ObjC
(unsigned int) 1
(lldb) p LangOpts.C99
(unsigned int) 1
(lldb) p LangOpts.C11
(unsigned int) 1
(lldb) p LangOpts.C17
(unsigned int) 0
(lldb) p LangOpts.C23
(unsigned int) 0
(lldb) p LangOpts.CPlusPlus
(unsigned int) 0
(lldb) p 
clang::LangStandard::getLangStandardForKind(LangOpts.LangStd).getLanguage()
(clang::Language) C
```

So my assert doesn't do the right thing for Objective-C. For Objective-C my 
assert thinks its C 臘‍♂️. So it seems like my assert would want to be 
`assert(!getLangOpts().CPlusPlus && !getLangOpts().ObjC)`. However, I think 
I'll remove this assert for now because I intend (provided I can make it to 
work with our internal code) to remove this code entirely in favor of re-using 
the late parsing code that's used for C++.

https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Henrik G. Olsson via cfe-commits


@@ -4944,6 +4944,26 @@ void Parser::ParseStructDeclaration(
   }
 }
 
+// TODO: All callers of this function should be moved to
+// `Parser::ParseLexedAttributeList`.
+void Parser::ParseLexedCAttributeList(LateParsedAttrList , bool EnterScope,
+  ParsedAttributes *OutAttrs) {
+  assert(LAs.parseSoon() &&
+ "Attribute list should be marked for immediate parsing.");
+#ifndef NDEBUG
+  auto LangStd = getLangOpts().LangStd;
+  if (LangStd != LangStandard::lang_unspecified) {
+auto Lang = LangStandard::getLangStandardForKind(LangStd).getLanguage();
+assert(Lang == Language::C || Lang == Language::OpenCL);

hnrklssn wrote:

> In general I think the approach of thinking that this function should be for 
> C only is actually flawed and I'm going to look into removing this entirely 
> after landing this.

At that point, why not just call `ParseLexedAttributeList` and add a non-C++ 
case to `ParseLexedAttribute` instead of creating separate C versions that seem 
to serve basically the same purpose?

https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits


@@ -4944,6 +4944,26 @@ void Parser::ParseStructDeclaration(
   }
 }
 
+// TODO: All callers of this function should be moved to
+// `Parser::ParseLexedAttributeList`.
+void Parser::ParseLexedCAttributeList(LateParsedAttrList , bool EnterScope,
+  ParsedAttributes *OutAttrs) {
+  assert(LAs.parseSoon() &&
+ "Attribute list should be marked for immediate parsing.");
+#ifndef NDEBUG
+  auto LangStd = getLangOpts().LangStd;
+  if (LangStd != LangStandard::lang_unspecified) {
+auto Lang = LangStandard::getLangStandardForKind(LangStd).getLanguage();
+assert(Lang == Language::C || Lang == Language::OpenCL);

delcypher wrote:

> Isn't it? I think getLangOpts().CPlusPlus should set true when it is 
> Objective-C++.
I'm not sure if that's the case but I'll check. But what about "Objective-C"? I 
didn't want this path taken for that so `!getLangOpts().CPlusPlus` would be 
true for Objective-C.

In general I think the approach of thinking that this function should be for C 
only is actually flawed and I'm going to look into removing this entirely after 
landing this.

https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Yeoul Na via cfe-commits

https://github.com/rapidsna edited 
https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Yeoul Na via cfe-commits


@@ -4944,6 +4944,26 @@ void Parser::ParseStructDeclaration(
   }
 }
 
+// TODO: All callers of this function should be moved to
+// `Parser::ParseLexedAttributeList`.
+void Parser::ParseLexedCAttributeList(LateParsedAttrList , bool EnterScope,
+  ParsedAttributes *OutAttrs) {
+  assert(LAs.parseSoon() &&
+ "Attribute list should be marked for immediate parsing.");
+#ifndef NDEBUG
+  auto LangStd = getLangOpts().LangStd;
+  if (LangStd != LangStandard::lang_unspecified) {
+auto Lang = LangStandard::getLangStandardForKind(LangStd).getLanguage();
+assert(Lang == Language::C || Lang == Language::OpenCL);

rapidsna wrote:

> That doesn't sound equivalent. The language could also be objective-C or 
> objective-C++.
I think `getLangOpts().CPlusPlus` should set true when it is `Objective-C++`.

https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits


@@ -4944,6 +4944,26 @@ void Parser::ParseStructDeclaration(
   }
 }
 
+// TODO: All callers of this function should be moved to
+// `Parser::ParseLexedAttributeList`.
+void Parser::ParseLexedCAttributeList(LateParsedAttrList , bool EnterScope,
+  ParsedAttributes *OutAttrs) {
+  assert(LAs.parseSoon() &&
+ "Attribute list should be marked for immediate parsing.");
+#ifndef NDEBUG
+  auto LangStd = getLangOpts().LangStd;
+  if (LangStd != LangStandard::lang_unspecified) {
+auto Lang = LangStandard::getLangStandardForKind(LangStd).getLanguage();
+assert(Lang == Language::C || Lang == Language::OpenCL);

delcypher wrote:

@hnrklssn Yes. The original assert didn't check for `Lang == Language::OpenCL` 
and that caused test failures due to the assertion failing.

https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Henrik G. Olsson via cfe-commits


@@ -4944,6 +4944,26 @@ void Parser::ParseStructDeclaration(
   }
 }
 
+// TODO: All callers of this function should be moved to
+// `Parser::ParseLexedAttributeList`.
+void Parser::ParseLexedCAttributeList(LateParsedAttrList , bool EnterScope,
+  ParsedAttributes *OutAttrs) {
+  assert(LAs.parseSoon() &&
+ "Attribute list should be marked for immediate parsing.");
+#ifndef NDEBUG
+  auto LangStd = getLangOpts().LangStd;
+  if (LangStd != LangStandard::lang_unspecified) {
+auto Lang = LangStandard::getLangStandardForKind(LangStd).getLanguage();
+assert(Lang == Language::C || Lang == Language::OpenCL);

hnrklssn wrote:

Do we have any tests triggering the OpenCL path?

https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits


@@ -4944,6 +4944,26 @@ void Parser::ParseStructDeclaration(
   }
 }
 
+// TODO: All callers of this function should be moved to
+// `Parser::ParseLexedAttributeList`.
+void Parser::ParseLexedCAttributeList(LateParsedAttrList , bool EnterScope,
+  ParsedAttributes *OutAttrs) {
+  assert(LAs.parseSoon() &&
+ "Attribute list should be marked for immediate parsing.");
+#ifndef NDEBUG
+  auto LangStd = getLangOpts().LangStd;
+  if (LangStd != LangStandard::lang_unspecified) {
+auto Lang = LangStandard::getLangStandardForKind(LangStd).getLanguage();
+assert(Lang == Language::C || Lang == Language::OpenCL);

delcypher wrote:

That doesn't sound equivalent. The language could also be objective-C or 
objective-C++.

https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Yeoul Na via cfe-commits


@@ -4944,6 +4944,26 @@ void Parser::ParseStructDeclaration(
   }
 }
 
+// TODO: All callers of this function should be moved to
+// `Parser::ParseLexedAttributeList`.
+void Parser::ParseLexedCAttributeList(LateParsedAttrList , bool EnterScope,
+  ParsedAttributes *OutAttrs) {
+  assert(LAs.parseSoon() &&
+ "Attribute list should be marked for immediate parsing.");
+#ifndef NDEBUG
+  auto LangStd = getLangOpts().LangStd;
+  if (LangStd != LangStandard::lang_unspecified) {
+auto Lang = LangStandard::getLangStandardForKind(LangStd).getLanguage();
+assert(Lang == Language::C || Lang == Language::OpenCL);

rapidsna wrote:

Was there a reason this couldn't be `!getLangOpts().CPlusPlus`?

https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Yeoul Na via cfe-commits


@@ -0,0 +1,196 @@
+// 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'}}
+};
+
+struct no_found_count_not_in_substruct {
+  unsigned long flags;
+  unsigned char count; // expected-note {{'count' declared here}}
+  struct A {
+int dummy;
+int array[] __counted_by(count); // expected-error {{'counted_by' field 
'count' isn't within the same struct as the flexible array}}
+  } a;
+};
+
+struct not_found_count_not_in_unnamed_substruct {
+  unsigned char count; // expected-note {{'count' declared here}}
+  struct {
+int dummy;
+int array[] __counted_by(count); // expected-error {{'counted_by' field 
'count' isn't within the same struct as the flexible array}}
+  } a;
+};
+
+struct not_found_count_not_in_unnamed_substruct_2 {
+  struct {
+unsigned char count; // expected-note {{'count' declared here}}
+  };
+  struct {
+int dummy;
+int array[] __counted_by(count); // expected-error {{'counted_by' field 
'count' isn't within the same struct as the flexible array}}
+  } a;
+};
+
+struct not_found_count_in_other_unnamed_substruct {
+  struct {
+unsigned char count;
+  } a1;
+
+  struct {
+int dummy;
+int array[] __counted_by(count); // expected-error {{use of undeclared 
identifier 'count'}}
+  };
+};
+
+struct not_found_count_in_other_substruct {
+  struct _a1 {
+unsigned char count;
+  } a1;
+
+  struct {
+int dummy;
+int array[] __counted_by(count); // expected-error {{use of undeclared 
identifier 'count'}}
+  };
+};
+
+struct not_found_count_in_other_substruct_2 {
+  struct _a2 {
+unsigned char count;
+  } a2;
+
+  int array[] __counted_by(count); // expected-error {{use of undeclared 
identifier 'count'}}
+};
+
+struct not_found_suggest {
+  int bork;
+  struct bar *fam[] __counted_by(blork); // expected-error {{use of undeclared 
identifier 'blork'}}
+};
+
+int global; // expected-note {{'global' declared here}}
+
+struct found_outside_of_struct {
+  int bork;
+  struct bar *fam[] __counted_by(global); // expected-error {{field 'global' 
in 'counted_by' not inside structure}}
+};
+
+struct self_referrential {
+  int bork;
+  struct bar *self[] __counted_by(self); // expected-error {{use of undeclared 
identifier 'self'}}
+};
+
+struct non_int_count {
+  double dbl_count;
+  struct bar *fam[] __counted_by(dbl_count); // expected-error {{'counted_by' 
requires a non-boolean integer type argument}}
+};
+
+struct array_of_ints_count {
+  int integers[2];
+  struct bar *fam[] __counted_by(integers); // expected-error {{'counted_by' 
requires a non-boolean integer type argument}}
+};
+
+struct not_a_fam {
+  int count;
+  // expected-error@+1{{'counted_by' cannot be applied to a pointer with 
pointee of unknown size because 'struct bar' is an incomplete type}}
+  struct bar *non_fam __counted_by(count);
+};
+
+struct not_a_c99_fam {
+  int count;
+  struct bar *non_c99_fam[0] __counted_by(count); // expected-error 
{{'counted_by' on arrays only applies to C99 flexible array members}}
+};
+
+struct annotated_with_anon_struct {
+  unsigned long flags;
+  struct {
+unsigned char count;
+int array[] __counted_by(crount); // expected-error {{use of undeclared 
identifier 'crount'}}
+  };
+};
+
+//==
+// __counted_by on a struct VLA with element type that has unknown size
+//==
+
+struct size_unknown; // expected-note 2{{forward declaration of 'struct 
size_unknown'}}
+struct on_member_arr_incomplete_ty_ty_pos {
+  int count;
+  // expected-error@+2{{'counted_by' only applies to pointers or C99 flexible 
array members}}
+  // expected-error@+1{{array has incomplete element type 'struct 
size_unknown'}}
+  struct size_unknown buf[] __counted_by(count);
+};
+
+struct on_member_arr_incomplete_const_ty_ty_pos {
+  int count;
+  // expected-error@+2{{'counted_by' only applies to pointers or C99 flexible 
array members}}
+  // expected-error@+1{{array has incomplete element type 'const struct 
size_unknown'}}
+  const struct size_unknown buf[] __counted_by(count);
+};
+
+struct on_member_arr_void_ty_ty_pos {
+  int count;
+  // expected-error@+2{{'counted_by' only applies to pointers or C99 flexible 
array members}}
+  // expected-error@+1{{array has incomplete element type 'void'}}
+  void buf[] __counted_by(count);
+};
+
+typedef void(fn_ty)(int);
+
+struct on_member_arr_fn_ptr_ty {
+  int count;
+  // An Array of function pointers is allowed
+  fn_ty* buf[] __counted_by(count);
+};
+
+struct on_member_arr_fn_ty {
+  int count;
+  // An array of functions is not allowed.
+  // expected-error@+2{{'counted_by' only applies to pointers or C99 flexible 
array members}}
+  // expected-error@+1{{'buf' 

[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Yeoul Na via cfe-commits

https://github.com/rapidsna edited 
https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Yeoul Na via cfe-commits

https://github.com/rapidsna approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits

https://github.com/delcypher updated 
https://github.com/llvm/llvm-project/pull/93121

>From 7f5af7cc180825e07b5f5292e3f6b860c8e8591e Mon Sep 17 00:00:00 2001
From: Dan Liew 
Date: Fri, 17 May 2024 12:07:40 -0700
Subject: [PATCH 1/7] [BoundsSafety] Allow 'counted_by' attribute on pointers
 in structs in C (#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 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.

There are also several other follow up changes that will need to be
addressed in future patches:

* Make late parsing working with anonymous structs (see
`on_pointer_anon_buf` in `attr-counted-by-late-parsed-struct-ptrs.c`).
* Allow `counted_by` on more subjects (e.g. parameters, returns types)
when `-fbounds-safety` is enabled.
* Make use of the attribute on pointer types in code gen (e.g. for
`_builtin_dynamic_object_size` and UBSan's array-bounds checks).

This work is heavily based on a patch originally written by Yeoul Na.

rdar://125400257

Co-authored-by: Dan Liew 
---
 clang/docs/ReleaseNotes.rst   |  21 +-
 clang/include/clang/AST/Type.h|   1 +
 clang/include/clang/Basic/Attr.td |   3 +-
 .../clang/Basic/DiagnosticSemaKinds.td|  17 +-
 clang/include/clang/Parse/Parser.h|   7 +-
 clang/include/clang/Sema/Sema.h   |   3 +-
 clang/lib/AST/Type.cpp|  10 +
 clang/lib/Parse/ParseDecl.cpp | 104 ++-
 clang/lib/Parse/ParseObjc.cpp |  10 +-
 clang/lib/Sema/SemaDeclAttr.cpp   |  82 --
 clang/lib/Sema/SemaType.cpp   |   6 +-
 clang/lib/Sema/TreeTransform.h|   2 +-
 .../attr-counted-by-late-parsed-struct-ptrs.c |  45 
 clang/test/AST/attr-counted-by-struct-ptrs.c  | 117 
 .../Sema/attr-counted-by-late-parsed-off.c|  26 ++
 .../attr-counted-by-late-parsed-struct-ptrs.c | 254 ++
 ...tr-counted-by-struct-ptrs-sizeless-types.c |  17 ++
 clang/test/Sema/attr-counted-by-struct-ptrs.c | 224 +++
 .../Sema/attr-counted-by-vla-sizeless-types.c |  11 +
 clang/test/Sema/attr-counted-by-vla.c | 193 +
 clang/test/Sema/attr-counted-by.c | 112 
 21 files changed, 1117 insertions(+), 148 deletions(-)
 create mode 100644 clang/test/AST/attr-counted-by-late-parsed-struct-ptrs.c
 create mode 100644 clang/test/AST/attr-counted-by-struct-ptrs.c
 create mode 100644 clang/test/Sema/attr-counted-by-late-parsed-off.c
 create mode 100644 clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c
 create mode 100644 clang/test/Sema/attr-counted-by-struct-ptrs-sizeless-types.c
 create mode 100644 clang/test/Sema/attr-counted-by-struct-ptrs.c
 create mode 100644 clang/test/Sema/attr-counted-by-vla-sizeless-types.c
 create mode 100644 clang/test/Sema/attr-counted-by-vla.c
 delete mode 100644 clang/test/Sema/attr-counted-by.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 81e9d0423f96a..5d59b3b053600 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -317,7 +317,8 @@ New Compiler Flags
 
 - 

[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits

delcypher wrote:

Hmm looks like I'll have to tweak the assert. Looks like some unit tests don't 
specify the language.

```
FAIL: Clang-Unit :: CodeGen/./ClangCodeGenTests/5/16 (19428 of 19653)
 TEST 'Clang-Unit :: CodeGen/./ClangCodeGenTests/5/16' 
FAILED 
Script(shard):
--
GTEST_OUTPUT=json:/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/tools/clang/unittests/CodeGen/./ClangCodeGenTests-Clang-Unit-78051-5-16.json
 GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=16 GTEST_SHARD_INDEX=5 
/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/tools/clang/unittests/CodeGen/./ClangCodeGenTests
--

Note: This is test shard 6 of 16.
[==] Running 1 test from 1 test suite.
[--] Global test environment set-up.
[--] 1 test from TBAAMetadataTest
[ RUN  ] TBAAMetadataTest.CTypedefFields2
LLVM ERROR: getLangStandardForKind() on unspecified kind
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH 
or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
0  ClangCodeGenTests0x000102c186e4 
llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 88
1  ClangCodeGenTests0x000102c18cc8 
PrintStackTraceSignalHandler(void*) + 28
2  ClangCodeGenTests0x000102c1695c llvm::sys::RunSignalHandlers() + 
152
3  ClangCodeGenTests0x000102c19e30 SignalHandler(int) + 276
4  libsystem_platform.dylib 0x0001813b9a24 _sigtramp + 56
5  libsystem_pthread.dylib  0x000181389cc0 pthread_kill + 288
6  libsystem_c.dylib0x000181295a40 abort + 180
7  ClangCodeGenTests0x000102ad60bc 
llvm::report_fatal_error(llvm::Twine const&, bool) + 364
8  ClangCodeGenTests0x000102ad5f50 
llvm::report_fatal_error(llvm::Twine const&, bool) + 0
9  ClangCodeGenTests0x00010388c44c 
clang::LangStandard::getLangStandardForKind(clang::LangStandard::Kind) + 88
10 ClangCodeGenTests0x0001048d565c 
clang::Parser::ParseLexedCAttributeList(clang::Parser::LateParsedAttrList&, 
bool, clang::ParsedAttributes*) + 144
11 ClangCodeGenTests0x0001048d6190 
clang::Parser::ParseStructUnionBody(clang::SourceLocation, 
clang::TypeSpecifierType, clang::RecordDecl*) + 1724
12 ClangCodeGenTests0x0001048ff868 
clang::Parser::ParseClassSpecifier(clang::tok::TokenKind, 
clang::SourceLocation, clang::DeclSpec&, clang::Parser::ParsedTemplateInfo&, 
clang::AccessSpecifier, bool, clang::Parser::DeclSpecContext, 
clang::ParsedAttributes&) + 8916
13 ClangCodeGenTests0x0001048ce33c 
clang::Parser::ParseDeclarationSpecifiers(clang::DeclSpec&, 
clang::Parser::ParsedTemplateInfo&, clang::AccessSpecifier, 
clang::Parser::DeclSpecContext, clang::Parser::LateParsedAttrList*, 
clang::ImplicitTypenameContext) + 15492
14 ClangCodeGenTests0x0001048c5908 
clang::Parser::ParseDeclarationSpecifiers(clang::DeclSpec&, 
clang::Parser::ParsedTemplateInfo&, clang::AccessSpecifier, 
clang::Parser::DeclSpecContext, clang::Parser::LateParsedAttrList*) + 124
15 ClangCodeGenTests0x0001048c54ac 
clang::Parser::ParseSimpleDeclaration(clang::DeclaratorContext, 
clang::SourceLocation&, clang::ParsedAttributes&, clang::ParsedAttributes&, 
bool, clang::Parser::ForRangeInit*, clang::SourceLocation*) + 260
16 ClangCodeGenTests0x0001048c5284 
clang::Parser::ParseDeclaration(clang::DeclaratorContext, 
clang::SourceLocation&, clang::ParsedAttributes&, clang::ParsedAttributes&, 
clang::SourceLocation*) + 1060
17 ClangCodeGenTests0x0001049ddbd0 
clang::Parser::ParseExternalDeclaration(clang::ParsedAttributes&, 
clang::ParsedAttributes&, clang::ParsingDeclSpec*) + 2196
18 ClangCodeGenTests0x0001049dc10c 
clang::Parser::ParseTopLevelDecl(clang::OpaquePtr&, 
clang::Sema::ModuleImportState&) + 2024
19 ClangCodeGenTests0x0001049db880 
clang::Parser::ParseFirstTopLevelDecl(clang::OpaquePtr&, 
clang::Sema::ModuleImportState&) + 64
20 ClangCodeGenTests0x0001048ac1c8 clang::ParseAST(clang::Sema&, 
bool, bool) + 476
21 ClangCodeGenTests0x0001024b1824 llvm::TestCompiler::compile() + 
48
22 ClangCodeGenTests0x0001024c1024 (anonymous 
namespace)::TBAAMetadataTest_CTypedefFields2_Test::TestBody() + 236
23 ClangCodeGenTests0x000102c9cf40 void 
testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) + 132
24 ClangCodeGenTests0x000102c6be7c void 
testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) + 96
25 ClangCodeGenTests0x000102c6bdcc testing::Test::Run() + 192
26 ClangCodeGenTests0x000102c6c894 testing::TestInfo::Run() + 304
27 ClangCodeGenTests0x000102c6d768 testing::TestSuite::Run() + 412
28 ClangCodeGenTests0x000102c77ee4 
testing::internal::UnitTestImpl::RunAllTests() + 1024
29 

[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Vlad Serebrennikov via cfe-commits

https://github.com/Endilll commented:

`Sema.h` changes look good.

https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits


@@ -4993,20 +4993,8 @@ void Parser::ParseLexedCAttribute(LateParsedAttribute 
,
  "late field attribute expects to have at most one declaration.");
 
   // Dispatch based on the attribute and parse it
-  const AttributeCommonInfo::Form ParsedForm = ParsedAttr::Form::GNU();

delcypher wrote:

@hnrklssn @rapidsna Heads up I plan to land this change too. I discovered that 
this this switch statement doesn't need to exist because 
`ParseGNUAttributeArgs` effectively already does what we need.

https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits

https://github.com/delcypher edited 
https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits

https://github.com/delcypher updated 
https://github.com/llvm/llvm-project/pull/93121

>From 7f5af7cc180825e07b5f5292e3f6b860c8e8591e Mon Sep 17 00:00:00 2001
From: Dan Liew 
Date: Fri, 17 May 2024 12:07:40 -0700
Subject: [PATCH 1/7] [BoundsSafety] Allow 'counted_by' attribute on pointers
 in structs in C (#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 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.

There are also several other follow up changes that will need to be
addressed in future patches:

* Make late parsing working with anonymous structs (see
`on_pointer_anon_buf` in `attr-counted-by-late-parsed-struct-ptrs.c`).
* Allow `counted_by` on more subjects (e.g. parameters, returns types)
when `-fbounds-safety` is enabled.
* Make use of the attribute on pointer types in code gen (e.g. for
`_builtin_dynamic_object_size` and UBSan's array-bounds checks).

This work is heavily based on a patch originally written by Yeoul Na.

rdar://125400257

Co-authored-by: Dan Liew 
---
 clang/docs/ReleaseNotes.rst   |  21 +-
 clang/include/clang/AST/Type.h|   1 +
 clang/include/clang/Basic/Attr.td |   3 +-
 .../clang/Basic/DiagnosticSemaKinds.td|  17 +-
 clang/include/clang/Parse/Parser.h|   7 +-
 clang/include/clang/Sema/Sema.h   |   3 +-
 clang/lib/AST/Type.cpp|  10 +
 clang/lib/Parse/ParseDecl.cpp | 104 ++-
 clang/lib/Parse/ParseObjc.cpp |  10 +-
 clang/lib/Sema/SemaDeclAttr.cpp   |  82 --
 clang/lib/Sema/SemaType.cpp   |   6 +-
 clang/lib/Sema/TreeTransform.h|   2 +-
 .../attr-counted-by-late-parsed-struct-ptrs.c |  45 
 clang/test/AST/attr-counted-by-struct-ptrs.c  | 117 
 .../Sema/attr-counted-by-late-parsed-off.c|  26 ++
 .../attr-counted-by-late-parsed-struct-ptrs.c | 254 ++
 ...tr-counted-by-struct-ptrs-sizeless-types.c |  17 ++
 clang/test/Sema/attr-counted-by-struct-ptrs.c | 224 +++
 .../Sema/attr-counted-by-vla-sizeless-types.c |  11 +
 clang/test/Sema/attr-counted-by-vla.c | 193 +
 clang/test/Sema/attr-counted-by.c | 112 
 21 files changed, 1117 insertions(+), 148 deletions(-)
 create mode 100644 clang/test/AST/attr-counted-by-late-parsed-struct-ptrs.c
 create mode 100644 clang/test/AST/attr-counted-by-struct-ptrs.c
 create mode 100644 clang/test/Sema/attr-counted-by-late-parsed-off.c
 create mode 100644 clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c
 create mode 100644 clang/test/Sema/attr-counted-by-struct-ptrs-sizeless-types.c
 create mode 100644 clang/test/Sema/attr-counted-by-struct-ptrs.c
 create mode 100644 clang/test/Sema/attr-counted-by-vla-sizeless-types.c
 create mode 100644 clang/test/Sema/attr-counted-by-vla.c
 delete mode 100644 clang/test/Sema/attr-counted-by.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 81e9d0423f96a..5d59b3b053600 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -317,7 +317,8 @@ New Compiler Flags
 
 - 

[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits

https://github.com/delcypher updated 
https://github.com/llvm/llvm-project/pull/93121

>From 7f5af7cc180825e07b5f5292e3f6b860c8e8591e Mon Sep 17 00:00:00 2001
From: Dan Liew 
Date: Fri, 17 May 2024 12:07:40 -0700
Subject: [PATCH 1/6] [BoundsSafety] Allow 'counted_by' attribute on pointers
 in structs in C (#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 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.

There are also several other follow up changes that will need to be
addressed in future patches:

* Make late parsing working with anonymous structs (see
`on_pointer_anon_buf` in `attr-counted-by-late-parsed-struct-ptrs.c`).
* Allow `counted_by` on more subjects (e.g. parameters, returns types)
when `-fbounds-safety` is enabled.
* Make use of the attribute on pointer types in code gen (e.g. for
`_builtin_dynamic_object_size` and UBSan's array-bounds checks).

This work is heavily based on a patch originally written by Yeoul Na.

rdar://125400257

Co-authored-by: Dan Liew 
---
 clang/docs/ReleaseNotes.rst   |  21 +-
 clang/include/clang/AST/Type.h|   1 +
 clang/include/clang/Basic/Attr.td |   3 +-
 .../clang/Basic/DiagnosticSemaKinds.td|  17 +-
 clang/include/clang/Parse/Parser.h|   7 +-
 clang/include/clang/Sema/Sema.h   |   3 +-
 clang/lib/AST/Type.cpp|  10 +
 clang/lib/Parse/ParseDecl.cpp | 104 ++-
 clang/lib/Parse/ParseObjc.cpp |  10 +-
 clang/lib/Sema/SemaDeclAttr.cpp   |  82 --
 clang/lib/Sema/SemaType.cpp   |   6 +-
 clang/lib/Sema/TreeTransform.h|   2 +-
 .../attr-counted-by-late-parsed-struct-ptrs.c |  45 
 clang/test/AST/attr-counted-by-struct-ptrs.c  | 117 
 .../Sema/attr-counted-by-late-parsed-off.c|  26 ++
 .../attr-counted-by-late-parsed-struct-ptrs.c | 254 ++
 ...tr-counted-by-struct-ptrs-sizeless-types.c |  17 ++
 clang/test/Sema/attr-counted-by-struct-ptrs.c | 224 +++
 .../Sema/attr-counted-by-vla-sizeless-types.c |  11 +
 clang/test/Sema/attr-counted-by-vla.c | 193 +
 clang/test/Sema/attr-counted-by.c | 112 
 21 files changed, 1117 insertions(+), 148 deletions(-)
 create mode 100644 clang/test/AST/attr-counted-by-late-parsed-struct-ptrs.c
 create mode 100644 clang/test/AST/attr-counted-by-struct-ptrs.c
 create mode 100644 clang/test/Sema/attr-counted-by-late-parsed-off.c
 create mode 100644 clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c
 create mode 100644 clang/test/Sema/attr-counted-by-struct-ptrs-sizeless-types.c
 create mode 100644 clang/test/Sema/attr-counted-by-struct-ptrs.c
 create mode 100644 clang/test/Sema/attr-counted-by-vla-sizeless-types.c
 create mode 100644 clang/test/Sema/attr-counted-by-vla.c
 delete mode 100644 clang/test/Sema/attr-counted-by.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 81e9d0423f96a..5d59b3b053600 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -317,7 +317,8 @@ New Compiler Flags
 
 - 

[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-23 Thread Dan Liew via cfe-commits

delcypher wrote:

Okay this is interesting...

```

FAIL: Clang :: SemaObjC/warn-thread-safety-analysis.m (15515 of 19369)
 TEST 'Clang :: SemaObjC/warn-thread-safety-analysis.m' 
FAILED 
Exit Code: 134

Command Output (stderr):
--
RUN: at line 1: 
/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/bin/clang 
-cc1 -internal-isystem 
/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/lib/clang/19/include
 -nostdsysteminc -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta 
-Wno-objc-root-class 
/Volumes/user_data/dev/llvm/llvm.org/main/src/llvm-project/clang/test/SemaObjC/warn-thread-safety-analysis.m
+ /Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/bin/clang 
-cc1 -internal-isystem 
/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/lib/clang/19/include
 -nostdsysteminc -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta 
-Wno-objc-root-class 
/Volumes/user_data/dev/llvm/llvm.org/main/src/llvm-project/clang/test/SemaObjC/warn-thread-safety-analysis.m
Unhandled late parsed attribute
UNREACHABLE executed at 
/Volumes/user_data/dev/llvm/llvm.org/main/src/llvm-project/clang/lib/Parse/ParseDecl.cpp:4990!
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and 
include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.  Program arguments: 
/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/bin/clang 
-cc1 -internal-isystem 
/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/lib/clang/19/include
 -nostdsysteminc -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta 
-Wno-objc-root-class 
/Volumes/user_data/dev/llvm/llvm.org/main/src/llvm-project/clang/test/SemaObjC/warn-thread-safety-analysis.m
1.  
/Volumes/user_data/dev/llvm/llvm.org/main/src/llvm-project/clang/test/SemaObjC/warn-thread-safety-analysis.m:7:61:
 current parser token '('
 #0 0x000107d741b8 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/bin/clang-19+0x1035d41b8)
 #1 0x000107d7479c PrintStackTraceSignalHandler(void*) 
(/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/bin/clang-19+0x1035d479c)
 #2 0x000107d72454 llvm::sys::RunSignalHandlers() 
(/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/bin/clang-19+0x1035d2454)
 #3 0x000107d758b8 SignalHandler(int) 
(/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/bin/clang-19+0x1035d58b8)
 #4 0x0001813b9a24 (/usr/lib/system/libsystem_platform.dylib+0x18046da24)
 #5 0x000181389cc0 (/usr/lib/system/libsystem_pthread.dylib+0x18043dcc0)
 #6 0x000181295a40 (/usr/lib/system/libsystem_c.dylib+0x180349a40)
 #7 0x000107c4f0dc llvm::install_out_of_memory_new_handler() 
(/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/bin/clang-19+0x1034af0dc)
 #8 0x00010c40fc90 
clang::Parser::ParseLexedCAttribute(clang::Parser::LateParsedAttribute&, 
clang::ParsedAttributes*) 
(/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/bin/clang-19+0x107c6fc90)
 #9 0x00010c3ed5ec 
clang::Parser::ParseLexedAttributeList(clang::Parser::LateParsedAttrList&, 
clang::Decl*, bool, bool) 
(/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/bin/clang-19+0x107c4d5ec)
#10 0x00010c51a05c 
clang::Parser::ParseFunctionDefinition(clang::ParsingDeclarator&, 
clang::Parser::ParsedTemplateInfo const&, clang::Parser::LateParsedAttrList*) 
(/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/bin/clang-19+0x107d7a05c)
#11 0x00010c400bb8 clang::Parser::ParseDeclGroup(clang::ParsingDeclSpec&, 
clang::DeclaratorContext, clang::ParsedAttributes&, 
clang::Parser::ParsedTemplateInfo&, clang::SourceLocation*, 
clang::Parser::ForRangeInit*) 
(/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/bin/clang-19+0x107c60bb8)
#12 0x00010c5190bc 
clang::Parser::ParseDeclOrFunctionDefInternal(clang::ParsedAttributes&, 
clang::ParsedAttributes&, clang::ParsingDeclSpec&, clang::AccessSpecifier) 
(/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/bin/clang-19+0x107d790bc)
#13 0x00010c5185f4 
clang::Parser::ParseDeclarationOrFunctionDefinition(clang::ParsedAttributes&, 
clang::ParsedAttributes&, clang::ParsingDeclSpec*, clang::AccessSpecifier) 
(/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/bin/clang-19+0x107d785f4)
#14 0x00010c5177f0 
clang::Parser::ParseExternalDeclaration(clang::ParsedAttributes&, 
clang::ParsedAttributes&, clang::ParsingDeclSpec*) 
(/Volumes/user_data/dev/llvm/llvm.org/main/builds/Debug_xc___sccache/bin/clang-19+0x107d777f0)
#15 0x00010c5158cc 
clang::Parser::ParseTopLevelDecl(clang::OpaquePtr&, 
clang::Sema::ModuleImportState&) 

[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-22 Thread Dan Liew via cfe-commits

delcypher wrote:

Hmm looks like I broke

```
  Clang :: Sema/attr-capabilities.c
  Clang :: Sema/diagnose_if.c
  Clang :: Sema/warn-thread-safety-analysis.c
  Clang :: SemaObjC/warn-thread-safety-analysis.m
```

Let's see if I can figure out what I did...

https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-22 Thread Dan Liew via cfe-commits

delcypher wrote:

@rapidsna Please check 604e274a5bc37b18c3bc89eed5749c8617de36e7 to check you 
are happy with the way I'm fixing the memory leak.

https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-22 Thread Dan Liew via cfe-commits


@@ -744,7 +744,13 @@ void Parser::ParseLexedAttributeList(LateParsedAttrList 
, Decl *D,
   for (unsigned i = 0, ni = LAs.size(); i < ni; ++i) {
 if (D)
   LAs[i]->addDecl(D);
-ParseLexedAttribute(*LAs[i], EnterScope, OnDefinition);
+// FIXME: There has to be a better way to ask "is this C?""
+if (LangStandard::getLangStandardForKind(getLangOpts().LangStd)

delcypher wrote:

@AaronBallman @erichkeane Is there a better way to check that we're building 
for C?

https://github.com/llvm/llvm-project/pull/93121
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-22 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Dan Liew (delcypher)


Changes

This PR attempts to reland https://github.com/llvm/llvm-project/pull/90786 that 
was reverted due to a memory leak.

This PR includes the following commits

1. The original commit in #90786  (originally 
0ec3b972e58bcbcdc1bebe1696ea37f2931287c3)
2. A follow up fix to `pragma-attribute-supported-attributes-list.test` 
(originally in cef6387e52578366c2332275dad88b9953b55336)
3. A relaxation of `counted_by` semantics (originally in 
112eadd55f06bee15caadff688ea0b45acbfa804)
4. A fix to the memory leak (in 604e274a5bc37b18c3bc89eed5749c8617de36e7)

The intention will be squash all the changes into a single commit once we've 
agreed to the memory leak fix.

## The Memory leak

The problem was that these lines parsed the attributes but then did nothing to 
free the memory

```c++
assert(!getLangOpts().CPlusPlus);
for (auto *LateAttr : LateFieldAttrs)
  ParseLexedCAttribute(*LateAttr);
```

To fix this the existing `Parser::ParseLexedAttributeList` method has been 
tweaked for our use case and has been re-used because it does the necessary 
memory management.

A more principled fixed here would be to fix the ownership of the 
`LateParsedAttribute` objects. In principle `LateParsedAttrList` should own its 
pointers and be responsible for deallocating them. Unfortunately this is 
complicated by `LateParsedAttribute` objects also being stored in another data 
structure (`LateParsedDeclarations`).

```c++
  // Handle attributes with arguments that require late parsing.
  LateParsedAttribute *LA =
  new LateParsedAttribute(this, *AttrName, AttrNameLoc);
  LateAttrs-push_back(LA);

  // Attributes in a class are parsed at the end of the class, along
  // with other late-parsed declarations.
  if (!ClassStack.empty()  !LateAttrs-parseSoon())
getCurrentClass().LateParsedDeclarations.push_back(LA);
```

this means the ownership `LateParsedAttribute` objects isn't clear :( 

---

Patch is 63.12 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/93121.diff


24 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+20-1) 
- (modified) clang/include/clang/AST/Type.h (+1) 
- (modified) clang/include/clang/Basic/Attr.td (+2-1) 
- (modified) clang/include/clang/Basic/DiagnosticGroups.td (+4) 
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+21-2) 
- (modified) clang/include/clang/Parse/Parser.h (+6-1) 
- (modified) clang/include/clang/Sema/Sema.h (+2-1) 
- (modified) clang/lib/AST/Type.cpp (+10) 
- (modified) clang/lib/Parse/ParseCXXInlineMethods.cpp (+7-1) 
- (modified) clang/lib/Parse/ParseDecl.cpp (+97-7) 
- (modified) clang/lib/Parse/ParseObjc.cpp (+6-4) 
- (modified) clang/lib/Sema/SemaDeclAttr.cpp (+80-15) 
- (modified) clang/lib/Sema/SemaType.cpp (+3-3) 
- (modified) clang/lib/Sema/TreeTransform.h (+1-1) 
- (added) clang/test/AST/attr-counted-by-late-parsed-struct-ptrs.c (+45) 
- (added) clang/test/AST/attr-counted-by-struct-ptrs.c (+117) 
- (modified) clang/test/Misc/pragma-attribute-supported-attributes-list.test 
(-1) 
- (added) clang/test/Sema/attr-counted-by-late-parsed-off.c (+26) 
- (added) clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c (+254) 
- (added) clang/test/Sema/attr-counted-by-struct-ptrs-sizeless-types.c (+17) 
- (added) clang/test/Sema/attr-counted-by-struct-ptrs.c (+224) 
- (added) clang/test/Sema/attr-counted-by-vla-sizeless-types.c (+11) 
- (added) clang/test/Sema/attr-counted-by-vla.c (+196) 
- (removed) clang/test/Sema/attr-counted-by.c (-112) 


``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 81e9d0423f96a..5d59b3b053600 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -317,7 +317,8 @@ New Compiler Flags
 
 - ``-fexperimental-late-parse-attributes`` enables an experimental feature to
   allow late parsing certain attributes in specific contexts where they would
-  not normally be late parsed.
+  not normally be late parsed. Currently this allows late parsing the
+  `counted_by` attribute in C. See `Attribute Changes in Clang`_.
 
 - ``-fseparate-named-sections`` uses separate unique sections for global
   symbols in named special sections (i.e. symbols annotated with
@@ -406,6 +407,24 @@ Attribute Changes in Clang
 - The ``clspv_libclc_builtin`` attribute has been added to allow clspv
   (`OpenCL-C to Vulkan SPIR-V compiler `_) to 
identify functions coming from libclc
   (`OpenCL-C builtin library `_).
+- The ``counted_by`` attribute is now allowed on pointers that are members of a
+  struct in C.
+
+- The ``counted_by`` attribute can now be late parsed in C when
+  ``-fexperimental-late-parse-attributes`` is passed but only when attribute is
+  used in the declaration attribute position. This allows using the
+  attribute on existing code where it previously impossible 

[clang] Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (PR #93121)

2024-05-22 Thread Dan Liew via cfe-commits

https://github.com/delcypher created 
https://github.com/llvm/llvm-project/pull/93121

This PR attempts to reland https://github.com/llvm/llvm-project/pull/90786 that 
was reverted due to a memory leak.

This PR includes the following commits

1. The original commit in #90786  (originally 
0ec3b972e58bcbcdc1bebe1696ea37f2931287c3)
2. A follow up fix to `pragma-attribute-supported-attributes-list.test` 
(originally in cef6387e52578366c2332275dad88b9953b55336)
3. A relaxation of `counted_by` semantics (originally in 
112eadd55f06bee15caadff688ea0b45acbfa804)
4. A fix to the memory leak (in 604e274a5bc37b18c3bc89eed5749c8617de36e7)

The intention will be squash all the changes into a single commit once we've 
agreed to the memory leak fix.

## The Memory leak

The problem was that these lines parsed the attributes but then did nothing to 
free the memory

```c++
assert(!getLangOpts().CPlusPlus);
for (auto *LateAttr : LateFieldAttrs)
  ParseLexedCAttribute(*LateAttr);
```

To fix this the existing `Parser::ParseLexedAttributeList` method has been 
tweaked for our use case and has been re-used because it does the necessary 
memory management.

A more principled fixed here would be to fix the ownership of the 
`LateParsedAttribute` objects. In principle `LateParsedAttrList` should own its 
pointers and be responsible for deallocating them. Unfortunately this is 
complicated by `LateParsedAttribute` objects also being stored in another data 
structure (`LateParsedDeclarations`).

```c++
  // Handle attributes with arguments that require late parsing.
  LateParsedAttribute *LA =
  new LateParsedAttribute(this, *AttrName, AttrNameLoc);
  LateAttrs->push_back(LA);

  // Attributes in a class are parsed at the end of the class, along
  // with other late-parsed declarations.
  if (!ClassStack.empty() && !LateAttrs->parseSoon())
getCurrentClass().LateParsedDeclarations.push_back(LA);
```

this means the ownership `LateParsedAttribute` objects isn't clear :( 

>From 7f5af7cc180825e07b5f5292e3f6b860c8e8591e Mon Sep 17 00:00:00 2001
From: Dan Liew 
Date: Fri, 17 May 2024 12:07:40 -0700
Subject: [PATCH 1/4] [BoundsSafety] Allow 'counted_by' attribute on pointers
 in structs in C (#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 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.

There are also several other follow up changes that will need to be
addressed in future patches:

* Make late parsing working with anonymous structs (see
`on_pointer_anon_buf` in `attr-counted-by-late-parsed-struct-ptrs.c`).
* Allow `counted_by` on more subjects (e.g. parameters, returns types)
when `-fbounds-safety` is enabled.
* Make use of the attribute on pointer types in code gen (e.g. for
`_builtin_dynamic_object_size` and UBSan's array-bounds checks).

This work is heavily based on a patch originally written by Yeoul Na.

rdar://125400257

Co-authored-by: Dan Liew 
---
 clang/docs/ReleaseNotes.rst   |  21 +-
 clang/include/clang/AST/Type.h|   1 +
 clang/include/clang/Basic/Attr.td |   3 +-
 .../clang/Basic/DiagnosticSemaKinds.td|  17 +-