r299774 - Sema: prevent __declspec(naked) use on x64

David Majnemer via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 7 11:48:42 PDT 2017


On Fri, Apr 7, 2017 at 8:30 AM, Aaron Ballman via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> On Fri, Apr 7, 2017 at 11:13 AM, Saleem Abdulrasool via cfe-commits
> <cfe-commits at 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?
>

FWIW, MSVC diagnoses this with an error: https://godbolt.org/g/T5sQr5


>
> (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 at -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 at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170407/6a383bc1/attachment.html>


More information about the cfe-commits mailing list