[all-commits] [llvm/llvm-project] 291897: Reland #90786 ([BoundsSafety] Allow 'counted_by' a...

[Dan Liew via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 29189738b832b111b905fcc037a287eeeb0aab2c
      https://github.com/llvm/llvm-project/commit/29189738b832b111b905fcc037a287eeeb0aab2c
  Author: Dan Liew <dan at su-root.co.uk>
  Date:   2024-05-23 (Thu, 23 May 2024)

  Changed paths:
    M clang/docs/ReleaseNotes.rst
    M clang/include/clang/AST/Type.h
    M clang/include/clang/Basic/Attr.td
    M clang/include/clang/Basic/DiagnosticGroups.td
    M clang/include/clang/Basic/DiagnosticSemaKinds.td
    M clang/include/clang/Parse/Parser.h
    M clang/include/clang/Sema/Sema.h
    M clang/lib/AST/Type.cpp
    M clang/lib/Parse/ParseDecl.cpp
    M clang/lib/Parse/ParseObjc.cpp
    M clang/lib/Sema/SemaDeclAttr.cpp
    M clang/lib/Sema/SemaType.cpp
    M clang/lib/Sema/TreeTransform.h
    A clang/test/AST/attr-counted-by-late-parsed-struct-ptrs.c
    A clang/test/AST/attr-counted-by-struct-ptrs.c
    M clang/test/Misc/pragma-attribute-supported-attributes-list.test
    A clang/test/Sema/attr-counted-by-late-parsed-off.c
    A clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c
    A clang/test/Sema/attr-counted-by-struct-ptrs-sizeless-types.c
    A clang/test/Sema/attr-counted-by-struct-ptrs.c
    A clang/test/Sema/attr-counted-by-vla-sizeless-types.c
    A clang/test/Sema/attr-counted-by-vla.c
    R clang/test/Sema/attr-counted-by.c

  Log Message:
  -----------
  Reland #90786 ([BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C) (#93121)

[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())
        getCurrentClass().LateParsedDeclarations.push_back(LA);
```

this means the ownership of LateParsedAttribute objects isn't very
clear.

rdar://125400257



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications