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 all commits
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>`_).

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 = [ExprArgument<"Deallocator", /*opt=*/ 1>,
ParamIdxArgument<"DeallocatorPtrArgIndex", /*opt=*/ 1>];
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
10 changes: 10 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3028,6 +3028,10 @@ def warn_auto_var_is_id : Warning<
InGroup<DiagGroup<"auto-var-id">>;

// Attributes
def warn_attribute_form_ignored : Warning<
"%0 attribute ignored because Clang does not yet support this attribute signature">,
InGroup<IgnoredAttributes>;

def warn_attribute_ignored_no_calls_in_stmt: Warning<
"%0 attribute is ignored because there exists no call expression inside the "
"statement">,
Expand Down Expand Up @@ -4248,6 +4252,12 @@ def err_attribute_cleanup_func_must_take_one_arg : Error<
def err_attribute_cleanup_func_arg_incompatible_type : Error<
"'cleanup' function %0 parameter has "
"%diff{type $ which is incompatible with type $|incompatible type}1,2">;
def err_attribute_malloc_arg_not_function : Error<
"'malloc' argument %select{for deallocator |%1 |%1 }0is not a %select{||single }0function">;
def err_attribute_malloc_arg_not_function_with_pointer_arg : Error<
"'malloc' argument %0 must take a pointer type as its first argument">;
def err_attribute_malloc_arg_refers_to_non_pointer_type : Error<
"'malloc' argument '%0' refers to non-pointer type %1 of %2">; // in GCC, this is a warning, not an error
def err_attribute_regparm_wrong_platform : Error<
"'regparm' is not valid on this platform">;
def err_attribute_regparm_invalid_number : Error<
Expand Down
80 changes: 77 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,87 @@ 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 (AL.getNumArgs() == 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;
}

// [[gnu::malloc(deallocator)]] with args specifies a deallocator function
Expr *DeallocE = AL.getArgAsExpr(0);
SourceLocation DeallocLoc = DeallocE->getExprLoc();
FunctionDecl *DeallocFD = nullptr;
DeclarationNameInfo DeallocNI;

if (auto *DRE = dyn_cast<DeclRefExpr>(DeallocE)) {
DeallocFD = dyn_cast<FunctionDecl>(DRE->getDecl());
DeallocNI = DRE->getNameInfo();
if (!DeallocFD) {
S.Diag(DeallocLoc, diag::err_attribute_malloc_arg_not_function)
<< 1 << DeallocNI.getName();
return;
}
} else if (auto *ULE = dyn_cast<UnresolvedLookupExpr>(DeallocE)) {
DeallocFD = S.ResolveSingleFunctionTemplateSpecialization(ULE, true);
DeallocNI = ULE->getNameInfo();
if (!DeallocFD) {
S.Diag(DeallocLoc, diag::err_attribute_malloc_arg_not_function)
<< 2 << DeallocNI.getName();
if (ULE->getType() == S.Context.OverloadTy)
S.NoteAllOverloadCandidates(ULE);
return;
}
} else {
S.Diag(DeallocLoc, diag::err_attribute_malloc_arg_not_function) << 0;
return;
}

// 2nd arg of [[gnu::malloc(deallocator, 2)]] with args specifies the param
// of deallocator that deallocates the pointer (defaults to 1)
ParamIdx DeallocPtrIdx;
if (AL.getNumArgs() == 1) {
DeallocPtrIdx = ParamIdx(1, DeallocFD);

if (!DeallocPtrIdx.isValid() ||
!getFunctionOrMethodParamType(DeallocFD, DeallocPtrIdx.getASTIndex())
.getCanonicalType()
->isPointerType()) {
S.Diag(DeallocLoc,
diag::err_attribute_malloc_arg_not_function_with_pointer_arg)
<< DeallocNI.getName();
return;
}
} else {
if (!checkFunctionOrMethodParameterIndex(S, DeallocFD, AL, 2,
AL.getArgAsExpr(1), DeallocPtrIdx,
/* CanIndexImplicitThis=*/false))
return;

QualType DeallocPtrArgType =
getFunctionOrMethodParamType(DeallocFD, DeallocPtrIdx.getASTIndex());
if (!DeallocPtrArgType.getCanonicalType()->isPointerType()) {
S.Diag(DeallocLoc,
diag::err_attribute_malloc_arg_refers_to_non_pointer_type)
<< DeallocPtrIdx.getSourceIndex() << DeallocPtrArgType
<< DeallocNI.getName();
return;
}
}

// FIXME: we should add this attribute to Clang's AST, so that clang-analyzer
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 think we SHOULD still add this to the AST anyway, as it represents code still. However we should teach CGCall.cpp to no longer emit the 'no-alias'.

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.

Note this would require a codegen test!

// can use it.
S.Diag(AL.getLoc(), diag::warn_attribute_form_ignored) << AL;
}

static void handleCPUSpecificAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
Expand Down
16 changes: 14 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,24 @@
// 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;
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}}

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(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