Don't emit prologues/epilogues for naked functions (PR18791, PR20028)

[hans]

Summary

For naked functions with parameters, Clang would emit prologues that end up clobbering the stack because LLVM doesn't set up a stack frame. For example, this function on X86:

attribute((naked)) int f(int x) {

asm("movl $42, %eax");
asm("retl");

}

Results in:

_Z1fi:

movl    12(%esp), %eax
movl    %eax, (%esp)  <--- Oops.
movl    $42, %eax
retl

(This was already reported as PR18791, and PR20028 for the epilogue.)

My patch does three things, which can be committed separately, but I figured it's easier to review them together:

• Don't emit prologues/epilogues for naked functions
• Don't allow non-asm statements in naked functions
• Don't allow asm statements to refer to parameters in naked functions.

Please take a look.

[compnerd]

Is supporting the attribute (( naked )) spelling on x86 intentional? It can certainly be a nice extension, but keep in mind that GCC only permits this on ARM, AVR, MCORE, MSP430, NDS32, RL78, RX and SPU ports. However, we need the __declspec(naked) spelling for MSVC compatibility. It might be nice to document that we support the attribute across the board rather than keep GCC compatibility.

[thakis]

lib/Sema/SemaStmtAsm.cpp

414

Do you need to Func->setInvalidDecl(); here too?

[rnk]

lgtm

lib/Sema/SemaStmtAsm.cpp

414

Let's do that.

test/CodeGen/attr-naked.c

21

This CHECK will fail in a non-asserts build, because the name 'entry' will be elided. I guess I'd just rely on the CHECK-NOT for alloca and store followed by CHECK unreachable.

[hans]

lib/Sema/SemaStmtAsm.cpp

414

I commented on the email thread that maybe "return ExprError()" is the right thing to do here. That seems to be how the rest of this function does it.

[hans]

Closed by commit rL217198 (authored by @hans).

include/clang/Basic/DiagnosticSemaKinds.td

 def err_unknown_any_var_function_type : Error<
   "variable %0 with unknown type cannot be given a function type">;
 def err_unknown_any_function : Error<
   "function %0 with unknown type must be given a function type">;
 
 def err_filter_expression_integral : Error<
   "filter expression type should be an integral value not %0">;
 
+def err_non_asm_stmt_in_naked_function : Error<
+  "non-ASM statement in naked function is not supported">;
+def err_asm_naked_parm_ref : Error<
+  "parameter references not allowed in naked functions">;
+
 // OpenCL warnings and errors.
 def err_invalid_astype_of_different_size : Error<
   "invalid reinterpretation: sizes of %0 and %1 must match">;
 def err_static_kernel : Error<
   "kernel functions cannot be declared static">;
 def err_opencl_ptrptr_kernel_param : Error<
   "kernel parameter cannot be declared as a pointer to a pointer">;
 def err_opencl_private_ptr_kernel_param : Error<

lib/CodeGen/CGCall.cpp

