Fix __attribute__((force_align_arg_pointer)) misalignment bug

[Gramner]

Summary

The force_align_arg_pointer attribute was using a hardcoded 16-byte
alignment value which in combination with -mstack-alignment=32 (or
larger) would produce a misaligned stack which could result in crashes
when accessing stack buffers using aligned AVX load/store instructions.

Fix the issue by using the "stackrealign" function attribute instead
of using a hardcoded 16-byte alignment.

[erichkeane]

Can you make sure that this doesn't cause stackrealign to happen 2x if using -mstackrealign? Additionally, please provide the diff with full-context (see https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface). What is the commit before this? The line numbers don't look right to me.

Barring any surprises from those two, this looks alright to me.

[Gramner]

Full context.

[Gramner]

In D45812#1072066, @erichkeane wrote:

Can you make sure that this doesn't cause stackrealign to happen 2x if using -mstackrealign? Additionally, please provide the diff with full-context (see https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface). What is the commit before this? The line numbers don't look right to me.

Barring any surprises from those two, this looks alright to me.

The commit before this one is git hash 4889058f23607ff086470afe013082ca4a278ddb

It does "the right thing" as far as I can see, e.g. no double stack realignment or anything weird, when tested on 64-bit Linux with various combinations (and lack of thereof) of -mstack-alignment=32, -mstack-alignment=64, -mstackrealign, -m32. Unable to test on other systems though.

[erichkeane]

LGTM, Do you have commit access, or do you want me to commit it for you?

[Gramner]

In D45812#1072128, @erichkeane wrote:

LGTM, Do you have commit access, or do you want me to commit it for you?

I do not have commit access, so please do. Thanks for the review.

lib/CodeGen/TargetInfo.cpp

 }
 
 void X86_32TargetCodeGenInfo::setTargetAttributes(
     const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule &CGM) const {
   if (GV->isDeclaration())
     return;
   if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D)) {
     if (FD->hasAttr<X86ForceAlignArgPointerAttr>()) {
-      // Get the LLVM function.
       llvm::Function *Fn = cast<llvm::Function>(GV);
-
-      // Now add the 'alignstack' attribute with a value of 16.
-      llvm::AttrBuilder B;
-      B.addStackAlignmentAttr(16);
-      Fn->addAttributes(llvm::AttributeList::FunctionIndex, B);
+      Fn->addFnAttr("stackrealign");
     }
     if (FD->hasAttr<AnyX86InterruptAttr>()) {
       llvm::Function *Fn = cast<llvm::Function>(GV);
       Fn->setCallingConv(llvm::CallingConv::X86_INTR);
     }
   }
 }
 
public:
   }
 
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
                            CodeGen::CodeGenModule &CGM) const override {
     if (GV->isDeclaration())
       return;
     if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D)) {
       if (FD->hasAttr<X86ForceAlignArgPointerAttr>()) {
-        // Get the LLVM function.
-        auto *Fn = cast<llvm::Function>(GV);
-
-        // Now add the 'alignstack' attribute with a value of 16.
-        llvm::AttrBuilder B;
-        B.addStackAlignmentAttr(16);
-        Fn->addAttributes(llvm::AttributeList::FunctionIndex, B);
+        llvm::Function *Fn = cast<llvm::Function>(GV);
+        Fn->addFnAttr("stackrealign");
       }
       if (FD->hasAttr<AnyX86InterruptAttr>()) {
         llvm::Function *Fn = cast<llvm::Function>(GV);
         Fn->setCallingConv(llvm::CallingConv::X86_INTR);
       }
     }
   }
 };

 
 void WinX86_64TargetCodeGenInfo::setTargetAttributes(
     const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule &CGM) const {
   TargetCodeGenInfo::setTargetAttributes(D, GV, CGM);
   if (GV->isDeclaration())
     return;
   if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D)) {
     if (FD->hasAttr<X86ForceAlignArgPointerAttr>()) {
-      // Get the LLVM function.
-      auto *Fn = cast<llvm::Function>(GV);
-
-      // Now add the 'alignstack' attribute with a value of 16.
-      llvm::AttrBuilder B;
-      B.addStackAlignmentAttr(16);
-      Fn->addAttributes(llvm::AttributeList::FunctionIndex, B);
+      llvm::Function *Fn = cast<llvm::Function>(GV);
+      Fn->addFnAttr("stackrealign");
     }
     if (FD->hasAttr<AnyX86InterruptAttr>()) {
       llvm::Function *Fn = cast<llvm::Function>(GV);
       Fn->setCallingConv(llvm::CallingConv::X86_INTR);
     }
   }
 
   addStackProbeTargetAttributes(D, GV, CGM);

test/CodeGen/function-attributes.c

 // CHECK-LABEL: define void @f15
 // CHECK: [[NUW]]
 // CHECK: {
 void f15(void) {
 }
 
 // PR5254
 // CHECK-LABEL: define void @f16
-// CHECK: [[ALIGN:#[0-9]+]]
+// CHECK: [[SR:#[0-9]+]]
 // CHECK: {
 void __attribute__((force_align_arg_pointer)) f16(void) {
 }
 
 // PR11038
 // CHECK-LABEL: define void @f18()
 // CHECK: [[RT:#[0-9]+]]
 // CHECK: {

 int _setjmp(jmp_buf);
 void f20(void) {
   _setjmp(0);
 }
 
 // CHECK: attributes [[NUW]] = { nounwind optsize{{.*}} }
 // CHECK: attributes [[AI]] = { alwaysinline nounwind optsize{{.*}} }
 // CHECK: attributes [[NUW_OS_RN]] = { nounwind optsize readnone{{.*}} }
-// CHECK: attributes [[ALIGN]] = { nounwind optsize alignstack=16{{.*}} }
+// CHECK: attributes [[SR]] = { nounwind optsize{{.*}} "stackrealign"{{.*}} }
 // CHECK: attributes [[RT]] = { nounwind optsize returns_twice{{.*}} }
 // CHECK: attributes [[NR]] = { noreturn optsize }
 // CHECK: attributes [[NUW_RN]] = { nounwind optsize readnone }
 // CHECK: attributes [[RT_CALL]] = { optsize returns_twice }