r359814 - [Sema] Emit warning for visibility attribute on internal-linkage declaration

Nico Weber via cfe-commits cfe-commits at lists.llvm.org
Thu May 2 20:14:04 PDT 2019


This also breaks building libc++ [1]. Reverted for now in r359858.

1:
FAILED: obj/buildtools/third_party/libc++/libc++/new.o
../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF
obj/buildtools/third_party/libc++/libc++/new.o.d -D_LIBCPP_BUILDING_LIBRARY
-DLIBCXX_BUILDING_LIBCXXABI -DUSE_UDEV -DUSE_AURA=1 -DUSE_GLIB=1
-DUSE_NSS_CERTS=1 -DUSE_X11=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD
-DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_GNU_SOURCE
-DCR_CLANG_REVISION=\"359857-0\" -DCOMPONENT_BUILD -D_LIBCPP_ABI_UNSTABLE
-D_LIBCPP_ABI_VERSION=Cr -D_LIBCPP_ENABLE_NODISCARD
-DCR_LIBCXX_REVISION=358423
-DCR_SYSROOT_HASH=e7c53f04bd88d29d075bfd1f62b073aeb69cbe09 -DNDEBUG
-DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -I../.. -Igen
-fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector
-funwind-tables -fPIC -B../../third_party/binutils/Linux_x64/Release/bin
-pthread -fcolor-diagnostics -fmerge-all-constants
-fcrash-diagnostics-dir=../../tools/clang/crashreports -Xclang -mllvm
-Xclang -instcombine-lower-dbg-declare=0 -fcomplete-member-pointers -m64
-march=x86-64 -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__=
-D__TIMESTAMP__= -Xclang -fdebug-compilation-dir -Xclang .
-no-canonical-prefixes -O2 -fno-ident -fdata-sections -ffunction-sections
-fno-omit-frame-pointer -g2 -ggnu-pubnames -Wheader-hygiene
-Wstring-conversion -Wtautological-overlap-compare -fstrict-aliasing -fPIC
-Werror -Wall -Wno-unused-variable -Wno-missing-field-initializers
-Wno-unused-parameter -Wno-c++11-narrowing
-Wno-unneeded-internal-declaration -Wno-undefined-var-template
-Wno-ignored-pragma-optimize -fvisibility=default -std=c++14 -nostdinc++
-isystem../../buildtools/third_party/libc++/trunk/include
-isystem../../buildtools/third_party/libc++abi/trunk/include
--sysroot=../../build/linux/debian_sid_amd64-sysroot -fexceptions -frtti -c
../../buildtools/third_party/libc++/trunk/src/new.cpp -o
obj/buildtools/third_party/libc++/libc++/new.o
In file included from
../../buildtools/third_party/libc++/trunk/src/new.cpp:12:
../../buildtools/third_party/libc++/trunk/src/include/atomic_support.h:55:8:
error: '__visibility__' attribute is ignored on a non-external symbol
[-Werror,-Wignored-attributes]
inline _LIBCPP_INLINE_VISIBILITY
       ^
../../buildtools/third_party/libc++/trunk/include/__config:820:35: note:
expanded from macro '_LIBCPP_INLINE_VISIBILITY'
#define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
                                  ^
../../buildtools/third_party/libc++/trunk/include/__config:805:35: note:
expanded from macro '_LIBCPP_HIDE_FROM_ABI'
#    define _LIBCPP_HIDE_FROM_ABI _LIBCPP_HIDDEN
_LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION
                                  ^
../../buildtools/third_party/libc++/trunk/include/__config:695:44: note:
expanded from macro '_LIBCPP_HIDDEN'
#    define _LIBCPP_HIDDEN __attribute__ ((__visibility__("hidden")))
                                           ^

On Thu, May 2, 2019 at 10:35 PM via cfe-commits <cfe-commits at lists.llvm.org>
wrote:

