From 8aee282f57f42773570fc5d29f03a03361ff7fb4 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers@google.com>
Date: Mon, 28 Jun 2021 13:53:55 -0700
Subject: [PATCH] [IR] remove assert since always_inline can appear on CallBase

I added an assertion in D91816 (documenting behavior added in D93422)
that callers and callees with mismatched fn attr's related to stack
protectors should not occur unless the callee was attributed
always_inline.

This falls apart when a call, invoke, or callbr (any instruction
inheriting from CallBase) itself has an always_inline attribute. Clang
will emit such attributes on Instructions when __attribute__((flatten))
is used to recursively force inlining from a caller.

Since these assertions only had the caller and callee Functions, and not
the call site (CallBase derived classes), we would have to search the
caller for such instructions to reconstruct the call site information.
But at that point, inlining has already occurred; the call site has
already been removed from the caller.

Remove the assertions, add a unit test for always_inline call sites, and
update the LangRef.

Another curiosity is that the always_inline Attribute on Instructions is
only expanded by the inline pass, not the always_inline pass.

Thanks to @pcc on this report when building Android's RunTime (ART)
interpreter.

Reviewed By: pcc, MaskRay

Differential Revision: https://reviews.llvm.org/D104944
---
 llvm/docs/LangRef.rst                       | 10 +++++++---
 llvm/lib/IR/Attributes.cpp                  | 10 ----------
 llvm/test/Transforms/Inline/inline_nossp.ll | 15 +++++++++++++++
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 083ece6..243e9ea 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -1922,7 +1922,8 @@ example:
     A function with the ``ssp`` attribute but without the ``alwaysinline``
     attribute cannot be inlined into a function without a
     ``ssp/sspreq/sspstrong`` attribute. If inlined, the caller will get the
-    ``ssp`` attribute.
+    ``ssp`` attribute. ``call``, ``invoke``, and ``callbr`` instructions with
+    the ``alwaysinline`` attribute force inlining.
 ``sspstrong``
     This attribute indicates that the function should emit a stack smashing
     protector. This attribute causes a strong heuristic to be used when
@@ -1950,7 +1951,9 @@ example:
     A function with the ``sspstrong`` attribute but without the
     ``alwaysinline`` attribute cannot be inlined into a function without a
     ``ssp/sspstrong/sspreq`` attribute. If inlined, the caller will get the
-    ``sspstrong`` attribute unless the ``sspreq`` attribute exists.
+    ``sspstrong`` attribute unless the ``sspreq`` attribute exists.  ``call``,
+    ``invoke``, and ``callbr`` instructions with the ``alwaysinline`` attribute
+    force inlining.
 ``sspreq``
     This attribute indicates that the function should *always* emit a stack
     smashing protector. This overrides the ``ssp`` and ``sspstrong`` function
@@ -1970,7 +1973,8 @@ example:
     A function with the ``sspreq`` attribute but without the ``alwaysinline``
     attribute cannot be inlined into a function without a
     ``ssp/sspstrong/sspreq`` attribute. If inlined, the caller will get the
-    ``sspreq`` attribute.
+    ``sspreq`` attribute.  ``call``, ``invoke``, and ``callbr`` instructions
+    with the ``alwaysinline`` attribute force inlining.
 
 ``strictfp``
     This attribute indicates that the function was called from a scope that
diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp
index daf0f17..27b5835 100644
--- a/llvm/lib/IR/Attributes.cpp
+++ b/llvm/lib/IR/Attributes.cpp
@@ -2159,16 +2159,6 @@ static void setOR(Function &Caller, const Function &Callee) {
 /// If the inlined function had a higher stack protection level than the
 /// calling function, then bump up the caller's stack protection level.
 static void adjustCallerSSPLevel(Function &Caller, const Function &Callee) {
-#ifndef NDEBUG
-  if (!Callee.hasFnAttribute(Attribute::AlwaysInline)) {
-    assert(!(!Callee.hasStackProtectorFnAttr() &&
-             Caller.hasStackProtectorFnAttr()) &&
-           "stack protected caller but callee requested no stack protector");
-    assert(!(!Caller.hasStackProtectorFnAttr() &&
-             Callee.hasStackProtectorFnAttr()) &&
-           "stack protected callee but caller requested no stack protector");
-  }
-#endif
   // If upgrading the SSP attribute, clear out the old SSP Attributes first.
   // Having multiple SSP attributes doesn't actually hurt, but it adds useless
   // clutter to the IR.
diff --git a/llvm/test/Transforms/Inline/inline_nossp.ll b/llvm/test/Transforms/Inline/inline_nossp.ll
index 586735e..2a4c8c6 100644
--- a/llvm/test/Transforms/Inline/inline_nossp.ll
+++ b/llvm/test/Transforms/Inline/inline_nossp.ll
@@ -35,3 +35,18 @@ define i32 @ssp_caller() sspstrong {
   %2 = call i32 @nossp_alwaysinline()
   ret i32 %2
 }
+
+; The alwaysinline attribute can also appear on the CallBase (ie. the call
+; site), ie. when __attribute__((flatten)) is used on the caller. Treat this
+; the same as if the caller had the fn attr alwaysinline and permit inline
+; substitution, despite the mismatch between caller and callee on ssp attrs.
+;
+; Curiously, the always_inline attribute on a CallInst is only expanded by the
+; inline pass, but not always_inline pass!
+define i32 @nossp_alwaysinline_caller() {
+; CHECK-INLINE-LABEL: @nossp_alwaysinline_caller(
+; CHECK-INLINE-NEXT:    ret i32 42
+;
+  %1 = call i32 @ssp() alwaysinline
+  ret i32 %1
+}
-- 
2.7.4