r255371 - Error on redeclaring with a conflicting asm label and on redeclaring with an asm label after the first ODR-use. Detects problems like the one in PR22830 where gcc and clang both compiled the file but with different behaviour.
Kostya Serebryany via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 4 16:29:31 PST 2016
On Mon, Jan 4, 2016 at 4:04 PM, Roland McGrath <mcgrathr at google.com> wrote:
> Kostya, do you remember the exact original case in glibc for 22830 and
> what glibc change fixed it?
>
Sadly, no.
I only know that the code was reduced from the code that was setting these
attributes for strtol.
I vaguely recall the case, but not enough to find the actual change
> and compare it to the new scenarios.
> I suspect that the original case was easy to fix because it was just
> that the hidden_proto magic had been put in the wrong place.
>
> The fstat/__fxstat case is harder. The original declaration of
> __fxstat and the extern inline (that is, 'extern __inline
> __attribute__ ((__gnu_inline__))') definition of fstat (which calls
> __fxstat) is in the installed header, while the hidden_proto (i.e.
> redeclaration with asm label) is in the wrapper header (and
> necessarily must come after the #include of the installed header).
>
> A GNU extern inline case is one where the rationale I mentioned in
> https://llvm.org/bugs/show_bug.cgi?id=22830#c1 for Clang not being
> able to match GCC's semantics might not really apply: you never need
> to emit the code for fstat before the redeclaration, because you only
> ever inline it and never emit an actual function.
>
> Perhaps there is a defensible reason that Clang really cannot match
> the GNU semantics even for this case. Regardless, Clang really needs
> to thoroughly document its semantics for every language extension when
> its semantics are not completely identical to the GNU language
> extension's semantics.
>
> If Clang's asm labels extension persists in having semantics
> incompatible with the original semantics of the asm labels extension
> invented by GNU, we might be able to work around it adequately in the
> glibc build just by making sure __USE_EXTERN_INLINES is not defined
> during the build.
>
>
> On Mon, Jan 4, 2016 at 3:39 PM, Kostya Serebryany <kcc at google.com> wrote:
> > Thanks for checking, Nick!
> > +Roland, FYI (recent changes in clang break compilation of all of the
> > glibc),
> > similar to https://llvm.org/bugs/show_bug.cgi?id=22830#c1
> >
> > On Mon, Jan 4, 2016 at 3:21 PM, Nick Lewycky <nicholas at mxc.ca> wrote:
> >>
> >> On 01/04/2016 01:40 PM, Kostya Serebryany wrote:
> >>>
> >>>
> >>>
> >>> On Thu, Dec 17, 2015 at 5:03 PM, Richard Smith <richard at metafoo.co.uk
> >>> <mailto:richard at metafoo.co.uk>> wrote:
> >>>
> >>> On Thu, Dec 17, 2015 at 3:59 PM, Nick Lewycky via cfe-commits
> >>> <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>>
> >>> wrote:
> >>>
> >>> On 12/17/2015 10:47 AM, Kostya Serebryany wrote:
> >>>
> >>> I am now observing this error message when building glibc
> >>> with clang
> >>> (from trunk):
> >>> ../include/string.h:101:28: error: cannot apply asm label
> to
> >>> function
> >>> after its first use
> >>> libc_hidden_builtin_proto (memcpy)
> >>> (many more instances)
> >>>
> >>>
> >>> Do you think this is a bug in glibc code, or the error
> >>> message could be
> >>> more relaxed?
> >>>
> >>>
> >>> Could you email me a .i copy of a failing build? That will help
> >>> me decide whether it's a bug in the error or in glibc.
> >>>
> >>>
> >>> [sorry for delay,]
> >>> here is the preprocessed source file from glibc:
> >>> % clang ~/tmp/a.i 2>&1 | grep error:
> >>> ../include/sys/stat.h:16:28: error: cannot apply asm label to function
> >>> after its first use
> >>> ../include/sys/stat.h:17:30: error: cannot apply asm label to function
> >>> after its first use
> >>> ../include/sys/stat.h:18:28: error: cannot apply asm label to function
> >>> after its first use
> >>> ../include/sys/stat.h:19:30: error: cannot apply asm label to function
> >>> after its first use
> >>> ...
> >>>
> >>>
> >>> Also PR22830 comment 1 seems relevant here.
> >>>
> >>>
> >>> Probably, but since this is a very recent regression in clang I thought
> >>> I should report it.
> >>
> >>
> >> This looks like it's WAI:
> >>
> >> 3783 extern int __fxstat (int __ver, int __fildes, struct stat
> >> *__stat_buf)
> >> 3784 __attribute__ ((__nothrow__ )) ;
> >> ...
> >> 3827 extern __inline int
> >> 3828 __attribute__ ((__nothrow__ )) fstat (int __fd, struct stat
> >> *__statbuf)
> >> 3829 {
> >> 3830 return __fxstat (1, __fd, __statbuf);
> >> 3831 }
> >> ...
> >> 3910 extern __typeof (__fxstat) __fxstat __asm__ ("" "__GI___fxstat")
> >> __attribute__ ((visibility ("hidden")));
> >>
> >> This is exactly the situation where GCC and Clang will emit a different
> .o
> >> file.
> >>
> >> Nick
> >>
> >>> On Fri, Dec 11, 2015 at 1:28 PM, Nick Lewycky via
> cfe-commits
> >>> <cfe-commits at lists.llvm.org
> >>> <mailto:cfe-commits at lists.llvm.org>
> >>> <mailto:cfe-commits at lists.llvm.org
> >>>
> >>> <mailto:cfe-commits at lists.llvm.org>>> wrote:
> >>>
> >>> Author: nicholas
> >>> Date: Fri Dec 11 15:28:55 2015
> >>> New Revision: 255371
> >>>
> >>> URL:
> >>> http://llvm.org/viewvc/llvm-project?rev=255371&view=rev
> >>> Log:
> >>> Error on redeclaring with a conflicting asm label and
> >>> on redeclaring
> >>> with an asm label after the first ODR-use. Detects
> >>> problems like the
> >>> one in PR22830 where gcc and clang both compiled the
> >>> file but with
> >>> different behaviour.
> >>>
> >>> Added:
> >>> cfe/trunk/test/Sema/asm-label.c
> >>> Modified:
> >>>
> >>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> >>> cfe/trunk/lib/Sema/SemaDecl.cpp
> >>>
> >>> Modified:
> >>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> >>> URL:
> >>>
> >>>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=255371&r1=255370&r2=255371&view=diff
> >>>
> >>>
> >>>
> ==============================================================================
> >>> ---
> >>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> >>> (original)
> >>> +++
> >>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri
> Dec
> >>> 11
> >>> 15:28:55 2015
> >>> @@ -4259,6 +4259,9 @@ def
> err_tag_definition_of_typedef
> >>> : Erro
> >>> def err_conflicting_types : Error<"conflicting types
> >>> for %0">;
> >>> def err_different_pass_object_size_params : Error<
> >>> "conflicting pass_object_size attributes on
> >>> parameters">;
> >>> +def err_late_asm_label_name : Error<
> >>> + "cannot apply asm label to
> >>> %select{variable|function}0 after its
> >>> first use">;
> >>> +def err_different_asm_label : Error<"conflicting asm
> >>> label">;
> >>> def err_nested_redefinition : Error<"nested
> >>> redefinition of %0">;
> >>> def err_use_with_wrong_tag : Error<
> >>> "use of %0 with tag type that does not match
> >>> previous declaration">;
> >>>
> >>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> >>> URL:
> >>>
> >>>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=255371&r1=255370&r2=255371&view=diff
> >>>
> >>>
> >>>
> ==============================================================================
> >>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> >>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Dec 11
> 15:28:55
> >>> 2015
> >>> @@ -2379,9 +2379,24 @@ void
> >>> Sema::mergeDeclAttributes(NamedDecl
> >>> if (!Old->hasAttrs() && !New->hasAttrs())
> >>> return;
> >>>
> >>> - // attributes declared post-definition are
> currently
> >>> ignored
> >>> + // Attributes declared post-definition are
> currently
> >>> ignored.
> >>> checkNewAttributesAfterDef(*this, New, Old);
> >>>
> >>> + if (AsmLabelAttr *NewA =
> >>> New->getAttr<AsmLabelAttr>()) {
> >>> + if (AsmLabelAttr *OldA =
> >>> Old->getAttr<AsmLabelAttr>()) {
> >>> + if (OldA->getLabel() != NewA->getLabel()) {
> >>> + // This redeclaration changes __asm__ label.
> >>> + Diag(New->getLocation(),
> >>> diag::err_different_asm_label);
> >>> + Diag(OldA->getLocation(),
> >>> diag::note_previous_declaration);
> >>> + }
> >>> + } else if (Old->isUsed()) {
> >>> + // This redeclaration adds an __asm__ label to
> a
> >>> declaration
> >>> that has
> >>> + // already been ODR-used.
> >>> + Diag(New->getLocation(),
> >>> diag::err_late_asm_label_name)
> >>> + << isa<FunctionDecl>(Old) <<
> >>> New->getAttr<AsmLabelAttr>()->getRange();
> >>> + }
> >>> + }
> >>> +
> >>> if (!Old->hasAttrs())
> >>> return;
> >>>
> >>>
> >>> Added: cfe/trunk/test/Sema/asm-label.c
> >>> URL:
> >>>
> >>>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/asm-label.c?rev=255371&view=auto
> >>>
> >>>
> >>>
> ==============================================================================
> >>> --- cfe/trunk/test/Sema/asm-label.c (added)
> >>> +++ cfe/trunk/test/Sema/asm-label.c Fri Dec 11
> 15:28:55
> >>> 2015
> >>> @@ -0,0 +1,30 @@
> >>> +// RUN: %clang_cc1 -verify %s
> >>> +
> >>> +void f();
> >>> +void f() __asm__("fish");
> >>> +void g();
> >>> +
> >>> +void f() {
> >>> + g();
> >>> +}
> >>> +void g() __asm__("gold"); // expected-error{{cannot
> >>> apply asm
> >>> label to function after its first use}}
> >>> +
> >>> +void h() __asm__("hose"); // expected-note{{previous
> >>> declaration
> >>> is here}}
> >>> +void h() __asm__("hair"); //
> >>> expected-error{{conflicting asm label}}
> >>> +
> >>> +int x;
> >>> +int x __asm__("xenon");
> >>> +int y;
> >>> +
> >>> +int test() { return y; }
> >>> +
> >>> +int y __asm__("yacht"); // expected-error{{cannot
> >>> apply asm label
> >>> to variable after its first use}}
> >>> +
> >>> +int z __asm__("zebra"); // expected-note{{previous
> >>> declaration is
> >>> here}}
> >>> +int z __asm__("zooms"); //
> >>> expected-error{{conflicting asm label}}
> >>> +
> >>> +
> >>> +// No diagnostics on the following.
> >>> +void __real_readlink() __asm("readlink");
> >>> +void readlink() __asm("__protected_readlink");
> >>> +void readlink() { __real_readlink(); }
> >>>
> >>>
> >>> _______________________________________________
> >>> cfe-commits mailing list
> >>> cfe-commits at lists.llvm.org
> >>> <mailto:cfe-commits at lists.llvm.org>
> >>> <mailto:cfe-commits at lists.llvm.org
> >>> <mailto: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 <mailto: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/20160104/138b4cb4/attachment-0001.html>
More information about the cfe-commits
mailing list