> Hi Richard,
>
> Thank you for the heads up. Feel free to revert, or I can when I can get
> back to a terminal. I'm not sure where the right place is to warn if using
> the linkage calculator here is too early, so it might take a bit of time
> for me to fix the patch.
>
> Thanks,
> Scott
>
> On May 2, 2019 9:42 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> This will trigger computation and caching the linkage of the
> declaration too early (before we actually know it) in some cases. For
> instance, I believe this is causing a rejects-valid on:
>
> typedef struct __attribute__((visibility("hidden"))) {} A;
>
> ... which we used to accept but now reject with:
>
> <stdin>:1:31: warning: 'visibility' attribute is ignored on a
> non-external symbol [-Wignored-attributes]
> typedef struct __attribute__((visibility("hidden"))) {} A;
>                               ^
> <stdin>:1:57: error: unsupported: typedef changes linkage of anonymous
> type, but linkage was already computed
> typedef struct __attribute__((visibility("hidden"))) {} A;
>                                                         ^
> <stdin>:1:15: note: use a tag name here to establish linkage prior to
> definition
> typedef struct __attribute__((visibility("hidden"))) {} A;
>               ^
>                A
>
> In addition to the error, note that the warning is wrong, because the
> linkage was computed too early (before it was known).
>
> On Thu, 2 May 2019 at 12:01, Scott Linder via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
> >
> > Author: scott.linder
> > Date: Thu May  2 12:03:57 2019
> > New Revision: 359814
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=359814&view=rev
> > Log:
> > [Sema] Emit warning for visibility attribute on internal-linkage
> declaration
> >
> > GCC warns on these cases, but we currently just silently ignore the
> attribute.
> >
> > Differential Revision: https://reviews.llvm.org/D61097
> >
> > Modified:
> >     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> >     cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> >     cfe/trunk/test/Sema/attr-visibility.c
> >     cfe/trunk/test/SemaCXX/ast-print.cpp
> >     cfe/trunk/test/SemaCXX/attr-visibility.cpp
> >
> > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=359814&r1=359813&r2=359814&view=diff
> >
> ==============================================================================
>
> > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu May  2
> 12:03:57 2019
> > @@ -2778,6 +2778,9 @@ def warn_attribute_ignored : Warning<"%0
> >  def warn_attribute_ignored_on_inline :
> >    Warning<"%0 attribute ignored on inline function">,
> >    InGroup<IgnoredAttributes>;
> > +def warn_attribute_ignored_on_non_external :
> > +  Warning<"%0 attribute is ignored on a non-external symbol">,
> > +  InGroup<IgnoredAttributes>;
> >  def warn_nocf_check_attribute_ignored :
> >    Warning<"'nocf_check' attribute ignored; use -fcf-protection to
> enable the attribute">,
> >    InGroup<IgnoredAttributes>;
> >
> > Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=359814&r1=359813&r2=359814&view=diff
> >
> ==============================================================================
>
> > --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> > +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Thu May  2 12:03:57 2019
> > @@ -2615,6 +2615,14 @@ static void handleVisibilityAttr(Sema &S
> >      return;
> >    }
> >
> > +  // Visibility attributes have no effect on symbols with internal
> linkage.
> > +  if (const auto *ND = dyn_cast<NamedDecl>(D)) {
> > +    if (!ND->isExternallyVisible())
> > +      S.Diag(AL.getRange().getBegin(),
> > +             diag::warn_attribute_ignored_on_non_external)
> > +          << AL;
> > +  }
> > +
> >    // Check that the argument is a string literal.
> >    StringRef TypeStr;
> >    SourceLocation LiteralLoc;
> >
> > Modified: cfe/trunk/test/Sema/attr-visibility.c
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-visibility.c?rev=359814&r1=359813&r2=359814&view=diff
> >
> ==============================================================================
>
> > --- cfe/trunk/test/Sema/attr-visibility.c (original)
> > +++ cfe/trunk/test/Sema/attr-visibility.c Thu May  2 12:03:57 2019
> > @@ -26,3 +26,9 @@ typedef int __attribute__((visibility("d
> >  int x __attribute__((type_visibility("default"))); // expected-error
> {{'type_visibility' attribute only applies to types and namespaces}}
> >
> >  int PR17105 __attribute__((visibility(hidden))); // expected-error
> {{'visibility' attribute requires a string}}
> > +
> > +static int test8 __attribute__((visibility("default"))); //
> expected-warning {{'visibility' attribute is ignored on a non-external
> symbol}}
> > +static int test9 __attribute__((visibility("hidden"))); //
> expected-warning {{'visibility' attribute is ignored on a non-external
> symbol}}
> > +static int test10 __attribute__((visibility("internal"))); //
> expected-warning {{'visibility' attribute is ignored on a non-external
> symbol}}
> > +
> > +static int test11() __attribute__((visibility("default"))); //
> expected-warning {{'visibility' attribute is ignored on a non-external
> symbol}}
> >
> > Modified: cfe/trunk/test/SemaCXX/ast-print.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ast-print.cpp?rev=359814&r1=359813&r2=359814&view=diff
> >
> ==============================================================================
>
> > --- cfe/trunk/test/SemaCXX/ast-print.cpp (original)
> > +++ cfe/trunk/test/SemaCXX/ast-print.cpp Thu May  2 12:03:57 2019
> > @@ -209,10 +209,8 @@ void test(int i) {
> >  }
> >  }
> >
> > -namespace {
> >  // CHECK: struct {{\[\[gnu::visibility\(\"hidden\"\)\]\]}} S;
> >  struct [[gnu::visibility("hidden")]] S;
> > -}
> >
> >  // CHECK:      struct CXXFunctionalCastExprPrint {
> >  // CHECK-NEXT: } fce = CXXFunctionalCastExprPrint{};
> >
> > Modified: cfe/trunk/test/SemaCXX/attr-visibility.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-visibility.cpp?rev=359814&r1=359813&r2=359814&view=diff
> >
> ==============================================================================
>
> > --- cfe/trunk/test/SemaCXX/attr-visibility.cpp (original)
> > +++ cfe/trunk/test/SemaCXX/attr-visibility.cpp Thu May  2 12:03:57 2019
> > @@ -18,3 +18,9 @@ void foo<int>() {
> >  struct x3 {
> >    static int y;
> >  } __attribute((visibility("default"))); // expected-warning {{attribute
> 'visibility' after definition is ignored}}
> > +
> > +const int test4 __attribute__((visibility("default"))) = 0; //
> expected-warning {{'visibility' attribute is ignored on a non-external
> symbol}}
> > +
> > +namespace {
> > +  int test5 __attribute__((visibility("default"))); // expected-warning
> {{'visibility' attribute is ignored on a non-external symbol}}
> > +};
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://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/20190502/453e4c50/attachment-0001.html>


More information about the cfe-commits mailing list