Disallow using function parameters in extended asm inputs or outputs in naked functions (PR21178)

[hans]

Summary

Clang won't emit any prologues for such functions, so it would assert trying to codegen the parameter references.

This patch makes Clang check the extended asm inputs and outputs for references to function parameters.

It is probably still possible to shoot oneself in the foot since other arbitrary expressions are allowed there :/

[majnemer]

lib/Sema/SemaStmtAsm.cpp

91–94 ↗

(On Diff #14487)

Is it worth doing the recursion if we aren't inside a naked function?

[hans]

Try to avoid doing work for non-naked functions.

[rnk]

lgtm

[hans]

Closed by commit rL219272 (authored by @hans).

cfe/trunk/lib/Sema/SemaStmtAsm.cpp

for (unsigned p = 0, e = AsmStrPieces.size(); p != e; ++p) {
     // If this is a reference to the input and if the input was the smaller
     // one, then we have to reject this asm.
     if (Piece.getOperandNo() == OpNo)
       return true;
   }
   return false;
 }
 
+static bool CheckNakedParmReference(Expr *E, Sema &S) {
+  FunctionDecl *Func = dyn_cast<FunctionDecl>(S.CurContext);
+  if (!Func)
+    return false;
+  if (!Func->hasAttr<NakedAttr>())
+    return false;
+
+  SmallVector<Expr*, 4> WorkList;
+  WorkList.push_back(E);
+  while (WorkList.size()) {
+    Expr *E = WorkList.pop_back_val();
+    if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E)) {
+      if (isa<ParmVarDecl>(DRE->getDecl())) {
+        S.Diag(DRE->getLocStart(), diag::err_asm_naked_parm_ref);
+        S.Diag(Func->getAttr<NakedAttr>()->getLocation(), diag::note_attribute);
+        return true;
+      }
+    }
+    for (Stmt *Child : E->children()) {
+      if (Expr *E = dyn_cast_or_null<Expr>(Child))
+        WorkList.push_back(E);
+    }
+  }
+  return false;
+}
+
 StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple,
                                  bool IsVolatile, unsigned NumOutputs,
                                  unsigned NumInputs, IdentifierInfo **Names,
                                  MultiExprArg constraints, MultiExprArg Exprs,
                                  Expr *asmString, MultiExprArg clobbers,
                                  SourceLocation RParenLoc) {
   unsigned NumClobbers = clobbers.size();
   StringLiteral **Constraints =
for (unsigned i = 0; i != NumOutputs; i++) {
 
     // Check that the output exprs are valid lvalues.
     Expr *OutputExpr = Exprs[i];
     if (CheckAsmLValue(OutputExpr, *this))
       return StmtError(Diag(OutputExpr->getLocStart(),
                             diag::err_asm_invalid_lvalue_in_output)
                        << OutputExpr->getSourceRange());
 
+    // Referring to parameters is not allowed in naked functions.
+    if (CheckNakedParmReference(OutputExpr, *this))
+      return StmtError();
+
     if (RequireCompleteType(OutputExpr->getLocStart(), Exprs[i]->getType(),
                             diag::err_dereference_incomplete_type))
       return StmtError();
 
     OutputConstraintInfos.push_back(Info);
 
     const Type *Ty = OutputExpr->getType().getTypePtr();
 
if (!Context.getTargetInfo().validateInputConstraint(OutputConstraintInfos.data(),
                                                 NumOutputs, Info)) {
       return StmtError(Diag(Literal->getLocStart(),
                             diag::err_asm_invalid_input_constraint)
                        << Info.getConstraintStr());
     }
 
     Expr *InputExpr = Exprs[i];
 
+    // Referring to parameters is not allowed in naked functions.
+    if (CheckNakedParmReference(InputExpr, *this))
+      return StmtError();
+
     // Only allow void types for memory constraints.
     if (Info.allowsMemory() && !Info.allowsRegister()) {
       if (CheckAsmLValue(InputExpr, *this))
         return StmtError(Diag(InputExpr->getLocStart(),
                               diag::err_asm_invalid_lvalue_in_input)
                          << Info.getConstraintStr()
                          << InputExpr->getSourceRange());
     } else {
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);
-          return ExprError();
-        }
-      }
-    }
-  }
+  if (CheckNakedParmReference(Result.get(), *this))
 
   QualType T = Result.get()->getType();
 
   // For now, reject dependent types.
   if (T->isDependentType()) {
     Diag(Id.getLocStart(), diag::err_asm_incomplete_type) << T;
     return ExprError();
   }

cfe/trunk/test/Sema/attr-naked.c

 __attribute__((naked)) void t6() {
   ;
 }
 
 __attribute__((naked)) void t7() {
   asm("movl $42, %eax");
   ;
 }
+
+extern int x, y;
+
+__attribute__((naked)) void t8(int z) { // expected-note{{attribute is here}}
+  __asm__ ("movl $42, %1"
+           : "=r"(x),
+             "=r"(z) // expected-error{{parameter references not allowed in naked functions}}
+           );
+}
+
+__attribute__((naked)) void t9(int z) { // expected-note{{attribute is here}}
+  __asm__ ("movl %eax, %1"
+           : : "r"(x),
+               "r"(z) // expected-error{{parameter references not allowed in naked functions}}
+           );
+}