r360637 - PR41817: Fix regression in r359260 that caused the MS compatibility

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Tue May 14 09:35:07 PDT 2019


On Tue, 14 May 2019, 03:09 Hans Wennborg via cfe-commits, <
cfe-commits at lists.llvm.org> wrote:

> Actually, we didn't notice r359260 in Chromium, however this one
> (r360637) caused Clang to start asserting during our builds
> (https://bugs.chromium.org/p/chromium/issues/detail?id=962840), here's
> a reduced test case:
>
> --
> extern int a;
> static int *use1 = &a;
> int **use2 = &use1;
> static int a = 0;
> --
>
> $ clang.bad -cc1 -triple i386-pc-windows-msvc19.11.0 -emit-obj
> -fms-extensions /tmp/a.cc
> clang.bad: /work/llvm.monorepo/clang/lib/AST/Decl.cpp:1494:
> clang::LinkageInfo clang::LinkageComputer::getLVForDecl(const
> clang::NamedDecl*, clang::LVComputationKind): Assertion
> `D->getCachedLinkage() == LV.getLinkage()' failed.
>
>
> I've reverted this one in r360657 in the meantime.
>

Yep, I'm not at all surprised. Perhaps we should stop claiming to support
this extension, given that it fundamentally violates assumptions made by
clang (that linkage doesn't change after the first declaration). Presumably
we instead used to miscompile the example you give above (and after the
revert, miscompile it again now)?

From: Richard Smith via cfe-commits <cfe-commits at lists.llvm.org>
> Date: Tue, May 14, 2019 at 2:24 AM
> To: <cfe-commits at lists.llvm.org>
>
> > Author: rsmith
> > Date: Mon May 13 17:27:16 2019
> > New Revision: 360637
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=360637&view=rev
> > Log:
> > PR41817: Fix regression in r359260 that caused the MS compatibility
> > extension allowing a "static" declaration to follow an "extern"
> > declaration to stop working.
> >
> > Added:
> >     cfe/trunk/test/CodeGen/ms-compat-extern-static.c
> > Modified:
> >     cfe/trunk/lib/AST/Decl.cpp
> >
> > Modified: cfe/trunk/lib/AST/Decl.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=360637&r1=360636&r2=360637&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/AST/Decl.cpp (original)
> > +++ cfe/trunk/lib/AST/Decl.cpp Mon May 13 17:27:16 2019
> > @@ -613,12 +613,41 @@ static LinkageInfo getExternalLinkageFor
> >  static StorageClass getStorageClass(const Decl *D) {
> >    if (auto *TD = dyn_cast<TemplateDecl>(D))
> >      D = TD->getTemplatedDecl();
> > -  if (D) {
> > -    if (auto *VD = dyn_cast<VarDecl>(D))
> > -      return VD->getStorageClass();
> > -    if (auto *FD = dyn_cast<FunctionDecl>(D))
> > -      return FD->getStorageClass();
> > +  if (!D)
> > +    return SC_None;
> > +
> > +  if (auto *VD = dyn_cast<VarDecl>(D)) {
> > +    // Generally, the storage class is determined by the first
> declaration.
> > +    auto SC = VD->getCanonicalDecl()->getStorageClass();
> > +
> > +    // ... except that MSVC permits an 'extern' declaration to be
> redeclared
> > +    // 'static' as an extension.
> > +    if (SC == SC_Extern) {
> > +      for (auto *Redecl : VD->redecls()) {
> > +        if (Redecl->getStorageClass() == SC_Static)
> > +          return SC_Static;
> > +        if (Redecl->getStorageClass() != SC_Extern &&
> > +            !Redecl->isLocalExternDecl() &&
> !Redecl->getFriendObjectKind())
> > +          break;
> > +      }
> > +    }
> > +    return SC;
> >    }
> > +
> > +  if (auto *FD = dyn_cast<FunctionDecl>(D)) {
> > +    auto SC = FD->getCanonicalDecl()->getStorageClass();
> > +    if (SC == SC_Extern) {
> > +      for (auto *Redecl : FD->redecls()) {
> > +        if (Redecl->getStorageClass() == SC_Static)
> > +          return SC_Static;
> > +        if (Redecl->getStorageClass() != SC_Extern &&
> > +            !Redecl->isLocalExternDecl() &&
> !Redecl->getFriendObjectKind())
> > +          break;
> > +      }
> > +    }
> > +    return SC;
> > +  }
> > +
> >    return SC_None;
> >  }
> >
> > @@ -634,7 +663,7 @@ LinkageComputer::getLVForNamespaceScopeD
> >    //   A name having namespace scope (3.3.6) has internal linkage if it
> >    //   is the name of
> >
> > -  if (getStorageClass(D->getCanonicalDecl()) == SC_Static) {
> > +  if (getStorageClass(D) == SC_Static) {
> >      // - a variable, variable template, function, or function template
> >      //   that is explicitly declared static; or
> >      // (This bullet corresponds to C99 6.2.2p3.)
> >
> > Added: cfe/trunk/test/CodeGen/ms-compat-extern-static.c
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-compat-extern-static.c?rev=360637&view=auto
> >
> ==============================================================================
> > --- cfe/trunk/test/CodeGen/ms-compat-extern-static.c (added)
> > +++ cfe/trunk/test/CodeGen/ms-compat-extern-static.c Mon May 13 17:27:16
> 2019
> > @@ -0,0 +1,11 @@
> > +// RUN: %clang_cc1 -emit-llvm %s -o - -fms-extensions -triple
> x86_64-windows | FileCheck %s
> > +
> > +// CHECK: @n = internal global i32 1
> > +extern int n;
> > +static int n = 1;
> > +int *use = &n;
> > +
> > +// CHECK: define internal void @f(
> > +extern void f();
> > +static void f() {}
> > +void g() { return f(); }
> >
> >
> > _______________________________________________
> > 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/20190514/a558f30e/attachment-0001.html>


More information about the cfe-commits mailing list