if (NNAttr->isNonNull(PVD->getFunctionScopeIndex()))
       return true;
   }
   return false;
 }
 
 void CodeGenFunction::EmitFunctionProlog(const CGFunctionInfo &FI,
                                          llvm::Function *Fn,
                                          const FunctionArgList &Args) {
+  if (CurCodeDecl && CurCodeDecl->hasAttr<NakedAttr>())
+    // Naked functions don't have prologues.
+    return;
+
   // If this is an implicit-return-zero function, go ahead and
   // initialize the return value.  TODO: it might be nice to have
   // a more general mechanism for this that didn't require synthesized
   // return statements.
   if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(CurCodeDecl)) {
     if (FD->hasImplicitReturnZero()) {
       QualType RetTy = FD->getReturnType().getUnqualifiedType();
       llvm::Type* LLVMTy = CGM.getTypes().ConvertType(RetTy);
static llvm::StoreInst *findDominatingStoreToReturnValue(CodeGenFunction &CGF) {
   // Okay, the store's basic block dominates the insertion point; we
   // can do our thing.
   return store;
 }
 
 void CodeGenFunction::EmitFunctionEpilog(const CGFunctionInfo &FI,
                                          bool EmitRetDbgLoc,
                                          SourceLocation EndLoc) {
+  if (CurCodeDecl && CurCodeDecl->hasAttr<NakedAttr>()) {
+    // Naked functions don't have epilogues.
+    Builder.CreateUnreachable();
+    return;
+  }
+
   // Functions with no result always return void.
   if (!ReturnValue) {
     Builder.CreateRetVoid();
     return;
   }
 
   llvm::DebugLoc RetDbgLoc;
   llvm::Value *RV = nullptr;

lib/Sema/SemaDecl.cpp

if (!getDiagnostics().hasUncompilableErrorOccurred() &&
       ActivePolicy = ℘
     }
 
     if (!IsInstantiation && FD && FD->isConstexpr() && !FD->isInvalidDecl() &&
         (!CheckConstexprFunctionDecl(FD) ||
          !CheckConstexprFunctionBody(FD, Body)))
       FD->setInvalidDecl();
 
+    if (FD && FD->hasAttr<NakedAttr>()) {
+      for (const Stmt *S : Body->children()) {
+        if (!isa<AsmStmt>(S)) {
+          Diag(S->getLocStart(), diag::err_non_asm_stmt_in_naked_function);
+          Diag(FD->getAttr<NakedAttr>()->getLocation(), diag::note_attribute);
+          FD->setInvalidDecl();
+          break;
+        }
+      }
+    }
+
     assert(ExprCleanupObjects.empty() && "Leftover temporaries in function");
     assert(!ExprNeedsCleanups && "Unaccounted cleanups in function");
     assert(MaybeODRUseExprs.empty() &&
            "Leftover expressions for odr-use checking");
   }
   
   if (!IsInstantiation)
     PopDeclContext();

lib/Sema/SemaStmtAsm.cpp

ExprResult Sema::LookupInlineAsmIdentifier(CXXScopeSpec &SS,
   if (IsUnevaluatedContext)
     PopExpressionEvaluationContext();
 
   if (!Result.isUsable()) return Result;
 
   Result = CheckPlaceholderExpr(Result.get());
   if (!Result.isUsable()) return Result;
 
+  // Referring to parameters is not allowed in naked functions.
+  if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Result.get())) {
+    if (ParmVarDecl *Parm = dyn_cast<ParmVarDecl>(DRE->getDecl())) {
+      if (FunctionDecl *Func = dyn_cast<FunctionDecl>(Parm->getDeclContext())) {
+        if (Func->hasAttr<NakedAttr>()) {
+          Diag(Id.getLocStart(), diag::err_asm_naked_parm_ref);
+          Diag(Func->getAttr<NakedAttr>()->getLocation(), diag::note_attribute);
+        }
+      }
+    }
+  }
+
   QualType T = Result.get()->getType();
 
   // For now, reject dependent types.
   if (T->isDependentType()) {
     Diag(Id.getLocStart(), diag::err_asm_incomplete_type) << T;
     return ExprError();
   }
 

test/CodeGen/attr-naked.c

 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.7.0 %s -emit-llvm -o - | FileCheck %s
 
 void t1() __attribute__((naked));
 
 // Basic functionality check
 // (Note that naked needs to imply noinline to work properly.)
 // CHECK: define void @t1() [[NAKED:#[0-9]+]] {
 void t1()
 {
 }
 
 // Make sure this doesn't explode in the verifier.
 // (It doesn't really make sense, but it isn't invalid.)
 // CHECK: define void @t2() [[NAKED]] {
-__attribute((naked, always_inline)) void t2()  {
+__attribute((naked, always_inline)) void t2() {
 }
 
+// Make sure not to generate prolog or epilog for naked functions.
+__attribute((naked)) void t3(int x) {
+// CHECK: define void @t3(i32)
+// CHECK-NEXT: entry
+// CHECK-NOT: alloca
+// CHECK-NOT: store
+// CHECK-NEXT: unreachable
+}
+
 // CHECK: attributes [[NAKED]] = { naked noinline nounwind{{.*}} }

test/Sema/attr-naked.c

-// RUN: %clang_cc1 %s -verify -fsyntax-only
+// RUN: %clang_cc1 %s -verify -fsyntax-only -triple i686-pc-linux
 
 int a __attribute__((naked)); // expected-warning {{'naked' attribute only applies to functions}}
 
 __attribute__((naked)) int t0(void) {
   __asm__ volatile("mov r0, #0");
 }
 
 void t1() __attribute__((naked));
 
 void t2() __attribute__((naked(2))); // expected-error {{'naked' attribute takes no arguments}}
 
+__attribute__((naked)) int t3() { // expected-note{{attribute is here}}
+  return 42; // expected-error{{non-ASM statement in naked function is not supported}}
+}
+
+__attribute__((naked)) int t4() {
+  asm("movl $42, %eax");
+  asm("retl");
+}
+
+__attribute__((naked)) int t5(int x) {
+  asm("movl x, %eax");
+  asm("retl");
+}

test/Sema/ms-inline-asm.c

void t4() {
   __asm { mov eax, fs:[0].A.a }
   __asm { mov eax, fs:[0].a } // expected-error {{Unable to lookup field reference!}}
   __asm { mov eax, fs:[0]. A.a } // expected-error {{Unexpected token type!}}
 }
 
 void test_operand_size() {
   __asm { call word t4 } // expected-error {{Expected 'PTR' or 'ptr' token!}}
 }
+
+__declspec(naked) int t5(int x) { // expected-note {{attribute is here}}
+  asm { movl eax, x } // expected-error {{parameter references not allowed in naked functions}}
+  asm { retl }
+}
+
+int y;
+__declspec(naked) int t6(int x) {
+  asm { mov eax, y } // No error.
+  asm { ret }
+}