arichardson created this revision.
arichardson added reviewers: george.burgess.iv, aaron.ballman, jdenny.
Herald added a subscriber: cfe-commits.
I have been trying to statically find and analyze all calls to heap
allocation functions to determine how many of them use sizes known at
compile time vs only at runtime. While doing so I saw that quite a few
projects use replaceable function pointers for heap allocation and noticed
that clang was not able to annotate functions pointers with alloc_size.
I have changed the Sema checks to allow alloc_size on all function pointers
and typedefs for function pointers now and added checks that these
attributes are propagated to the LLVM IR correctly.
With this patch we can also compute __builtin_object_size() for calls to
allocation function pointers with the alloc_size attribute.
Repository:
rC Clang
https://reviews.llvm.org/D55212
Files:
include/clang/Basic/Attr.td
lib/AST/ExprConstant.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
test/CodeGen/alloc-size-fnptr.c
test/CodeGen/alloc-size.c
test/Sema/alloc-size.c
Index: test/Sema/alloc-size.c
===================================================================
--- test/Sema/alloc-size.c
+++ test/Sema/alloc-size.c
@@ -14,7 +14,7 @@
int fail9(int a) __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to return values that are pointers}}
-int fail10 __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to functions}}
+int fail10 __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
void *fail11(void *a) __attribute__((alloc_size(1))); //expected-error{{'alloc_size' attribute argument may only refer to a function parameter of integer type}}
@@ -22,4 +22,25 @@
void *fail12(int a) __attribute__((alloc_size(1, "abc"))); //expected-error{{'alloc_size' attribute requires parameter 2 to be an integer constant}}
void *fail13(int a) __attribute__((alloc_size(1U<<31))); //expected-error{{integer constant expression evaluates to value 2147483648 that cannot be represented in a 32-bit signed integer type}}
-int (*PR31453)(int) __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to functions}}
+void *(*PR31453)(int)__attribute__((alloc_size(1)));
+
+void *KR() __attribute__((alloc_size(1))); //expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
+
+// Applying alloc_size to function pointers should work:
+void *(__attribute__((alloc_size(1))) * func_ptr1)(int);
+void *(__attribute__((alloc_size(1, 2))) func_ptr2)(int, int);
+
+// TODO: according to GCC documentation the following should actually be the type
+// “pointer to pointer to alloc_size attributed function returning void*” and should
+// therefore be supported
+void *(__attribute__((alloc_size(1))) **ptr_to_func_ptr)(int); // expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
+// The following definitions apply the attribute to the pointer to the function pointer which should not be possible
+void *(*__attribute__((alloc_size(1))) * ptr_to_func_ptr2)(int); // expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
+void *(**__attribute__((alloc_size(1))) ptr_to_func_ptr2)(int); // expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
+
+// It should also work for typedefs:
+typedef void *(__attribute__((alloc_size(1))) allocator_function_typdef)(int);
+typedef void *(__attribute__((alloc_size(1, 2))) * allocator_function_typdef2)(int, int);
+void *(__attribute__((alloc_size(1, 2))) * allocator_function_typdef3)(int, int);
+// This typedef applies the alloc_size to the pointer to the function pointer and should not be allowed
+void *(**__attribute__((alloc_size(1, 2))) * allocator_function_typdef4)(int, int); // expected-warning{{'alloc_size' attribute only applies to non-K&R-style functions}}
Index: test/CodeGen/alloc-size.c
===================================================================
--- test/CodeGen/alloc-size.c
+++ test/CodeGen/alloc-size.c
@@ -1,3 +1,4 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm %s -o -
// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm %s -o - 2>&1 | FileCheck %s
#define NULL ((void *)0)
@@ -350,3 +351,128 @@
// CHECK: store i32 -1
gi = __builtin_object_size(my_signed_calloc(-2, 1), 0);
}
+
+void* (*malloc_function_pointer)(int) __attribute__((alloc_size(1)));
+void* (*calloc_function_pointer)(int, int) __attribute__((alloc_size(1, 2)));
+
+// CHECK-LABEL: @test_fn_pointer
+void test_fn_pointer() {
+ void *const vp = malloc_function_pointer(100);
+ // CHECK: store i32 100
+ gi = __builtin_object_size(vp, 0);
+ // CHECK: store i32 100
+ gi = __builtin_object_size(vp, 1);
+ // CHECK: store i32 100
+ gi = __builtin_object_size(vp, 2);
+ // CHECK: store i32 100
+ gi = __builtin_object_size(vp, 3);
+
+ void *const arr = calloc_function_pointer(100, 5);
+ // CHECK: store i32 500
+ gi = __builtin_object_size(arr, 0);
+ // CHECK: store i32 500
+ gi = __builtin_object_size(arr, 1);
+ // CHECK: store i32 500
+ gi = __builtin_object_size(arr, 2);
+ // CHECK: store i32 500
+ gi = __builtin_object_size(arr, 3);
+
+ // CHECK: store i32 100
+ gi = __builtin_object_size(malloc_function_pointer(100), 0);
+ // CHECK: store i32 100
+ gi = __builtin_object_size(malloc_function_pointer(100), 1);
+ // CHECK: store i32 100
+ gi = __builtin_object_size(malloc_function_pointer(100), 2);
+ // CHECK: store i32 100
+ gi = __builtin_object_size(malloc_function_pointer(100), 3);
+
+ // CHECK: store i32 500
+ gi = __builtin_object_size(calloc_function_pointer(100, 5), 0);
+ // CHECK: store i32 500
+ gi = __builtin_object_size(calloc_function_pointer(100, 5), 1);
+ // CHECK: store i32 500
+ gi = __builtin_object_size(calloc_function_pointer(100, 5), 2);
+ // CHECK: store i32 500
+ gi = __builtin_object_size(calloc_function_pointer(100, 5), 3);
+
+ void *const zeroPtr = malloc_function_pointer(0);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(zeroPtr, 0);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(malloc_function_pointer(0), 0);
+
+ void *const zeroArr1 = calloc_function_pointer(0, 1);
+ void *const zeroArr2 = calloc_function_pointer(1, 0);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(zeroArr1, 0);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(zeroArr2, 0);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(calloc_function_pointer(1, 0), 0);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(calloc_function_pointer(0, 1), 0);
+}
+
+typedef void *(__attribute__((warn_unused_result, alloc_size(1))) * my_malloc_function_pointer_type)(int);
+typedef void *(__attribute__((alloc_size(1, 2))) * my_calloc_function_pointer_type)(int, int);
+extern my_malloc_function_pointer_type malloc_function_pointer_with_typedef;
+extern my_calloc_function_pointer_type calloc_function_pointer_with_typedef;
+
+// CHECK-LABEL: @test_fn_pointer_typedef
+void test_fn_pointer_typedef() {
+ malloc_function_pointer_with_typedef(100);
+ void *const vp = malloc_function_pointer_with_typedef(100);
+ // CHECK: store i32 100
+ gi = __builtin_object_size(vp, 0);
+ // CHECK: store i32 100
+ gi = __builtin_object_size(vp, 1);
+ // CHECK: store i32 100
+ gi = __builtin_object_size(vp, 2);
+ // CHECK: store i32 100
+ gi = __builtin_object_size(vp, 3);
+
+ void *const arr = calloc_function_pointer_with_typedef(100, 5);
+ // CHECK: store i32 500
+ gi = __builtin_object_size(arr, 0);
+ // CHECK: store i32 500
+ gi = __builtin_object_size(arr, 1);
+ // CHECK: store i32 500
+ gi = __builtin_object_size(arr, 2);
+ // CHECK: store i32 500
+ gi = __builtin_object_size(arr, 3);
+
+ // CHECK: store i32 100
+ gi = __builtin_object_size(malloc_function_pointer_with_typedef(100), 0);
+ // CHECK: store i32 100
+ gi = __builtin_object_size(malloc_function_pointer_with_typedef(100), 1);
+ // CHECK: store i32 100
+ gi = __builtin_object_size(malloc_function_pointer_with_typedef(100), 2);
+ // CHECK: store i32 100
+ gi = __builtin_object_size(malloc_function_pointer_with_typedef(100), 3);
+
+ // CHECK: store i32 500
+ gi = __builtin_object_size(calloc_function_pointer_with_typedef(100, 5), 0);
+ // CHECK: store i32 500
+ gi = __builtin_object_size(calloc_function_pointer_with_typedef(100, 5), 1);
+ // CHECK: store i32 500
+ gi = __builtin_object_size(calloc_function_pointer_with_typedef(100, 5), 2);
+ // CHECK: store i32 500
+ gi = __builtin_object_size(calloc_function_pointer_with_typedef(100, 5), 3);
+
+ void *const zeroPtr = malloc_function_pointer_with_typedef(0);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(zeroPtr, 0);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(malloc_function_pointer_with_typedef(0), 0);
+
+ void *const zeroArr1 = calloc_function_pointer_with_typedef(0, 1);
+ void *const zeroArr2 = calloc_function_pointer_with_typedef(1, 0);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(zeroArr1, 0);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(zeroArr2, 0);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(calloc_function_pointer_with_typedef(1, 0), 0);
+ // CHECK: store i32 0
+ gi = __builtin_object_size(calloc_function_pointer_with_typedef(0, 1), 0);
+}
Index: test/CodeGen/alloc-size-fnptr.c
===================================================================
--- /dev/null
+++ test/CodeGen/alloc-size-fnptr.c
@@ -0,0 +1,55 @@
+// Check that the alloc_size attribute is propagated to the call instruction
+// for both direct and indirect calls
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm %s -o - 2>&1 | FileCheck %s
+
+#define NULL ((void *)0)
+
+int gi;
+
+typedef unsigned long size_t;
+
+extern void *my_malloc(int) __attribute__((alloc_size(1)));
+extern void *my_calloc(int, int) __attribute__((alloc_size(1, 2)));
+
+// CHECK-LABEL: @call_direct
+void call_direct(void) {
+ my_malloc(50);
+ // CHECK: call i8* @my_malloc(i32 50) [[DIRECT_MALLOC_ATTR:#[0-9]+]]
+ my_calloc(1, 16);
+ // CHECK: call i8* @my_calloc(i32 1, i32 16) [[DIRECT_CALLOC_ATTR:#[0-9]+]]
+}
+
+extern void *(*malloc_function_pointer)(void *, int)__attribute__((alloc_size(2)));
+extern void *(*calloc_function_pointer)(void *, int, int)__attribute__((alloc_size(2, 3)));
+
+// CHECK-LABEL: @call_function_pointer
+void call_function_pointer(void) {
+ malloc_function_pointer(NULL, 100);
+ // CHECK: [[MALLOC_FN_PTR:%.+]] = load i8* (i8*, i32)*, i8* (i8*, i32)** @malloc_function_pointer, align 8
+ // CHECK: call i8* [[MALLOC_FN_PTR]](i8* null, i32 100) [[INDIRECT_MALLOC_ATTR:#[0-9]+]]
+ calloc_function_pointer(NULL, 2, 4);
+ // CHECK: [[CALLOC_FN_PTR:%.+]] = load i8* (i8*, i32, i32)*, i8* (i8*, i32, i32)** @calloc_function_pointer, align 8
+ // CHECK: call i8* [[CALLOC_FN_PTR]](i8* null, i32 2, i32 4) [[INDIRECT_CALLOC_ATTR:#[0-9]+]]
+}
+
+typedef void *(__attribute__((alloc_size(3))) * my_malloc_fn_pointer_type)(void *, void *, int);
+typedef void *(__attribute__((alloc_size(3, 4))) * my_calloc_fn_pointer_type)(void *, void *, int, int);
+extern my_malloc_fn_pointer_type malloc_function_pointer_with_typedef;
+extern my_calloc_fn_pointer_type calloc_function_pointer_with_typedef;
+
+// CHECK-LABEL: @call_function_pointer_typedef
+void call_function_pointer_typedef(void) {
+ malloc_function_pointer_with_typedef(NULL, NULL, 200);
+ // CHECK: [[INDIRECT_TYPEDEF_MALLOC_FN_PTR:%.+]] = load i8* (i8*, i8*, i32)*, i8* (i8*, i8*, i32)** @malloc_function_pointer_with_typedef, align 8
+ // CHECK: call i8* [[INDIRECT_TYPEDEF_MALLOC_FN_PTR]](i8* null, i8* null, i32 200) [[INDIRECT_TYPEDEF_MALLOC_ATTR:#[0-9]+]]
+ calloc_function_pointer_with_typedef(NULL, NULL, 8, 4);
+ // CHECK: [[INDIRECT_TYPEDEF_CALLOC_FN_PTR:%.+]] = load i8* (i8*, i8*, i32, i32)*, i8* (i8*, i8*, i32, i32)** @calloc_function_pointer_with_typedef, align 8
+ // CHECK: call i8* [[INDIRECT_TYPEDEF_CALLOC_FN_PTR]](i8* null, i8* null, i32 8, i32 4) [[INDIRECT_TYPEDEF_CALLOC_ATTR:#[0-9]+]]
+}
+
+// CHECK: attributes [[DIRECT_MALLOC_ATTR]] = { allocsize(0) }
+// CHECK: attributes [[DIRECT_CALLOC_ATTR]] = { allocsize(0,1) }
+// CHECK: attributes [[INDIRECT_MALLOC_ATTR]] = { allocsize(1) }
+// CHECK: attributes [[INDIRECT_CALLOC_ATTR]] = { allocsize(1,2) }
+// CHECK: attributes [[INDIRECT_TYPEDEF_MALLOC_ATTR]] = { allocsize(2) }
+// CHECK: attributes [[INDIRECT_TYPEDEF_CALLOC_ATTR]] = { allocsize(2,3) }
Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -810,20 +810,20 @@
///
/// AttrArgNo is used to actually retrieve the argument, so it's base-0.
template <typename AttrInfo>
-static bool checkParamIsIntegerType(Sema &S, const FunctionDecl *FD,
- const AttrInfo &AI, unsigned AttrArgNo) {
+static bool checkParamIsIntegerType(Sema &S, const Decl *D, const AttrInfo &AI,
+ unsigned AttrArgNo) {
assert(AI.isArgExpr(AttrArgNo) && "Expected expression argument");
Expr *AttrArg = AI.getArgAsExpr(AttrArgNo);
ParamIdx Idx;
- if (!checkFunctionOrMethodParameterIndex(S, FD, AI, AttrArgNo + 1, AttrArg,
+ if (!checkFunctionOrMethodParameterIndex(S, D, AI, AttrArgNo + 1, AttrArg,
Idx))
return false;
- const ParmVarDecl *Param = FD->getParamDecl(Idx.getASTIndex());
- if (!Param->getType()->isIntegerType() && !Param->getType()->isCharType()) {
+ QualType ParamTy = getFunctionOrMethodParamType(D, Idx.getASTIndex());
+ if (!ParamTy->isIntegerType() && !ParamTy->isCharType()) {
SourceLocation SrcLoc = AttrArg->getBeginLoc();
S.Diag(SrcLoc, diag::err_attribute_integers_only)
- << AI << Param->getSourceRange();
+ << AI << getFunctionOrMethodParamRange(D, Idx.getASTIndex());
return false;
}
return true;
@@ -834,8 +834,10 @@
!checkAttributeAtMostNumArgs(S, AL, 2))
return;
- const auto *FD = cast<FunctionDecl>(D);
- if (!FD->getReturnType()->isPointerType()) {
+ assert(isFunctionOrMethod(D) && hasFunctionProto(D));
+
+ QualType RetTy = getFunctionOrMethodResultType(D);
+ if (!RetTy->isPointerType()) {
S.Diag(AL.getLoc(), diag::warn_attribute_return_pointers_only) << AL;
return;
}
@@ -845,7 +847,7 @@
// Parameter indices are 1-indexed, hence Index=1
if (!checkPositiveIntArgument(S, AL, SizeExpr, SizeArgNoVal, /*Index=*/1))
return;
- if (!checkParamIsIntegerType(S, FD, AL, /*AttrArgNo=*/0))
+ if (!checkParamIsIntegerType(S, D, AL, /*AttrArgNo=*/0))
return;
ParamIdx SizeArgNo(SizeArgNoVal, D);
@@ -856,7 +858,7 @@
// Parameter indices are 1-based, hence Index=2
if (!checkPositiveIntArgument(S, AL, NumberExpr, Val, /*Index=*/2))
return;
- if (!checkParamIsIntegerType(S, FD, AL, /*AttrArgNo=*/1))
+ if (!checkParamIsIntegerType(S, D, AL, /*AttrArgNo=*/1))
return;
NumberArgNo = ParamIdx(Val, D);
}
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -6264,6 +6264,16 @@
llvm_unreachable("Unknown type of decl!");
}
+template <typename AttrTy>
+static void copyAttrFromTypedefToDecl(Sema &S, Decl *D, const TypedefType *TT) {
+ TypedefNameDecl *TND = TT->getDecl();
+ if (AttrTy *Attribute = TND->getAttr<AttrTy>()) {
+ AttrTy *Clone = Attribute->clone(S.Context);
+ Clone->setInherited(true);
+ D->addAttr(Clone);
+ }
+}
+
NamedDecl *Sema::ActOnVariableDeclarator(
Scope *S, Declarator &D, DeclContext *DC, TypeSourceInfo *TInfo,
LookupResult &Previous, MultiTemplateParamsArg TemplateParamLists,
@@ -6706,6 +6716,15 @@
// Handle attributes prior to checking for duplicates in MergeVarDecl
ProcessDeclAttributes(S, NewVD, D);
+ // FIXME: this is probably the wrong location to be doing this and we should
+ // probably be doing this for more attributes (especially for function
+ // pointer attributes (such as format, warn_unused_result, etc)
+ if (R->isFunctionPointerType()) {
+ if (auto TT = R->getAs<TypedefType>()) {
+ copyAttrFromTypedefToDecl<AllocSizeAttr>(*this, NewVD, TT);
+ }
+ }
+
if (getLangOpts().CUDA || getLangOpts().OpenMPIsDevice) {
if (EmitTLSUnsupportedError &&
((getLangOpts().CUDA && DeclAttrsMatchCUDAMode(getLangOpts(), NewVD)) ||
Index: lib/AST/ExprConstant.cpp
===================================================================
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -129,8 +129,11 @@
/// Given a CallExpr, try to get the alloc_size attribute. May return null.
static const AllocSizeAttr *getAllocSizeAttr(const CallExpr *CE) {
- const FunctionDecl *Callee = CE->getDirectCallee();
- return Callee ? Callee->getAttr<AllocSizeAttr>() : nullptr;
+ if (const FunctionDecl *DirectCallee = CE->getDirectCallee())
+ return DirectCallee->getAttr<AllocSizeAttr>();
+ if (const Decl *IndirectCallee = CE->getCalleeDecl())
+ return IndirectCallee->getAttr<AllocSizeAttr>();
+ return nullptr;
}
/// Attempts to unwrap a CallExpr (with an alloc_size attribute) from an Expr.
Index: include/clang/Basic/Attr.td
===================================================================
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -1070,7 +1070,7 @@
def AllocSize : InheritableAttr {
let Spellings = [GCC<"alloc_size">];
- let Subjects = SubjectList<[Function]>;
+ let Subjects = SubjectList<[HasFunctionProto]>;
let Args = [ParamIdxArgument<"ElemSizeParam">,
ParamIdxArgument<"NumElemsParam", /*opt*/ 1>];
let TemplateDependent = 1;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
