[IR] remove assert since always_inline can appear on CallBase

[nickdesaulniers]

Summary

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.

[nickdesaulniers]

I've added optimization remark tests locally; inlining is failing due to:

remark: <unknown>:0:0: ssp not inlined into nossp because it should never be inlined (cost=never): stack protected callee but caller requested no stack protector
remark: <unknown>:0:0: ssp not inlined into nossp_alwaysinline because it should never be inlined (cost=never): stack protected callee but caller requested no stack protector
remark: <unknown>:0:0: ssp has uninlinable pattern (dynamic alloca) and cost is not fully computed
remark: <unknown>:0:0: ssp not inlined into nossp_caller because it should never be inlined (cost=never): dynamic alloca
remark: <unknown>:0:0: nossp has uninlinable pattern (dynamic alloca) and cost is not fully computed
remark: <unknown>:0:0: nossp not inlined into ssp2 because it should never be inlined (cost=never): dynamic alloca

the dynamic allocas mean this test, as introduced in D91816, might not be testing the right thing. We were getting the right results, but for the wrong reasons. Let me attempt to fix that first, then revisit this. It's confusing to me at the moment because the dynamic allocas are the whole point of having stack protectors, and we can and do inline such code, so it's not initially clear to my why inlining would fail due to that, at least with the tests as written.

[nickdesaulniers]

Let me rebase this on D104958.

[nickdesaulniers]

• rebase on D104958

[pcc]

LGTM

[jdoerfert]

FWIW, this seems sensible :)

llvm/docs/LangRef.rst

- Calls to alloca() with variable sizes or constant sizes greater than
       ``ssp-buffer-size``.
 
     Variables that are identified as requiring a protector will be arranged
     on the stack such that they are adjacent to the stack protector guard.
 
     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
     determining if a function needs stack protectors. The strong heuristic
     will enable protectors for functions with:
 
     - Arrays of any size and type
     - Aggregates containing an array of any size and type.
``sspstrong``
     #. Variables that have had their address taken are 3rd closest to the
        protector.
 
     This overrides the ``ssp`` function attribute.
 
     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
     attributes.
 
     Variables that are identified as requiring a protector will be arranged
     on the stack such that they are adjacent to the stack protector guard.
     The specific layout rules are:
 
     #. Large arrays and structures containing large arrays
        (``>= ssp-buffer-size``) are closest to the stack protector.
     #. Small arrays and structures containing small arrays
        (``< ssp-buffer-size``) are 2nd closest to the protector.
     #. Variables that have had their address taken are 3rd closest to the
        protector.
 
     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
     requires strict floating-point semantics.  LLVM will not attempt any
     optimizations that require assumptions about the floating-point rounding
     mode or that might alter the state of floating-point status flags that
     might otherwise be set or cleared by calling this function. LLVM will
     not introduce any new floating-point instructions that may trap.

llvm/lib/IR/Attributes.cpp

static void setOR(Function &Caller, const Function &Callee) {
   if (!AttrClass::isSet(Caller, AttrClass::getKind()) &&
       AttrClass::isSet(Callee, AttrClass::getKind()))
     AttrClass::set(Caller, AttrClass::getKind(), true);
 }
 
 /// 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.
   AttrBuilder OldSSPAttr;
   OldSSPAttr.addAttribute(Attribute::StackProtect)
       .addAttribute(Attribute::StackProtectStrong)
       .addAttribute(Attribute::StackProtectReq);
 

llvm/test/Transforms/Inline/inline_nossp.ll

 ; CHECK-LABEL: @ssp_caller(
 ; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @nossp()
 ; CHECK-NEXT:    ret i32 43
 ;
   call i32 @nossp()
   %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
+}