Skip to content

[clang] Ignore GCC 11 [[malloc(x)]] attribute #68059

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

Closed
wants to merge 5 commits into from
Closed

[clang] Ignore GCC 11 [[malloc(x)]] attribute #68059

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a76561f
[clang] Ignore GCC 11 [[malloc(x)]] attribute
aloisklink Oct 2, 2023
02a153d
fixup! [clang] Ignore GCC 11 [[malloc(x)]] attribute
aloisklink Oct 29, 2023
b7e1258
fixup! [clang] Ignore GCC 11 [[malloc(x)]] attribute
aloisklink Nov 1, 2023
5f6f893
fixup! [clang] Ignore GCC 11 [[malloc(x)]] attribute
aloisklink Nov 1, 2023
0659620
fixup! [clang] Ignore GCC 11 [[malloc(x)]] attribute
aloisklink Nov 1, 2023
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
5 changes: 5 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,11 @@ Bug Fixes to Compiler Builtins
Bug Fixes to Attribute Support
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

- Clang now emits a warning instead of an error when using the one or two
argument form of GCC 11's ``__attribute__((malloc(deallocator)))``
or ``__attribute__((malloc(deallocator, ptr-index)))``
(`#51607 <https://github.com/llvm/llvm-project/issues/51607>`).
Copy link
Collaborator

@AaronBallman AaronBallman Oct 11, 2023

Choose a reason for hiding this comment

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

Suggested change
(`#51607 <https://github.com/llvm/llvm-project/issues/51607>`).
(`#51607 <https://github.com/llvm/llvm-project/issues/51607>`_).

Copy link
Contributor Author

@aloisklink aloisklink Nov 1, 2023

Choose a reason for hiding this comment

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

Sorry! That's my fault for being lazy and skipping build the docs! Fixed in 5f6f893


Bug Fixes to C++ Support
^^^^^^^^^^^^^^^^^^^^^^^^

Expand Down
2 changes: 2 additions & 0 deletions clang/include/clang/Basic/Attr.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1629,6 +1629,8 @@ def IFunc : Attr, TargetSpecificAttr<TargetELF> {

def Restrict : InheritableAttr {
let Spellings = [Declspec<"restrict">, GCC<"malloc">];
let Args = [IdentifierArgument<"Deallocator", /*opt*/ 1>,
ParamIdxArgument<"DeallocatorPtrArgIndex", /*opt*/ 1>];
Copy link
Collaborator

@shafik shafik Oct 9, 2023

Choose a reason for hiding this comment

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

Suggested change
let Args = [IdentifierArgument<"Deallocator", /*opt*/ 1>,
ParamIdxArgument<"DeallocatorPtrArgIndex", /*opt*/ 1>];
let Args = [IdentifierArgument<"Deallocator", /*opt=*/1>,
ParamIdxArgument<"DeallocatorPtrArgIndex", /*opt=*/ 1>];

Copy link
Collaborator

@shafik shafik Oct 9, 2023

Choose a reason for hiding this comment

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

Nit: Unfortunately we have not been consistent through our codebase in argument comment format but e should strive to be consistent with bugprone-argument-comment

Copy link
Contributor Author

@aloisklink aloisklink Nov 1, 2023

Choose a reason for hiding this comment

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

👍 Fixed in b7e1258

let Subjects = SubjectList<[Function]>;
let Documentation = [RestrictDocs];
}
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/AttrDocs.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4122,6 +4122,9 @@ def RestrictDocs : Documentation {
The ``malloc`` attribute indicates that the function acts like a system memory
allocation function, returning a pointer to allocated storage disjoint from the
storage for any other object accessible to the caller.

The form of ``malloc`` with one or two arguments (supported by GCC 11) is
currently ignored by Clang.
Copy link
Contributor

@tbaederr tbaederr Oct 3, 2023

Choose a reason for hiding this comment

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

Is it really ignored or is it just treated as if it had no arguments?

Copy link
Contributor Author

@aloisklink aloisklink Oct 3, 2023

Choose a reason for hiding this comment

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

In this PR, it's completely ignored, it's not even added to the Clang AST.

It just reaches the end of this function where nothing happens (let me know if I can improve the comment!):

https://github.com/llvm/llvm-project/blob/f9c914729a5f5ac7f8b61ea2d39509ff0236a228/clang/lib/Sema/SemaDeclAttr.cpp#L2084-L2087

We can't treat it as if has no arguments because in GCC it means two different things:

  • __attribute__((malloc)) with no args means the return value is guaranteed not to alias to any other pointer (aka like __declspec(restrict) in MSVC) and that the function will normally return non-NULL, allowing optimizations,
  • __attribute__((malloc(deallocator))) instead says the returned pointer should be deallocated by the specified deallocator, allowing a static analyzer to print warnings.

See the malloc section of https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html for more info.

To be honest, I feel like GCC should have given the one/two argument form a different attribute name to make things less confusing, but GCC 11 has already been out for years unfortunately.

Copy link
Collaborator

@erichkeane erichkeane Feb 25, 2025

Choose a reason for hiding this comment

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

I would like it if we updated the documentation here to be something more like:

The ``malloc`` attribute has two forms with different functionality. The first
is when it is used without arguments, where it marks that a function acts like
a system memory allocation function, returning a pointer to allocated storage
that does not alias storage from any other object accessible to the caller.

The second form is when ``malloc`` takes one or two arguments. The first argument
names a function that should be associated with this function as its deallocation 
function. When this form is used, it enables the compiler to diagnose when the
incorrect deallocation function is used with this variable. However the associated
warning, spelled `-Wmismatched-dealloc` in GCC, is not yet implemented in clang.

}];
}

