Skip to content

[Clang] Add __builtin_counted_by_ref builtin #114495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 73 commits into from
Nov 7, 2024
Merged

[Clang] Add __builtin_counted_by_ref builtin #114495

Changes from 1 commit
Commits
Show all changes
73 commits
Select commit Hold shift + click to select a range
fe49528
[Clang] Add __builtin_counted_by_ref builtin
bwendling Aug 7, 2024
e3583c7
If the 'counted_by' attribute doesn't exist, then return nullptr.
bwendling Aug 7, 2024
053bfcd
Remove debugging code.
bwendling Aug 7, 2024
4ce6525
Make returning a nullptr the default.
bwendling Aug 7, 2024
8017e73
clang-format
bwendling Aug 7, 2024
8808431
Use a visitor to retrieve the MemberExpr. It's intentionally harsh an…
bwendling Aug 7, 2024
122d2b8
Add testcases.
bwendling Aug 7, 2024
2b58283
Combine the code to grab the MemberExpr, if it exists.
bwendling Aug 8, 2024
76f01fe
Move FindCountedByField into a central place.
bwendling Aug 9, 2024
c95e38c
Use CustomTypeChecking for the builtin. It allows us explicitly to se…
bwendling Aug 14, 2024
925fbd2
[Clang][NFC] Move FindCountedByField into FieldDecl (#104235)
bwendling Aug 15, 2024
ec62a2b
Fix a build failure.
bwendling Aug 15, 2024
d3ca48c
Remove empty line.
bwendling Aug 15, 2024
85b4474
Remove merge errors.
bwendling Aug 15, 2024
91a7d1b
Use 'size_t *' for the default return type and improve the testcase.
bwendling Aug 15, 2024
2d95e44
Don't use a Visitor to retrieve the MemberExpr. Instead accept only n…
bwendling Aug 15, 2024
9628b0f
Improve error messages. Make the side-effects error a warning.
bwendling Aug 15, 2024
c278c1f
Add _Generic checks.
bwendling Aug 15, 2024
0f42e5d
Remove unrelated change
bwendling Aug 21, 2024
751a55d
Use MemberExpr::Create instead of MemberExpr::CreateImplicit.
bwendling Aug 21, 2024
53272cf
Regenerate
bwendling Aug 21, 2024
ecda8e0
Add -W flag for warning.
bwendling Aug 21, 2024
612000f
Handle any anonymous structs surrounding the flexible array member an…
bwendling Aug 22, 2024
b555af9
Add anonymous struct testcase.
bwendling Aug 22, 2024
48707ae
Report a fatal error if we can't find the 'count' field.
bwendling Aug 23, 2024
5c778ca
Use 'cast' instead of 'dyn_cast' and default to 'void *' for the retu…
isanbard Aug 23, 2024
a1a6ead
A 'void *' can result in an error message about assigning to a 'void'…
bwendling Aug 23, 2024
6b388b5
Add some documentation.
bwendling Aug 26, 2024
791443b
Rename to __builtin_counted_by_ref.
bwendling Sep 10, 2024
768e7b1
Rename method to reflect new builtin name.
bwendling Sep 10, 2024
4111cc0
Emit an error if the 'count' reference is assigned to a variable, use…
bwendling Sep 11, 2024
d897d09
Ignore paren casts.
bwendling Sep 11, 2024
4f1c313
Don't allow __builtin_counted_by_ref in complex expressions.
bwendling Sep 11, 2024
7002f50
Add test case for pointers.
bwendling Sep 11, 2024
7ef517b
Add SourceRange to the diagnostic.
bwendling Sep 12, 2024
19e00f1
Check that __builtin_counted_by_ref isn't used on the LHS by a comple…
bwendling Sep 13, 2024
8f6ef28
Update clang/include/clang/Basic/DiagnosticSemaKinds.td
bwendling Sep 13, 2024
cd65acc
Add a flag indicating that the 'MemberExpr' was once a call to '__bui…
bwendling Sep 19, 2024
9b36f19
Remove invalid tests that take the address of the bounds counter.
bwendling Sep 20, 2024
c08d8d9
Add testcase that discards side-effects.
bwendling Sep 20, 2024
906dd2e
Delay warnings for _Generic statements.
bwendling Sep 26, 2024
e3fbc03
Marking some diagnostics as 'potentially evaluated' somehow prevented…
bwendling Oct 3, 2024
ac5ff7c
Regenerate testcase.
bwendling Oct 3, 2024
072f7e4
Fix testcase.
bwendling Oct 3, 2024
391e2f6
Fix some of the naming and remove dead code.
bwendling Oct 4, 2024
4555763
Correct include ordering.
bwendling Oct 4, 2024
4e22bd6
Improve the argument checking so that it's checking specifically for …
bwendling Oct 16, 2024
985cd02
Move setting that the counter is a bounds-safety counter to where __b…
bwendling Oct 18, 2024
f9ebf9f
Retain the __builtin_counted_by_ref CallExpr for AST dumping reasons.…
bwendling Nov 1, 2024
a5245e5
Fix
bwendling Nov 1, 2024
271bb8f
Reformat
bwendling Nov 1, 2024
ceb43e5
Add correct attribute checking and set the return type.
bwendling Nov 1, 2024
b01526d
Generate LLVM IR for the __builtin_counted_by_ref.
bwendling Nov 1, 2024
76aabde
Set InBounds on the GEP.
bwendling Nov 1, 2024
c390168
Regenerate testcase.
bwendling Nov 1, 2024
213b00d
Remove warning that no longer shows up.
bwendling Nov 1, 2024
c4509f2
Regenerate testcase
bwendling Nov 1, 2024
e22f7ef
Expand testing. This includes a hack to prevent multiple diagnostics …
bwendling Nov 1, 2024
90d6ea8
Add test rejecting using of 'counted_by' in C++.
bwendling Nov 1, 2024
894a7da
Add -ast-print testcase.
bwendling Nov 4, 2024
f8de7dc
Re-add inbounds to the GEP.
bwendling Nov 4, 2024
ed875eb
Add test for &ptr->array
bwendling Nov 4, 2024
3b50e8d
Remote -O2 from codegen.
bwendling Nov 4, 2024
d2112e5
Remove setting the 'inBounds' flag.
bwendling Nov 4, 2024
d595618
Upgrade the side-effects warning into an error.
bwendling Nov 5, 2024
faffcf8
Restrict __builtin_counted_by_ref to 'ptr->array' to match GCC's impl…
bwendling Nov 5, 2024
426782f
Regenerate.
bwendling Nov 5, 2024
4892df7
Revert the modifications as they're not affected by this feature.
bwendling Nov 5, 2024
e26e5b6
Add testcase.
bwendling Nov 7, 2024
2a0c003
Revert.
bwendling Nov 7, 2024
26d9e13
Revert unaffected tests.
bwendling Nov 7, 2024
5a66155
Merge branch 'llvm:main' into builtin_get_counted_by
bwendling Nov 7, 2024
c37a6b3
Fix Sphinx formatting.
bwendling Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/test/Sema/builtin-counted-by-ref.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ void test1(struct fam_struct *ptr, int size, int idx) {

*__builtin_counted_by_ref(ptr->array) = size; // ok
*__builtin_counted_by_ref(&ptr->array[idx]) = size; // ok
*__builtin_counted_by_ref(&ptr->array) = size; // ok
Copy link
Contributor

@rapidsna rapidsna Nov 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it okay? Or should we make it an ill-form?

Copy link
Collaborator Author

@bwendling bwendling Nov 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like it generates correct code:

  {
      unsigned long int __ignored_assignment;
      *_Generic(
          __flex_counter(&p->array),
              void *: &__ignored_assignment,
              default: __flex_counter(&p->array)) = 42;
  }

/*
define dso_local noalias noundef ptr @alloc_s(i32 noundef %size) local_unnamed_addr #0 {
entry:
  %0 = load ptr, ptr @p, align 8, !tbaa !5
  %..counted_by.gep = getelementptr inbounds i8, ptr %0, i64 4
  store i8 42, ptr %..counted_by.gep, align 1, !tbaa !9
  ret ptr null
}
 */

But if you prefer it to be ill-formed, then I can do that.

Copy link
Contributor

@rapidsna rapidsna Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this will just work too? *__builtin_counted_by_ref(ptr->array[idx]) = size;

Do you have a use case? I'd prefer these being ill-formed because the user might find it working unexpectedly. But I don't have a strong opinion on this. I'd leave it up to you/Linux users if you find this would be useful.

Copy link
Collaborator Author

@bwendling bwendling Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*__builtin_counted_by_ref(ptr->array[idx]) = size; doesn't work because the builtin requires a pointer to the FAM. (If you meant &ptr->array[idx], that does work.)

I don't have any use case for &ptr->array or the like. I just don't want to be overly restrictive on what's considered a valid argument to the builtin. I think I'll do a comparison with gcc's version and see what they accept. We can match them.

Copy link
Collaborator Author

@bwendling bwendling Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GCC rejects everything that isn't an array. So not even &ptr->array[0]. I guess we can do that.

Copy link
Collaborator Author

@bwendling bwendling Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that, because GCC only checks if the argument is an array, it allows for things like:

int global_array[];

void foo(int val) {
  *__builtin_counted_by_ref(global_array) = val;
}

which isn't great.

Copy link
Contributor

@rapidsna rapidsna Nov 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can still keep it an error and have GCC to reflect our model on this one, unless Linux relies on this?

Copy link
Collaborator Author

@bwendling bwendling Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I don't want to allow for global arrays either, just flexible array members.



{
size_t __ignored_assignment;
Expand Down