-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[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.
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
Changes from all commits
a76561f
02a153d
b7e1258
5f6f893
0659620
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 |
|---|---|---|
|
|
@@ -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 | ||
|
Collaborator
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. I think we SHOULD still add this to the AST anyway, as it represents code still. However we should teach
Collaborator
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. 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) { | ||
|
|
||
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.
Is it really ignored or is it just treated as if it had no arguments?
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.
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
mallocsection 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.
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.
I would like it if we updated the documentation here to be something more like: