MS ABI: Implement the MSVC 2015 scheme for scope disambiguation

consider C++ that looks like:
  inline int &f(bool b) {
    if (b) {
      static int i;
      return i;
    }
    static int i;
    return i;
  }

Both 'i' variables must have distinct (and stable) names for linkage
purposes.  The MSVC 2013 ABI would number the variables using a count of
the number of scopes that have been created.  However, the final 'i'
returns to a scope that has already been created leading to a mangling
collision.

MSVC 2015 fixes this by giving the second 'i' the name it would have if
it were declared before the 'if'.  However, this results in ABI breakage
because the mangled name, in cases where there was no ambiguity, would
now be different.

We implement the new behavior and only enable it if we are targeting the
MSVC 2015 ABI, otherwise the old behavior will be used.

This fixes PR18131.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@232766 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h
index bb23b74a4b26b9e8bc73c5acd2a55c60b82b8c42..02f59c020bdec63885af1c8d93067bd5bb8ba3f4 100644 (file)
--- a/include/clang/Parse/Parser.h
+++ b/include/clang/Parse/Parser.h
@@ -245,8 +245,8 @@ public:
 
   const Token &getCurToken() const { return Tok; }
   Scope *getCurScope() const { return Actions.getCurScope(); }
-  void incrementMSLocalManglingNumber() const {
-    return Actions.incrementMSLocalManglingNumber();
+  void incrementMSManglingNumber() const {
+    return Actions.incrementMSManglingNumber();
   }
 
   Decl  *getObjCDeclContext() const { return Actions.getObjCDeclContext(); }
@@ -759,7 +759,7 @@ public:
         Self->EnterScope(ScopeFlags);
       else {
         if (BeforeCompoundStmt)
-          Self->incrementMSLocalManglingNumber();
+          Self->incrementMSManglingNumber();
 
         this->Self = nullptr;
       }

diff --git a/include/clang/Sema/Scope.h b/include/clang/Sema/Scope.h
index 238469a2d5d373bb1e10675d4dbfbae5a153f65b..dfc6f9c4628f02461725dcac5e97cf4b6744f6d4 100644 (file)
--- a/include/clang/Sema/Scope.h
+++ b/include/clang/Sema/Scope.h
@@ -139,7 +139,9 @@ private:
 
   /// \brief Declarations with static linkage are mangled with the number of
   /// scopes seen as a component.
-  unsigned short MSLocalManglingNumber;
+  unsigned short MSLastManglingNumber;
+
+  unsigned short MSCurManglingNumber;
 
   /// PrototypeDepth - This is the number of function prototype scopes
   /// enclosing this scope, including this scope.
@@ -152,7 +154,7 @@ private:
   /// FnParent - If this scope has a parent scope that is a function body, this
   /// pointer is non-null and points to it.  This is used for label processing.
   Scope *FnParent;
-  Scope *MSLocalManglingParent;
+  Scope *MSLastManglingParent;
 
   /// BreakParent/ContinueParent - This is a direct link to the innermost
   /// BreakScope/ContinueScope which contains the contents of this scope
@@ -218,10 +220,10 @@ public:
   const Scope *getFnParent() const { return FnParent; }
   Scope *getFnParent() { return FnParent; }
 
-  const Scope *getMSLocalManglingParent() const {
-    return MSLocalManglingParent;
+  const Scope *getMSLastManglingParent() const {
+    return MSLastManglingParent;
   }
-  Scope *getMSLocalManglingParent() { return MSLocalManglingParent; }
+  Scope *getMSLastManglingParent() { return MSLastManglingParent; }
 
   /// getContinueParent - Return the closest scope that a continue statement
   /// would be affected by.
@@ -275,22 +277,30 @@ public:
     DeclsInScope.erase(D);
   }
 
-  void incrementMSLocalManglingNumber() {
-    if (Scope *MSLMP = getMSLocalManglingParent())
-      MSLMP->MSLocalManglingNumber += 1;
+  void incrementMSManglingNumber() {
+    if (Scope *MSLMP = getMSLastManglingParent()) {
+      MSLMP->MSLastManglingNumber += 1;
+      MSCurManglingNumber += 1;
+    }
   }
 
-  void decrementMSLocalManglingNumber() {
-    if (Scope *MSLMP = getMSLocalManglingParent())
-      MSLMP->MSLocalManglingNumber -= 1;
+  void decrementMSManglingNumber() {
+    if (Scope *MSLMP = getMSLastManglingParent()) {
+      MSLMP->MSLastManglingNumber -= 1;
+      MSCurManglingNumber -= 1;
+    }
   }
 
-  unsigned getMSLocalManglingNumber() const {
-    if (const Scope *MSLMP = getMSLocalManglingParent())
-      return MSLMP->MSLocalManglingNumber;
+  unsigned getMSLastManglingNumber() const {
+    if (const Scope *MSLMP = getMSLastManglingParent())
+      return MSLMP->MSLastManglingNumber;
     return 1;
   }
 
+  unsigned getMSCurManglingNumber() const {
+    return MSCurManglingNumber;
+  }
+
   /// isDeclScope - Return true if this is the scope that the specified decl is
   /// declared in.
   bool isDeclScope(Decl *D) {

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 6f6faf49294af5be59ef2ac1360eb8b98677a398..92d3b735e51198d6a55ff7512518f28fe8eafb2d 100644 (file)
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -8610,8 +8610,8 @@ public:
   /// template substitution or instantiation.
   Scope *getCurScope() const { return CurScope; }
 
-  void incrementMSLocalManglingNumber() const {
-    return CurScope->incrementMSLocalManglingNumber();
+  void incrementMSManglingNumber() const {
+    return CurScope->incrementMSManglingNumber();
   }
 
   IdentifierInfo *getSuperIdentifier() const;

diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
index 27c757b5081bff76201f2ecef2d2e221fb4c81ff..d15f2af65eb28a4fd633d537085703fcc8f3f456 100644 (file)
--- a/lib/Parse/ParseStmt.cpp
+++ b/lib/Parse/ParseStmt.cpp
@@ -1268,7 +1268,7 @@ StmtResult Parser::ParseSwitchStatement(SourceLocation *TrailingElseLoc) {
   // We have incremented the mangling number for the SwitchScope and the
   // InnerScope, which is one too many.
   if (C99orCXX)
-    getCurScope()->decrementMSLocalManglingNumber();
+    getCurScope()->decrementMSManglingNumber();
 
   // Read the body statement.
   StmtResult Body(ParseStatement(TrailingElseLoc));
@@ -1710,7 +1710,7 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
   // It will only be incremented if the body contains other things that would
   // normally increment the mangling number (like a compound statement).
   if (C99orCXXorObjC)
-    getCurScope()->decrementMSLocalManglingNumber();
+    getCurScope()->decrementMSManglingNumber();
 
   // Read the body statement.
   StmtResult Body(ParseStatement(TrailingElseLoc));

diff --git a/lib/Sema/Scope.cpp b/lib/Sema/Scope.cpp
index eeb2e01781e53c3082e94bbc35ff752249e4d212..7c70048acfbf6ad2a4247ec9949d581366d0ba65 100644 (file)
--- a/lib/Sema/Scope.cpp
+++ b/lib/Sema/Scope.cpp
@@ -38,7 +38,8 @@ void Scope::Init(Scope *parent, unsigned flags) {
     FnParent       = parent->FnParent;
     BlockParent    = parent->BlockParent;
     TemplateParamParent = parent->TemplateParamParent;
-    MSLocalManglingParent = parent->MSLocalManglingParent;
+    MSLastManglingParent = parent->MSLastManglingParent;
+    MSCurManglingNumber = getMSLastManglingNumber();
     if ((Flags & (FnScope | ClassScope | BlockScope | TemplateParamScope |
                   FunctionPrototypeScope | AtCatchScope | ObjCMethodScope)) ==
         0)
@@ -47,9 +48,10 @@ void Scope::Init(Scope *parent, unsigned flags) {
     Depth = 0;
     PrototypeDepth = 0;
     PrototypeIndex = 0;
-    MSLocalManglingParent = FnParent = BlockParent = nullptr;
+    MSLastManglingParent = FnParent = BlockParent = nullptr;
     TemplateParamParent = nullptr;
-    MSLocalManglingNumber = 1;
+    MSLastManglingNumber = 1;
+    MSCurManglingNumber = 1;
   }
 
   // If this scope is a function or contains breaks/continues, remember it.
@@ -57,8 +59,9 @@ void Scope::Init(Scope *parent, unsigned flags) {
   // The MS mangler uses the number of scopes that can hold declarations as
   // part of an external name.
   if (Flags & (ClassScope | FnScope)) {
-    MSLocalManglingNumber = getMSLocalManglingNumber();
-    MSLocalManglingParent = this;
+    MSLastManglingNumber = getMSLastManglingNumber();
+    MSLastManglingParent = this;
+    MSCurManglingNumber = 1;
   }
   if (flags & BreakScope)         BreakParent = this;
   if (flags & ContinueScope)      ContinueParent = this;
@@ -78,7 +81,7 @@ void Scope::Init(Scope *parent, unsigned flags) {
     else if ((flags & EnumScope))
       ; // Don't increment for enum scopes.
     else
-      incrementMSLocalManglingNumber();
+      incrementMSManglingNumber();
   }
 
   DeclsInScope.clear();
@@ -209,12 +212,13 @@ void Scope::dumpImpl(raw_ostream &OS) const {
     OS << "Parent: (clang::Scope*)" << Parent << '\n';
 
   OS << "Depth: " << Depth << '\n';
-  OS << "MSLocalManglingNumber: " << getMSLocalManglingNumber() << '\n';
+  OS << "MSLastManglingNumber: " << getMSLastManglingNumber() << '\n';
+  OS << "MSCurManglingNumber: " << getMSCurManglingNumber() << '\n';
   if (const DeclContext *DC = getEntity())
     OS << "Entity : (clang::DeclContext*)" << DC << '\n';
 
   if (NRVO.getInt())
-    OS << "NRVO not allowed";
+    OS << "NRVO not allowed\n";
   else if (NRVO.getPointer())
     OS << "NRVO candidate : (clang::VarDecl*)" << NRVO.getPointer() << '\n';
 }

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 2fd231b720b9436fef922adc99c1e0d37f1b2528..1234afb8fb7ae720239af3c54dd26eef396913a3 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -3416,6 +3416,17 @@ Decl *Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS,
   return ParsedFreeStandingDeclSpec(S, AS, DS, MultiTemplateParamsArg());
 }
 
+// The MS ABI changed between VS2013 and VS2015 with regard to numbers used to
+// disambiguate entities defined in different scopes.
+// While the VS2015 ABI fixes potential miscompiles, it is also breaks
+// compatibility.
+// We will pick our mangling number depending on which version of MSVC is being
+// targeted.
+static unsigned getMSManglingNumber(const LangOptions &LO, Scope *S) {
+  return LO.isCompatibleWithMSVC(19) ? S->getMSCurManglingNumber()
+                                     : S->getMSLastManglingNumber();
+}
+
 void Sema::handleTagNumbering(const TagDecl *Tag, Scope *TagScope) {
   if (!Context.getLangOpts().CPlusPlus)
     return;
@@ -3428,7 +3439,8 @@ void Sema::handleTagNumbering(const TagDecl *Tag, Scope *TagScope) {
     MangleNumberingContext &MCtx =
         Context.getManglingNumberContext(Tag->getParent());
     Context.setManglingNumber(
-        Tag, MCtx.getManglingNumber(Tag, TagScope->getMSLocalManglingNumber()));
+        Tag, MCtx.getManglingNumber(
+                 Tag, getMSManglingNumber(getLangOpts(), TagScope)));
     return;
   }
 
@@ -3437,8 +3449,8 @@ void Sema::handleTagNumbering(const TagDecl *Tag, Scope *TagScope) {
   if (MangleNumberingContext *MCtx = getCurrentMangleNumberContext(
           Tag->getDeclContext(), ManglingContextDecl)) {
     Context.setManglingNumber(
-        Tag,
-        MCtx->getManglingNumber(Tag, TagScope->getMSLocalManglingNumber()));
+        Tag, MCtx->getManglingNumber(
+                 Tag, getMSManglingNumber(getLangOpts(), TagScope)));
   }
 }
 
@@ -4135,10 +4147,11 @@ Decl *Sema::BuildAnonymousStructOrUnion(Scope *S, DeclSpec &DS,
   if (VarDecl *NewVD = dyn_cast<VarDecl>(Anon)) {
     if (getLangOpts().CPlusPlus && NewVD->isStaticLocal()) {
       Decl *ManglingContextDecl;
-      if (MangleNumberingContext *MCtx =
-              getCurrentMangleNumberContext(NewVD->getDeclContext(),
-                                            ManglingContextDecl)) {
-        Context.setManglingNumber(NewVD, MCtx->getManglingNumber(NewVD, S->getMSLocalManglingNumber()));
+      if (MangleNumberingContext *MCtx = getCurrentMangleNumberContext(
+              NewVD->getDeclContext(), ManglingContextDecl)) {
+        Context.setManglingNumber(
+            NewVD, MCtx->getManglingNumber(
+                       NewVD, getMSManglingNumber(getLangOpts(), S)));
         Context.setStaticLocalNumber(NewVD, MCtx->getStaticLocalNumber(NewVD));
       }
     }
@@ -5925,11 +5938,11 @@ Sema::ActOnVariableDeclarator(Scope *S, Declarator &D, DeclContext *DC,
 
   if (getLangOpts().CPlusPlus && NewVD->isStaticLocal()) {
     Decl *ManglingContextDecl;
-    if (MangleNumberingContext *MCtx =
-            getCurrentMangleNumberContext(NewVD->getDeclContext(),
-                                          ManglingContextDecl)) {
+    if (MangleNumberingContext *MCtx = getCurrentMangleNumberContext(
+            NewVD->getDeclContext(), ManglingContextDecl)) {
       Context.setManglingNumber(
-          NewVD, MCtx->getManglingNumber(NewVD, S->getMSLocalManglingNumber()));
+          NewVD, MCtx->getManglingNumber(
+                     NewVD, getMSManglingNumber(getLangOpts(), S)));
       Context.setStaticLocalNumber(NewVD, MCtx->getStaticLocalNumber(NewVD));
     }
   }

diff --git a/test/CodeGenCXX/mangle-ms-abi-examples.cpp b/test/CodeGenCXX/mangle-ms-abi-examples.cpp
index d6ff94b6e3bbf4fb00ab8d160ecc9d73e04c3507..6b6ad89b43441c93434ac19692d3c6e138bd3e08 100644 (file)
--- a/test/CodeGenCXX/mangle-ms-abi-examples.cpp
+++ b/test/CodeGenCXX/mangle-ms-abi-examples.cpp
@@ -1,8 +1,10 @@
-// RUN: %clang_cc1 -fms-extensions -fno-rtti -emit-llvm %s -o - -triple=i386-pc-win32 | FileCheck %s
+// RUN: %clang_cc1 -fms-extensions -fno-rtti -emit-llvm %s -o - -triple=i386-pc-win32 -fms-compatibility-version=19.00 | FileCheck %s --check-prefix=CHECK --check-prefix=MSVC2015
+// RUN: %clang_cc1 -fms-extensions -fno-rtti -emit-llvm %s -o - -triple=i386-pc-win32 -fms-compatibility-version=18.00 | FileCheck %s --check-prefix=CHECK --check-prefix=MSVC2013
 
 // CHECK: @"\01??_7B@?1??foo@A@@QAEXH@Z@6B@" =
 // CHECK: @"\01??_7D@C@?1??foo@@YAXXZ@6B@" =
-// CHECK: define {{.*}} @"\01?baz@E@?3??bar@C@?1??foo@@YAXXZ@QAEXXZ@QAEXXZ"(
+// MSVC2013: define {{.*}} @"\01?baz@E@?3??bar@C@?1??foo@@YAXXZ@QAEXXZ@QAEXXZ"(
+// MSVC2015: define {{.*}} @"\01?baz@E@?1??bar@C@?1??foo@@YAXXZ@QAEXXZ@QAEXXZ"(
 
 // Microsoft Visual C++ ABI examples.
 struct A {

diff --git a/test/CodeGenCXX/mangle-ms-cxx11.cpp b/test/CodeGenCXX/mangle-ms-cxx11.cpp
index fde6f274980e8f576451bf683cc0e930734b2cb8..3f4075f7f3728e25b74e5e655aac9d74355d0ccd 100644 (file)
--- a/test/CodeGenCXX/mangle-ms-cxx11.cpp
+++ b/test/CodeGenCXX/mangle-ms-cxx11.cpp
@@ -172,7 +172,8 @@ inline int define_lambda() {
 // CHECK-DAG: @"\01??R<lambda_1>@?define_lambda@@YAHXZ@QBEHXZ"
 // Finally, we have the local which is inside of "<lambda_1>" which is inside of
 // "define_lambda". Hooray.
-// CHECK-DAG: @"\01?local@?2???R<lambda_1>@?define_lambda@@YAHXZ@QBEHXZ@4HA"
+// MSVC2013-DAG: @"\01?local@?2???R<lambda_1>@?define_lambda@@YAHXZ@QBEHXZ@4HA"
+// MSVC2015-DAG: @"\01?local@?1???R<lambda_1>@?define_lambda@@YAHXZ@QBEHXZ@4HA"
   return lambda();
 }
 

diff --git a/test/CodeGenCXX/mangle-ms-cxx14.cpp b/test/CodeGenCXX/mangle-ms-cxx14.cpp
index c06efe2edee9b5a66577021bd37a6f520891953c..a3a86851f7f9b933965bc1ff9e573975aa59be8e 100644 (file)
--- a/test/CodeGenCXX/mangle-ms-cxx14.cpp
+++ b/test/CodeGenCXX/mangle-ms-cxx14.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -std=c++1y -fms-extensions -emit-llvm %s -o - -triple=i386-pc-win32 | FileCheck %s
+// RUN: %clang_cc1 -std=c++1y -fms-extensions -emit-llvm %s -o - -triple=i386-pc-win32 -fms-compatibility-version=19.00 | FileCheck %s --check-prefix=CHECK --check-prefix=MSVC2015
+// RUN: %clang_cc1 -std=c++1y -fms-extensions -emit-llvm %s -o - -triple=i386-pc-win32 -fms-compatibility-version=18.00 | FileCheck %s --check-prefix=CHECK --check-prefix=MSVC2013
 
 template <typename> int x = 0;
 
@@ -34,7 +35,10 @@ auto TemplateFuncionWithLocalLambda(T) {
   return LocalLambdaWithLocalType();
 }
 
-// CHECK: "\01?ValueFromTemplateFuncionWithLocalLambda@@3ULocalType@?2???R<lambda_1>@??$TemplateFuncionWithLocalLambda@H@@YA?A?<auto>@@H@Z@QBA?A?3@XZ@A"
+// MSVC2013-DAG: "\01?ValueFromTemplateFuncionWithLocalLambda@@3ULocalType@?2???R<lambda_1>@??$TemplateFuncionWithLocalLambda@H@@YA?A?<auto>@@H@Z@QBA?A?3@XZ@A"
+// MSVC2013-DAG: "\01?ValueFromTemplateFuncionWithLocalLambda@@3ULocalType@?2???R<lambda_1>@??$TemplateFuncionWithLocalLambda@H@@YA?A?<auto>@@H@Z@QBA?A?3@XZ@A"
+// MSVC2015-DAG: "\01?ValueFromTemplateFuncionWithLocalLambda@@3ULocalType@?1???R<lambda_1>@??$TemplateFuncionWithLocalLambda@H@@YA?A?<auto>@@H@Z@QBA?A?3@XZ@A"
+// MSVC2015-DAG: "\01?ValueFromTemplateFuncionWithLocalLambda@@3ULocalType@?1???R<lambda_1>@??$TemplateFuncionWithLocalLambda@H@@YA?A?<auto>@@H@Z@QBA?A?3@XZ@A"
 // CHECK: "\01??$TemplateFuncionWithLocalLambda@H@@YA?A?<auto>@@H@Z"
 // CHECK: "\01??R<lambda_1>@??$TemplateFuncionWithLocalLambda@H@@YA?A?<auto>@@H@Z@QBA?A?1@XZ"
 auto ValueFromTemplateFuncionWithLocalLambda = TemplateFuncionWithLocalLambda(0);