This is an archive of the discontinued LLVM Phabricator instance.

Differential D7329

Diagnose CXX "this" reference in functions with naked attr
ClosedPublic

Authored by weimingz on Feb 1 2015, 10:34 PM.

Details

Reviewers
hans
rnk
Commits
rG71ac240620c4: Diagnose CXX 'this' pointer reference in funcs with naked attr
rC228052: Diagnose CXX 'this' pointer reference in funcs with naked attr
rL228052: Diagnose CXX 'this' pointer reference in funcs with naked attr
Summary

Currently, clang will assert for non-static class member func with naked attribute because clang won't emit any prologues for such functions, so it would assert when it tries to codegen 'this' pointer

This patch fixes bug http://llvm.org/bugs/show_bug.cgi?id=22429

Diff Detail

Event Timeline

weimingz updated this revision to Diff 19128.Feb 1 2015, 10:34 PM
weimingz retitled this revision from to Diagnose class member function with naked attr.
weimingz updated this object.
weimingz edited the test plan for this revision. (Show Details)
weimingz added a subscriber: Unknown Object (MLST).
majnemer added a reviewer: hans.Feb 2 2015, 11:35 AM
weimingz updated this object.Feb 2 2015, 12:01 PM
rnk added a subscriber: rnk.Feb 2 2015, 2:04 PM

I think it would be preferable to extend the asm constraint checks to disallow CXXThisExpr.

hans edited edge metadata.Feb 2 2015, 2:18 PM

I think it would be preferable to extend the asm constraint checks to disallow CXXThisExpr.

Yes, I don't see why we would need to disallow naked member functions.

weimingz added a comment.Feb 2 2015, 3:22 PM
In D7329#117184, @hans wrote:

I think it would be preferable to extend the asm constraint checks to disallow CXXThisExpr.

Yes, I don't see why we would need to disallow naked member functions.

Agree. Will do

weimingz updated this revision to Diff 19199.Feb 2 2015, 3:59 PM
weimingz retitled this revision from Diagnose class member function with naked attr to Diagnose CXX "this" reference in functions with naked attr.
weimingz edited edge metadata.
weimingz updated this revision to Diff 19200.Feb 2 2015, 4:02 PM

(undo unnecessary change made in previous patch)

rnk accepted this revision.Feb 2 2015, 6:20 PM
rnk added a reviewer: rnk.

lgtm

include/clang/Basic/DiagnosticSemaKinds.td
7178

We don't do smart backtick quotes like this.

This revision is now accepted and ready to land.Feb 2 2015, 6:20 PM
weimingz updated this revision to Diff 19243.Feb 3 2015, 10:22 AM
weimingz edited edge metadata.

Hi Hans,

Are you OK with the patch?

Thanks,
Weiming

hans accepted this revision.Feb 3 2015, 2:00 PM
hans edited edge metadata.

Hi Hans,

Are you OK with the patch?

Yes, this looks good to me.

Closed by commit rL228052: Diagnose CXX 'this' pointer reference in funcs with naked attr (authored by weimingz). · Explain WhyFeb 3 2015, 2:37 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
include/
clang/
Basic/
2 lines
lib/
Sema/
7 lines
test/
Sema/
17 lines

Diff 19199

include/clang/Basic/DiagnosticSemaKinds.td

Context not available.
def err_non_asm_stmt_in_naked_function : Error< def err_non_asm_stmt_in_naked_function : Error<
"non-ASM statement in naked function is not supported">; "non-ASM statement in naked function is not supported">;
def err_asm_naked_this_ref : Error<
"`this' pointer references not allowed in naked functions">;
rnkUnsubmitted
Not Done
ReplyInline Actions

We don't do smart backtick quotes like this.

rnk: We don't do smart backtick quotes like this.
def err_asm_naked_parm_ref : Error< def err_asm_naked_parm_ref : Error<
"parameter references not allowed in naked functions">; "parameter references not allowed in naked functions">;
Context not available.

lib/Sema/SemaStmtAsm.cpp

Context not available.
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
#include "clang/Sema/SemaInternal.h" #include "clang/Sema/SemaInternal.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/RecordLayout.h" #include "clang/AST/RecordLayout.h"
#include "clang/AST/TypeLoc.h" #include "clang/AST/TypeLoc.h"
#include "clang/Basic/TargetInfo.h" #include "clang/Basic/TargetInfo.h"
Context not available.
return false; return false;
if (!Func->hasAttr<NakedAttr>()) if (!Func->hasAttr<NakedAttr>())
return false; return false;
SmallVector<Expr*, 4> WorkList; SmallVector<Expr*, 4> WorkList;
WorkList.push_back(E); WorkList.push_back(E);
while (WorkList.size()) { while (WorkList.size()) {
Expr *E = WorkList.pop_back_val(); Expr *E = WorkList.pop_back_val();
if (isa<CXXThisExpr>(E)) {
S.Diag(E->getLocStart(), diag::err_asm_naked_this_ref);
S.Diag(Func->getAttr<NakedAttr>()->getLocation(), diag::note_attribute);
return true;
}
if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E)) { if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E)) {
if (isa<ParmVarDecl>(DRE->getDecl())) { if (isa<ParmVarDecl>(DRE->getDecl())) {
S.Diag(DRE->getLocStart(), diag::err_asm_naked_parm_ref); S.Diag(DRE->getLocStart(), diag::err_asm_naked_parm_ref);
Context not available.

test/Sema/attr-naked.cpp

  • This file was added.
// RUN: %clang_cc1 %s -verify -fsyntax-only -triple arm-none-linux
class Foo {
void bar();
static void bar2();
unsigned v;
static unsigned s;
};
void __attribute__((naked)) Foo::bar() { // expected-note{{attribute is here}}
asm("mov r2, %0" : : "r"(v)); // expected-error{{`this' pointer references not allowed in naked functions}}
}
void __attribute__((naked)) Foo::bar2() {
asm("mov r2, %0" : : "r"(s)); // static member reference is OK
}