[llvm] 1170743 - [inferattrs] Don't infer lib func attributes for nobuiltin functions

[Philip Reames via llvm-commits]

Author: Philip Reames
Date: 2021-04-16T15:36:15-07:00
New Revision: 11707435ccb44a9377bfed407453e0646a159636

URL: https://github.com/llvm/llvm-project/commit/11707435ccb44a9377bfed407453e0646a159636
DIFF: https://github.com/llvm/llvm-project/commit/11707435ccb44a9377bfed407453e0646a159636.diff

LOG: [inferattrs] Don't infer lib func attributes for nobuiltin functions

If we have a nobuiltin function, we can't assume we know anything about the implementation.

I noticed this when tracing through a log from an in the wild miscompile (https://github.com/emscripten-core/emscripten/issues/9443) triggered after 8666463.  We were incorrectly assuming that a custom allocator could not free.  (It's not clear yet this is the only problem in said issue.)

I also noticed something similiar mentioned in the commit message of ab243e when scrolling back through history.  Through, from what I can tell, that commit fixed symptom not root cause.

The interface we have for library function detection is extremely error prone, but given the interaction between ``nobuiltin`` decls and ``builtin`` callsites, it's really hard to imagine something much cleaner.  I may iterate on that, but it'll be invasive enough I didn't want to hold an obvious functional fix on it.

Added: 
    llvm/test/Transforms/InferFunctionAttrs/nobuiltin.ll

Modified: 
    llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp b/llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp
index 30402f109f302..c32e09875a12b 100644
--- a/llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp
+++ b/llvm/lib/Transforms/IPO/InferFunctionAttrs.cpp
@@ -31,7 +31,8 @@ static bool inferAllPrototypeAttributes(
     // attribute logic on all calls to declarations (as declarations aren't
     // explicitly visited by CGSCC passes in the new pass manager.)
     if (F.isDeclaration() && !F.hasOptNone()) {
-      Changed |= inferLibFuncAttributes(F, GetTLI(F));
+      if (!F.hasFnAttribute(Attribute::NoBuiltin))
+        Changed |= inferLibFuncAttributes(F, GetTLI(F));
       Changed |= inferAttributesFromOthers(F);
     }
 

diff  --git a/llvm/test/Transforms/InferFunctionAttrs/nobuiltin.ll b/llvm/test/Transforms/InferFunctionAttrs/nobuiltin.ll
new file mode 100644
index 0000000000000..1239a22a3a05c
--- /dev/null
+++ b/llvm/test/Transforms/InferFunctionAttrs/nobuiltin.ll
@@ -0,0 +1,5 @@
+; RUN: opt -S -inferattrs < %s | FileCheck %s
+
+; CHECK: Function Attrs: nobuiltin allocsize(0)
+; CHECK: declare i8* @_Znwm(i32)
+declare i8* @_Znwm(i32) nobuiltin allocsize(0)