[llvm] r294998 - [FunctionAttrs] try to extend nonnull-ness of arguments from a callsite back to its parent function
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 13 15:10:52 PST 2017
Author: spatel
Date: Mon Feb 13 17:10:51 2017
New Revision: 294998
URL: http://llvm.org/viewvc/llvm-project?rev=294998&view=rev
Log:
[FunctionAttrs] try to extend nonnull-ness of arguments from a callsite back to its parent function
As discussed here:
http://lists.llvm.org/pipermail/llvm-dev/2016-December/108182.html
...we should be able to propagate 'nonnull' info from a callsite back to its parent.
The original motivation for this patch is our botched optimization of "dyn_cast" (PR28430),
but this won't solve that problem.
The transform is currently disabled by default while we wait for clang to work-around
potential security problems:
http://lists.llvm.org/pipermail/cfe-dev/2017-January/052066.html
Differential Revision: https://reviews.llvm.org/D27855
Modified:
llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp
llvm/trunk/test/Transforms/FunctionAttrs/nonnull.ll
Modified: llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp?rev=294998&r1=294997&r2=294998&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp Mon Feb 13 17:10:51 2017
@@ -49,6 +49,14 @@ STATISTIC(NumNoAlias, "Number of functio
STATISTIC(NumNonNullReturn, "Number of function returns marked nonnull");
STATISTIC(NumNoRecurse, "Number of functions marked as norecurse");
+// FIXME: This is disabled by default to avoid exposing security vulnerabilities
+// in C/C++ code compiled by clang:
+// http://lists.llvm.org/pipermail/cfe-dev/2017-January/052066.html
+static cl::opt<bool> EnableNonnullArgPropagation(
+ "enable-nonnull-arg-prop", cl::Hidden,
+ cl::desc("Try to propagate nonnull argument attributes from callsites to "
+ "caller functions."));
+
namespace {
typedef SmallSetVector<Function *, 8> SCCNodeSet;
}
@@ -531,6 +539,49 @@ static bool addArgumentReturnedAttrs(con
return Changed;
}
+/// If a callsite has arguments that are also arguments to the parent function,
+/// try to propagate attributes from the callsite's arguments to the parent's
+/// arguments. This may be important because inlining can cause information loss
+/// when attribute knowledge disappears with the inlined call.
+static bool addArgumentAttrsFromCallsites(Function &F) {
+ if (!EnableNonnullArgPropagation)
+ return false;
+
+ bool Changed = false;
+
+ // For an argument attribute to transfer from a callsite to the parent, the
+ // call must be guaranteed to execute every time the parent is called.
+ // Conservatively, just check for calls in the entry block that are guaranteed
+ // to execute.
+ // TODO: This could be enhanced by testing if the callsite post-dominates the
+ // entry block or by doing simple forward walks or backward walks to the
+ // callsite.
+ BasicBlock &Entry = F.getEntryBlock();
+ for (Instruction &I : Entry) {
+ if (auto CS = CallSite(&I)) {
+ if (auto *CalledFunc = CS.getCalledFunction()) {
+ for (auto &CSArg : CalledFunc->args()) {
+ if (!CSArg.hasNonNullAttr())
+ continue;
+
+ // If the non-null callsite argument operand is an argument to 'F'
+ // (the caller) and the call is guaranteed to execute, then the value
+ // must be non-null throughout 'F'.
+ auto *FArg = dyn_cast<Argument>(CS.getArgOperand(CSArg.getArgNo()));
+ if (FArg && !FArg->hasNonNullAttr()) {
+ FArg->addAttr(Attribute::NonNull);
+ Changed = true;
+ }
+ }
+ }
+ }
+ if (!isGuaranteedToTransferExecutionToSuccessor(&I))
+ break;
+ }
+
+ return Changed;
+}
+
/// Deduce nocapture attributes for the SCC.
static bool addArgumentAttrs(const SCCNodeSet &SCCNodes) {
bool Changed = false;
@@ -549,6 +600,8 @@ static bool addArgumentAttrs(const SCCNo
if (!F->hasExactDefinition())
continue;
+ Changed |= addArgumentAttrsFromCallsites(*F);
+
// Functions that are readonly (or readnone) and nounwind and don't return
// a value can't capture arguments. Don't analyze them.
if (F->onlyReadsMemory() && F->doesNotThrow() &&
Modified: llvm/trunk/test/Transforms/FunctionAttrs/nonnull.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/nonnull.ll?rev=294998&r1=294997&r2=294998&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/FunctionAttrs/nonnull.ll (original)
+++ llvm/trunk/test/Transforms/FunctionAttrs/nonnull.ll Mon Feb 13 17:10:51 2017
@@ -1,4 +1,4 @@
-; RUN: opt -S -functionattrs %s | FileCheck %s
+; RUN: opt -S -functionattrs -enable-nonnull-arg-prop %s | FileCheck %s
declare nonnull i8* @ret_nonnull()
; Return a pointer trivially nonnull (call return attribute)
@@ -71,4 +71,148 @@ exit:
ret i8* %phi
}
+; Test propagation of nonnull callsite args back to caller.
+
+declare void @use1(i8* %x)
+declare void @use2(i8* %x, i8* %y);
+declare void @use3(i8* %x, i8* %y, i8* %z);
+
+declare void @use1nonnull(i8* nonnull %x);
+declare void @use2nonnull(i8* nonnull %x, i8* nonnull %y);
+declare void @use3nonnull(i8* nonnull %x, i8* nonnull %y, i8* nonnull %z);
+
+declare i8 @use1safecall(i8* %x) readonly nounwind ; readonly+nounwind guarantees that execution continues to successor
+
+; Can't extend non-null to parent for any argument because the 2nd call is not guaranteed to execute.
+
+define void @parent1(i8* %a, i8* %b, i8* %c) {
+; CHECK-LABEL: @parent1(i8* %a, i8* %b, i8* %c)
+; CHECK-NEXT: call void @use3(i8* %c, i8* %a, i8* %b)
+; CHECK-NEXT: call void @use3nonnull(i8* %b, i8* %c, i8* %a)
+; CHECK-NEXT: ret void
+;
+ call void @use3(i8* %c, i8* %a, i8* %b)
+ call void @use3nonnull(i8* %b, i8* %c, i8* %a)
+ ret void
+}
+
+; Extend non-null to parent for all arguments.
+
+define void @parent2(i8* %a, i8* %b, i8* %c) {
+; CHECK-LABEL: @parent2(i8* nonnull %a, i8* nonnull %b, i8* nonnull %c)
+; CHECK-NEXT: call void @use3nonnull(i8* %b, i8* %c, i8* %a)
+; CHECK-NEXT: call void @use3(i8* %c, i8* %a, i8* %b)
+; CHECK-NEXT: ret void
+;
+ call void @use3nonnull(i8* %b, i8* %c, i8* %a)
+ call void @use3(i8* %c, i8* %a, i8* %b)
+ ret void
+}
+
+; Extend non-null to parent for 1st argument.
+
+define void @parent3(i8* %a, i8* %b, i8* %c) {
+; CHECK-LABEL: @parent3(i8* nonnull %a, i8* %b, i8* %c)
+; CHECK-NEXT: call void @use1nonnull(i8* %a)
+; CHECK-NEXT: call void @use3(i8* %c, i8* %b, i8* %a)
+; CHECK-NEXT: ret void
+;
+ call void @use1nonnull(i8* %a)
+ call void @use3(i8* %c, i8* %b, i8* %a)
+ ret void
+}
+
+; Extend non-null to parent for last 2 arguments.
+
+define void @parent4(i8* %a, i8* %b, i8* %c) {
+; CHECK-LABEL: @parent4(i8* %a, i8* nonnull %b, i8* nonnull %c)
+; CHECK-NEXT: call void @use2nonnull(i8* %c, i8* %b)
+; CHECK-NEXT: call void @use2(i8* %a, i8* %c)
+; CHECK-NEXT: call void @use1(i8* %b)
+; CHECK-NEXT: ret void
+;
+ call void @use2nonnull(i8* %c, i8* %b)
+ call void @use2(i8* %a, i8* %c)
+ call void @use1(i8* %b)
+ ret void
+}
+
+; The callsite must execute in order for the attribute to transfer to the parent.
+; It appears benign to extend non-null to the parent in this case, but we can't do that
+; because it would incorrectly propagate the wrong information to its callers.
+
+define void @parent5(i8* %a, i1 %a_is_notnull) {
+; CHECK-LABEL: @parent5(i8* %a, i1 %a_is_notnull)
+; CHECK-NEXT: br i1 %a_is_notnull, label %t, label %f
+; CHECK: t:
+; CHECK-NEXT: call void @use1nonnull(i8* %a)
+; CHECK-NEXT: ret void
+; CHECK: f:
+; CHECK-NEXT: ret void
+;
+ br i1 %a_is_notnull, label %t, label %f
+t:
+ call void @use1nonnull(i8* %a)
+ ret void
+f:
+ ret void
+}
+
+; The callsite must execute in order for the attribute to transfer to the parent.
+; The volatile load might trap, so there's no guarantee that we'll ever get to the call.
+
+define i8 @parent6(i8* %a, i8* %b) {
+; CHECK-LABEL: @parent6(i8* %a, i8* %b)
+; CHECK-NEXT: [[C:%.*]] = load volatile i8, i8* %b
+; CHECK-NEXT: call void @use1nonnull(i8* %a)
+; CHECK-NEXT: ret i8 [[C]]
+;
+ %c = load volatile i8, i8* %b
+ call void @use1nonnull(i8* %a)
+ ret i8 %c
+}
+
+; The nonnull callsite is guaranteed to execute, so the argument must be nonnull throughout the parent.
+
+define i8 @parent7(i8* %a) {
+; CHECK-LABEL: @parent7(i8* nonnull %a)
+; CHECK-NEXT: [[RET:%.*]] = call i8 @use1safecall(i8* %a)
+; CHECK-NEXT: call void @use1nonnull(i8* %a)
+; CHECK-NEXT: ret i8 [[RET]]
+;
+ %ret = call i8 @use1safecall(i8* %a)
+ call void @use1nonnull(i8* %a)
+ ret i8 %ret
+}
+
+; Make sure that an invoke works similarly to a call.
+
+declare i32 @esfp(...)
+
+define i1 @parent8(i8* %a, i8* %bogus1, i8* %b) personality i8* bitcast (i32 (...)* @esfp to i8*){
+; CHECK-LABEL: @parent8(i8* nonnull %a, i8* nocapture readnone %bogus1, i8* nonnull %b)
+; CHECK-NEXT: entry:
+; CHECK-NEXT: invoke void @use2nonnull(i8* %a, i8* %b)
+; CHECK-NEXT: to label %cont unwind label %exc
+; CHECK: cont:
+; CHECK-NEXT: [[NULL_CHECK:%.*]] = icmp eq i8* %b, null
+; CHECK-NEXT: ret i1 [[NULL_CHECK]]
+; CHECK: exc:
+; CHECK-NEXT: [[LP:%.*]] = landingpad { i8*, i32 }
+; CHECK-NEXT: filter [0 x i8*] zeroinitializer
+; CHECK-NEXT: unreachable
+;
+entry:
+ invoke void @use2nonnull(i8* %a, i8* %b)
+ to label %cont unwind label %exc
+
+cont:
+ %null_check = icmp eq i8* %b, null
+ ret i1 %null_check
+
+exc:
+ %lp = landingpad { i8*, i32 }
+ filter [0 x i8*] zeroinitializer
+ unreachable
+}
More information about the llvm-commits
mailing list