On Fri, Apr 7, 2017 at 11:13 AM, Saleem Abdulrasool via cfe-commits <cfe-commits@lists.llvm.org> wrote: > Author: compnerd > Date: Fri Apr 7 10:13:47 2017 > New Revision: 299774 > > URL: http://llvm.org/viewvc/llvm-project?rev=299774&view=rev > Log: > Sema: prevent __declspec(naked) use on x64 > > MSDN (https://msdn.microsoft.com/en-us/library/h5w10wxs.aspx) indicates > that `__declspec(naked)` is only permitted on x86 and ARM targets. > Testing with cl does confirm this behaviour. Provide a warning for use > of `__declspec(naked)` on x64.
Your patch is not providing a warning, it's providing an error, which seems inappropriate (to me). Why is this attribute not silently ignored there instead? (Btw, I did not see a review thread for this, did I miss one?) More comments below. > > Added: > cfe/trunk/test/Sema/declspec-naked.c > Modified: > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > cfe/trunk/lib/Sema/SemaDeclAttr.cpp > cfe/trunk/test/Sema/ms-inline-asm.c > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=299774&r1=299773&r2=299774&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Apr 7 10:13:47 > 2017 > @@ -3231,7 +3231,8 @@ def err_attribute_regparm_invalid_number > "'regparm' parameter must be between 0 and %0 inclusive">; > def err_attribute_not_supported_in_lang : Error< > "%0 attribute is not supported in %select{C|C++|Objective-C}1">; > - > +def err_attribute_not_supported_on_arch > + : Error<"%0 attribute is not supported on '%1'">; > > // Clang-Specific Attributes > def warn_attribute_iboutlet : Warning< > > Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=299774&r1=299773&r2=299774&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) > +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Apr 7 10:13:47 2017 > @@ -1923,6 +1923,17 @@ static void handleNakedAttr(Sema &S, Dec > Attr.getName())) > return; > > + if (Attr.isDeclspecAttribute()) { > + const auto &Triple = S.getASTContext().getTargetInfo().getTriple(); No need to have the Triple variable (which is a rather poor name given that it's also the type name). > + const auto &Arch = Triple.getArch(); Do not use auto for either of these. ~Aaron > + if (Arch != llvm::Triple::x86 && > + (Arch != llvm::Triple::arm && Arch != llvm::Triple::thumb)) { > + S.Diag(Attr.getLoc(), diag::err_attribute_not_supported_on_arch) > + << Attr.getName() << Triple.getArchName(); > + return; > + } > + } > + > D->addAttr(::new (S.Context) NakedAttr(Attr.getRange(), S.Context, > > Attr.getAttributeSpellingListIndex())); > } > > Added: cfe/trunk/test/Sema/declspec-naked.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/declspec-naked.c?rev=299774&view=auto > ============================================================================== > --- cfe/trunk/test/Sema/declspec-naked.c (added) > +++ cfe/trunk/test/Sema/declspec-naked.c Fri Apr 7 10:13:47 2017 > @@ -0,0 +1,11 @@ > +// RUN: %clang_cc1 -triple i686-unknown-windows-msvc -fsyntax-only > -fdeclspec -verify %s > +// RUN: %clang_cc1 -triple thumbv7-unknown-windows-msvc -fsyntax-only > -fdeclspec -verify %s > +// RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -fsyntax-only > -fdeclspec -verify %s > +#if defined(_M_IX86) || defined(_M_ARM) > +// CHECK: expected-no-diagnostics > +#endif > + > +void __declspec(naked) f(void) {} > +#if !defined(_M_IX86) && !defined(_M_ARM) > +// expected-error@-2{{'naked' attribute is not supported on 'x86_64'}} > +#endif > > Modified: cfe/trunk/test/Sema/ms-inline-asm.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/ms-inline-asm.c?rev=299774&r1=299773&r2=299774&view=diff > ============================================================================== > --- cfe/trunk/test/Sema/ms-inline-asm.c (original) > +++ cfe/trunk/test/Sema/ms-inline-asm.c Fri Apr 7 10:13:47 2017 > @@ -1,5 +1,5 @@ > // REQUIRES: x86-registered-target > -// RUN: %clang_cc1 %s -triple x86_64-apple-darwin10 -fms-extensions > -fasm-blocks -Wno-microsoft -Wunused-label -verify -fsyntax-only > +// RUN: %clang_cc1 %s -triple i386-apple-darwin10 -fms-extensions > -fasm-blocks -Wno-microsoft -Wunused-label -verify -fsyntax-only > > void t1(void) { > __asm __asm // expected-error {{__asm used with no assembly instructions}} > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
