[FunctionAttrs] Re-enable propagation of nonnull argument attributes from callsites to callers

[xbolva00]

Summary

Some time ago, this propagation was disabled due to some security concers:
http://lists.llvm.org/pipermail/cfe-dev/2017-January/052066.html

Later, this issue was fixed on Clang side in https://reviews.llvm.org/rC298491. This patch (re)enables it by default in LLVM.

[spatel]

Later, this issue was fixed on Clang side in https://reviews.llvm.org/rC298491. This patch (re)enables it by default in LLVM.

Repeating a comment from the now closed D30806 for reference:

But it got reverted soon after due to miscompiles:
https://reviews.llvm.org/rL298695

And the functionality hasn't been reimplemented AFAICT.

[xbolva00]

Ah, okay then. Closing this rev.

[xbolva00]

Btw, cannot we just drop nonnull arg info (which we get from libc annotations) related to problematic functions (like memcpy, strncmp,..) ?

And then we can mark these function args "safely" at callsites.

[spatel]

In D52635#1248976, @xbolva00 wrote:

Btw, cannot we just drop nonnull arg info (which we get from libc annotations) related to problematic functions (like memcpy, strncmp,..) ?

And then we can mark these function args "safely" at callsites.

I don't understand all of the details, so I recommend reviving D30806 or the dev-thread that led to it to discuss further how to proceed with this patch.
You might also be interested in applying/commandeering D29999 and chasing down how it broke everything. :)

lib/Transforms/IPO/FunctionAttrs.cpp

 STATISTIC(NumReturned, "Number of arguments marked returned");
 STATISTIC(NumReadNoneArg, "Number of arguments marked readnone");
 STATISTIC(NumReadOnlyArg, "Number of arguments marked readonly");
 STATISTIC(NumNoAlias, "Number of function returns marked noalias");
 STATISTIC(NumNonNullReturn, "Number of function returns marked nonnull");
 STATISTIC(NumNoRecurse, "Number of functions marked as norecurse");
 STATISTIC(NumNoUnwind, "Number of functions marked as nounwind");
 
-// 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."));
-
 static cl::opt<bool> DisableNoUnwindInference(
     "disable-nounwind-inference", cl::Hidden,
     cl::desc("Stop inferring nounwind attribute during function-attrs pass"));
 
 namespace {
 
 using SCCNodeSet = SmallSetVector<Function *, 8>;
 
static bool addArgumentReturnedAttrs(const SCCNodeSet &SCCNodes) {
   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

test/Transforms/FunctionAttrs/nonnull.ll

-; RUN: opt -S -functionattrs -enable-nonnull-arg-prop %s | FileCheck %s
-; RUN: opt -S -passes=function-attrs -enable-nonnull-arg-prop %s | FileCheck %s
+; RUN: opt -S -functionattrs %s | FileCheck %s
+; RUN: opt -S -passes=function-attrs %s | FileCheck %s
 
 declare nonnull i8* @ret_nonnull()
 
 ; Return a pointer trivially nonnull (call return attribute)
 define i8* @test1() {
 ; CHECK: define nonnull i8* @test1
   %ret = call i8* @ret_nonnull()
   ret i8* %ret