-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[Clang] Introduce malloc_span attribute #167010
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1839,6 +1839,38 @@ static void handleRestrictAttr(Sema &S, Decl *D, const ParsedAttr &AL) { | |
| RestrictAttr(S.Context, AL, DeallocE, DeallocPtrIdx)); | ||
| } | ||
|
|
||
| static bool isSpanLikeType(const QualType &Ty) { | ||
| // Check that the type is a plain record with the first field being a pointer | ||
| // type and the second field being an integer. This matches the common | ||
| // implementation of std::span or sized_allocation_t in P0901R11. | ||
| // Note that there may also be numerous cases of pointer+integer structures | ||
| // not actually exhibiting a span-like semantics, so sometimes | ||
| // this heuristic expectedly leads to false positive results. | ||
| const RecordDecl *RD = Ty->getAsRecordDecl(); | ||
| if (!RD || RD->isUnion()) | ||
| return false; | ||
| const RecordDecl *Def = RD->getDefinition(); | ||
| if (!Def) | ||
| return false; // This is an incomplete type. | ||
| auto FieldsBegin = Def->field_begin(); | ||
| if (std::distance(FieldsBegin, Def->field_end()) != 2) | ||
| return false; | ||
| const FieldDecl *FirstField = *FieldsBegin; | ||
| const FieldDecl *SecondField = *std::next(FieldsBegin); | ||
| return FirstField->getType()->isAnyPointerType() && | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be easy to allow both orderings (ptr+size, size+ptr). For the Is there any technical reason we should not allow this now? Because I fear in future someone may want this, and then we're stuck again with version checks.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we are to relax this requirement as well, I wonder if we should go one step further and also lift the restriction on having just 2 fields in this struct, i.e. only ensure that the struct has some pointer field and (at least one) integer field.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That on the other hand will cause more complexity, because then you have to potentially walk a large struct and find those fields. Also later where you may want to use this to improve codegen, it would have to go and find the pointer - what if there are 2 pointers? I think if we talk about "span-like struct" it's exactly that: a struct with a pointer + size. Nothing more or less. But I think order of these members does not invalidate span semantics. On the other hand if you have a large struct with many members, is it a span? I can't tell. |
||
| SecondField->getType()->isIntegerType(); | ||
| } | ||
|
|
||
| static void handleMallocSpanAttr(Sema &S, Decl *D, const ParsedAttr &AL) { | ||
| QualType ResultType = getFunctionOrMethodResultType(D); | ||
| if (!isSpanLikeType(ResultType)) { | ||
| S.Diag(AL.getLoc(), diag::warn_attribute_return_span_only) | ||
| << AL << getFunctionOrMethodResultSourceRange(D); | ||
| return; | ||
| } | ||
| D->addAttr(::new (S.Context) MallocSpanAttr(S.Context, AL)); | ||
| } | ||
|
|
||
| static void handleCPUSpecificAttr(Sema &S, Decl *D, const ParsedAttr &AL) { | ||
| // Ensure we don't combine these with themselves, since that causes some | ||
| // confusing behavior. | ||
|
|
@@ -7278,6 +7310,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, | |
| case ParsedAttr::AT_Restrict: | ||
| handleRestrictAttr(S, D, AL); | ||
| break; | ||
| case ParsedAttr::AT_MallocSpan: | ||
| handleMallocSpanAttr(S, D, AL); | ||
| break; | ||
| case ParsedAttr::AT_Mode: | ||
| handleModeAttr(S, D, AL); | ||
| break; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| // RUN: %clang_cc1 -verify -fsyntax-only %s | ||
| // RUN: %clang_cc1 -emit-llvm -o %t %s | ||
|
|
||
| #include <stddef.h> | ||
|
|
||
| typedef struct { | ||
| void *ptr; | ||
| size_t n; | ||
| } sized_ptr; | ||
| sized_ptr returns_sized_ptr (void) __attribute((malloc_span)); // no-warning | ||
|
|
||
| // The first struct field must be pointer and the second must be an integer. | ||
| // Check the possible ways to violate it. | ||
| typedef struct { | ||
| size_t n; | ||
| void *ptr; | ||
| } invalid_span1; | ||
| invalid_span1 returns_non_std_span1 (void) __attribute((malloc_span)); // expected-warning {{attribute only applies to return values that are span-like structures}} | ||
|
|
||
| typedef struct { | ||
| void *ptr; | ||
| void *ptr2; | ||
| } invalid_span2; | ||
| invalid_span2 returns_non_std_span2 (void) __attribute((malloc_span)); // expected-warning {{attribute only applies to return values that are span-like structures}} | ||
|
|
||
| typedef struct { | ||
| void *ptr; | ||
| size_t n; | ||
| size_t n2; | ||
| } invalid_span3; | ||
| invalid_span3 returns_non_std_span3 (void) __attribute((malloc_span)); // expected-warning {{attribute only applies to return values that are span-like structures}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.