Expand Down
4 changes: 4 additions & 0 deletions clang/include/clang/Basic/DiagnosticCommonKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,10 @@ def warn_unknown_attribute_ignored : Warning<
"unknown attribute %0 ignored">, InGroup<UnknownAttributes>;
def warn_attribute_ignored : Warning<"%0 attribute ignored">,
InGroup<IgnoredAttributes>;
def warn_multiarg_malloc_attribute_ignored: Warning<
"'malloc' attribute ignored because"
" Clang does not support the one/two argument form">,
InGroup<IgnoredAttributes>;
Copy link
Collaborator

@AaronBallman AaronBallman Oct 11, 2023

Choose a reason for hiding this comment

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

Suggested change
def warn_multiarg_malloc_attribute_ignored: Warning<
"'malloc' attribute ignored because"
" Clang does not support the one/two argument form">,
InGroup<IgnoredAttributes>;
def warn_attribute_form_ignored : Warning<
"%0 attribute ignored; Clang does not yet support this attribute signature">,
InGroup<IgnoredAttributes>;

If we generalize the wording a bit, we can reuse this diagnostic for other circumstances where we don't yet support a particular signature.

Also, this diagnostic should be moved to DiagnosticSemaKinds.td (it's only diagnosed from clang/lib/Sema, so no need for it to be in the "common" diagnostics file for cross-lib diagnostics).

Copy link
Contributor Author

@aloisklink aloisklink Nov 1, 2023

Choose a reason for hiding this comment

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

Fixed in 02a153d, although I've very slightly changed the wording to match other warnings by using because instead of the ;.

I've also added a couple of other error diagnostics in other commit, so please let me know if you think it's worth generalizing them too.

def err_keyword_not_supported_on_target : Error<
"%0 is not supported on this target">;
def err_use_of_tag_name_without_tag : Error<
Expand Down
20 changes: 17 additions & 3 deletions clang/lib/Sema/SemaDeclAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2064,13 +2064,27 @@ static void handleTLSModelAttr(Sema &S, Decl *D, const ParsedAttr &AL) {

static void handleRestrictAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
QualType ResultType = getFunctionOrMethodResultType(D);
if (ResultType->isAnyPointerType() || ResultType->isBlockPointerType()) {
if (!ResultType->isAnyPointerType() && !ResultType->isBlockPointerType()) {
S.Diag(AL.getLoc(), diag::warn_attribute_return_pointers_only)
<< AL << getFunctionOrMethodResultSourceRange(D);
return;
}

if (getNumAttributeArgs(AL) == 0) {
D->addAttr(::new (S.Context) RestrictAttr(S.Context, AL));
return;
}

S.Diag(AL.getLoc(), diag::warn_attribute_return_pointers_only)
<< AL << getFunctionOrMethodResultSourceRange(D);
if (AL.getAttributeSpellingListIndex() == RestrictAttr::Declspec_restrict) {
// __declspec(restrict) accepts no arguments
S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << AL << 0;
return;
}

// FIXME: GCC uses [[malloc(my_func)]] to specify a deallocator for the
Copy link
Collaborator

@erichkeane erichkeane Oct 5, 2023

Choose a reason for hiding this comment

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

I think I'd like this to be diagnosed. We shouldn't be silently ignoring the attribute.

Copy link
Contributor Author

@aloisklink aloisklink Oct 7, 2023

Choose a reason for hiding this comment

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

I slightly disagree, since even if we do add support for this attribute, only the Clang analyzer would use it, but I've added a 'malloc' attribute ignored because Clang does not support the one/two argument form warning in commit a76561f.

// returned pointer, but this isn't currently supported in LLVM
// see https://github.com/llvm/llvm-project/issues/51607
S.Diag(AL.getLoc(), diag::warn_multiarg_malloc_attribute_ignored);
}

static void handleCPUSpecificAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
Expand Down
6 changes: 4 additions & 2 deletions clang/test/Sema/attr-args.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
// RUN: %clang_cc1 -verify -Wunused -Wused-but-marked-unused -Wunused-parameter -fsyntax-only %s
// RUN: %clang_cc1 -verify -Wunused -Wused-but-marked-unused -Wunused-parameter -fsyntax-only -fdeclspec %s
int a;

inline __attribute__((noreturn(a))) void *f1(void); // expected-error {{'noreturn' attribute takes no arguments}}
inline __attribute__((always_inline(a))) void *f2(void); // expected-error {{'always_inline' attribute takes no arguments}}
inline __attribute__((cdecl(a))) void *f3(void); // expected-error {{'cdecl' attribute takes no arguments}}
inline __attribute__((const(a))) void *f4(void); // expected-error {{'const' attribute takes no arguments}}
inline __attribute__((fastcall(a))) void *f5(void); // expected-error {{'fastcall' attribute takes no arguments}}
inline __attribute__((malloc(a))) void *f5(void); // expected-error {{'malloc' attribute takes no arguments}}
inline __declspec(restrict(a)) void *f6_a(void); // expected-error {{'restrict' attribute takes no arguments}}
inline __attribute__((malloc(a, 1, a))) void *f6_b(void); // expected-error {{'malloc' attribute takes no more than 2 arguments}}
inline __attribute__((malloc(a, 1))) void *f6_c(void); // expected-warning {{'malloc' attribute ignored because Clang does not support the one/two argument form}}
Copy link
Collaborator

@AaronBallman AaronBallman Oct 11, 2023

Choose a reason for hiding this comment

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

One more test I'd like to see is:

inline __attribute__((malloc(1))) void *func(void) // error: '1' does not name a function

Copy link
Contributor Author

@aloisklink aloisklink Nov 1, 2023

Choose a reason for hiding this comment

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

Fixed in 0659620, although with a different error diagnostic. I've also added a bunch more error checking (adapting the code from handleCleanupAttr), so now we should even be stricter than GCC on what args we accept:

llvm-project/clang/test/Sema/attr-args.c

Lines 2 to 6 in 0659620

int a;
void func_a(void * ptr, int a);
void func_b(int a);
void __attribute__((overloadable)) ambigious_func(void *); // expected-note {{candidate function}}
void __attribute__((overloadable)) ambigious_func(void *, int); // expected-note {{candidate function}}

llvm-project/clang/test/Sema/attr-args.c

Lines 13 to 21 in 0659620

inline __declspec(restrict(a)) void *f6_a(void); // expected-error {{'restrict' attribute takes no arguments}}
inline __attribute__((malloc(func_a, 1, a))) void *f6_b(void); // expected-error {{'malloc' attribute takes no more than 2 arguments}}
inline __attribute__((malloc(func_a, 1))) void *f6_c(void); // expected-warning {{'malloc' attribute ignored because Clang does not yet support this attribute signature}}
inline __attribute__((malloc(1234))) void *f6_d(void); // expected-error {{'malloc' argument for deallocator is not a function}}
inline __attribute__((malloc(a))) void *f6_e(void); // expected-error {{'malloc' argument 'a' is not a function}}
inline __attribute__((malloc(ambigious_func))) void *f6_f(void); // expected-error {{'malloc' argument 'ambigious_func' is not a single function}}
inline __attribute__((malloc(func_b))) void *f6_g(void); // expected-error {{'malloc' argument 'func_b' must take a pointer type as its first argument}}
inline __attribute__((malloc(func_a, 3))) void *f6_h(void); // expected-error {{'malloc' attribute parameter 2 is out of bounds}}
inline __attribute__((malloc(func_a, 2))) void *f6_i(void); // expected-error {{'malloc' argument '2' refers to non-pointer type 'int' of 'func_a'}}

inline __attribute__((nothrow(a))) void *f7(void); // expected-error {{'nothrow' attribute takes no arguments}}
inline __attribute__((stdcall(a))) void *f8(void); // expected-error {{'stdcall' attribute takes no arguments}}
inline __attribute__((used(a))) void *f9(void); // expected-error {{'used' attribute takes no arguments}}
Expand Down
8 changes: 8 additions & 0 deletions clang/test/SemaCXX/attr-print.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ int v __attribute__((visibility("hidden")));
// CHECK: char *PR24565() __attribute__((malloc))
char *PR24565() __attribute__((__malloc__));

void my_cleanup_func(char *);

// using __attribute__(malloc()) with args is currently ignored by Clang
// CHECK: char *PR52265_a()
__attribute__((malloc(my_cleanup_func))) char *PR52265_a();
// CHECK: char *PR52265_b()
__attribute__((malloc(my_cleanup_func, 1))) char *PR52265_b();

// CHECK: class __attribute__((consumable("unknown"))) AttrTester1
class __attribute__((consumable(unknown))) AttrTester1 {
// CHECK: void callableWhen() __attribute__((callable_when("unconsumed", "consumed")));
Expand Down