Update Microsoft name mangling scheme for exception specifiers in the type system

[zahen]

Summary

The msvc exception specifier for noexcept function types has changed from the prior default of "Z" to "_E" if the function cannot throw when compiling with /std:C++17.

[zturner]

lib/AST/MicrosoftMangle.cpp

2311–2314

I knew that the mangling changed whenever a pointer to a noexcept function is passed as an argument, and we don't yet handle that, but I'm surprised to hear that they changed an existing mangling, since it's a hard ABI break.

Do you know the major and minor version numbers that this changed in? I'd like to test it out for starters, but also since this is an ABI break we would need to put it behind -fms-compatibility-version and only mangle using the new scheme when the compatibility version is sufficiently high.

[zturner]

Ahh I read the new test cases, and all of the test cases are about the case where noexcept function is a parameter, so maybe this is actually the case I was referring to. Can you add a test case for something like this:

void f() noexcept { }

I get this:

00A 00000000 SECT4  notype ()    External     | ?foo@@YAXXZ (void __cdecl foo(void))
00B 00000010 SECT4  notype ()    External     | ?bar@@YAXP6AXX_E@Z (void __cdecl bar(void (__cdecl*)(void) noexcept))

Showing that it is in fact only for function parameters. So we should have a test to confirm that the behavior when the noexcept function is not a parameter remains Z (from the code it looks like this might be broken under this patch).

[zturner]

Also we still need to put this behind -fms-compatibility-version. Finally, it would be nice if you could also update the demangler (llvm/lib/Demangle/MicrosoftDemangle.cpp)

[zahen]

In D55685#1330717, @zturner wrote:

Also we still need to put this behind -fms-compatibility-version. Finally, it would be nice if you could also update the demangler (llvm/lib/Demangle/MicrosoftDemangle.cpp)

This was introduced in 15.5 (1912). What's the preferred way to represent that in clang code?

lib/AST/MicrosoftMangle.cpp

2311–2314

It's only when a function is used as a type. My original rathole was trying to enumerate all of the places where that could be, but instead I settled on "everywhere but the initial definition". It's why false is passed in the 4th parameter on line 516.

I've confirmed this changed in 15.5 so I'll use that as the compat version.

[rnk]

lib/AST/MicrosoftMangle.cpp

2311–2314

I see existing code that uses this pattern: getLangOpts().isCompatibleWithMSVC(LangOptions::MSVC2015)

The MSVCMajorVersion enum is symbolic, so I think you might have to multiply it by a hundred and modify LangOptions::isCompatibleWithMSVC to multiply by two fewer places.

I guess to fit with the existing enums we'd say MSVC2017_5, even though that conflates VS and VC version numbers.

[zturner]

BTW, as far as updating the demangler, as long as it doesn't crash or generate an error on a valid _E mangling, that should be sufficient (with a test). If you want bonus points you can make it print out noexcept when you see the _E, but I won't require it as it's a bit of extra work. If you decide not to do that, I'll just file a bug for it so that we don't forget.

lib/AST/MicrosoftMangle.cpp

2311–2314

Ok, I see it now. That should be fine.

[zahen]

In D55685#1331810, @zturner wrote:

BTW, as far as updating the demangler, as long as it doesn't crash or generate an error on a valid _E mangling, that should be sufficient (with a test). If you want bonus points you can make it print out noexcept when you see the _E, but I won't require it as it's a bit of extra work. If you decide not to do that, I'll just file a bug for it so that we don't forget.

I have the full set of demangler updates ready to go and will upload shortly (Monday at the latest due to travel)

[zahen]

Added support for msvc minor version as requested. Tied the updated mangling scheme to C++17 & compatibility mode > 1912 (15.5). Added additional tests.

[rnk]

lgtm, thanks!

[zahen]

I don't have check-in permission, so I'd appreciate if someone could handle the actual commit.

include/clang/Basic/LangOptions.h

 
   enum AddrSpaceMapMangling { ASMM_Target, ASMM_On, ASMM_Off };
 
+  // Corresponds to _MSC_VER
   enum MSVCMajorVersion {
-    MSVC2010 = 16,
-    MSVC2012 = 17,
-    MSVC2013 = 18,
-    MSVC2015 = 19
+    MSVC2010 = 1600,
+    MSVC2012 = 1700,
+    MSVC2013 = 1800,
+    MSVC2015 = 1900,
+    MSVC2017 = 1910,
+    MSVC2017_5 = 1912
   };
 
   /// Clang versions with different platform ABI conformance.

   }
 
   bool isCompatibleWithMSVC(MSVCMajorVersion MajorVersion) const {
-    return MSCompatibilityVersion >= MajorVersion * 10000000U;
+    return MSCompatibilityVersion >= MajorVersion * 100000U;
   }
 
   /// Reset all of the options that are not considered when building a

lib/AST/MicrosoftMangle.cpp

                   QualifierMangleMode QMM = QMM_Mangle);
   void mangleFunctionType(const FunctionType *T,
                           const FunctionDecl *D = nullptr,
-                          bool ForceThisQuals = false);
+                          bool ForceThisQuals = false,
+                          bool MangleExceptionSpec = true);
   void mangleNestedName(const NamedDecl *ND);
 
 private:

 
     mangleFunctionClass(FD);
 
-    mangleFunctionType(FT, FD);
+    mangleFunctionType(FT, FD, false, false);
   } else {
     Out << '9';
   }

 
 void MicrosoftCXXNameMangler::mangleFunctionType(const FunctionType *T,
                                                  const FunctionDecl *D,
-                                                 bool ForceThisQuals) {
+                                                 bool ForceThisQuals,
+                                                 bool MangleExceptionSpec) {
   // <function-type> ::= <this-cvr-qualifiers> <calling-convention>
   //                     <return-type> <argument-list> <throw-spec>
   const FunctionProtoType *Proto = dyn_cast<FunctionProtoType>(T);

       Out << '@';
   }
 
-  mangleThrowSpecification(Proto);
+  if (MangleExceptionSpec && getASTContext().getLangOpts().CPlusPlus17 &&
+      getASTContext().getLangOpts().isCompatibleWithMSVC(
+          LangOptions::MSVC2017_5))
+    mangleThrowSpecification(Proto);
+  else
+    Out << 'Z';
 }
 
 void MicrosoftCXXNameMangler::mangleFunctionClass(const FunctionDecl *FD) {

 void MicrosoftCXXNameMangler::mangleCallingConvention(const FunctionType *T) {
   mangleCallingConvention(T->getCallConv());
 }
+
 void MicrosoftCXXNameMangler::mangleThrowSpecification(
                                                 const FunctionProtoType *FT) {
-  // <throw-spec> ::= Z # throw(...) (default)
-  //              ::= @ # throw() or __declspec/__attribute__((nothrow))
-  //              ::= <type>+
-  // NOTE: Since the Microsoft compiler ignores throw specifications, they are
-  // all actually mangled as 'Z'. (They're ignored because their associated
-  // functionality isn't implemented, and probably never will be.)
-  Out << 'Z';
+  // <throw-spec> ::= Z # (default)
+  //              ::= _E # noexcept
+  if (FT->canThrow())
+    Out << 'Z';
+  else
+    Out << "_E";
 }
 
 void MicrosoftCXXNameMangler::mangleType(const UnresolvedUsingType *T,

lib/Driver/ToolChains/MSVC.cpp

   if (MSVT.empty() &&
       Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
                    IsWindowsMSVC)) {
-    // -fms-compatibility-version=19.11 is default, aka 2017
+    // -fms-compatibility-version=19.11 is default, aka 2017, 15.3
     MSVT = VersionTuple(19, 11);
   }
   return MSVT;

test/CodeGenCXX/mangle-ms-exception-spec.cpp

+// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o - -triple=x86_64-pc-win32 -Wno-noexcept-type -fms-compatibility-version=19.12 | FileCheck %s --check-prefix=CHECK --check-prefix=CXX11
+// RUN: %clang_cc1 -std=c++17 -fms-extensions -emit-llvm %s -o - -triple=x86_64-pc-win32 | FileCheck %s --check-prefix=CHECK --check-prefix=NOCOMPAT
+// RUN: %clang_cc1 -std=c++17 -fms-extensions -emit-llvm %s -o - -triple=x86_64-pc-win32 -fms-compatibility-version=19.12 | FileCheck %s --check-prefix=CHECK --check-prefix=CXX17
+
+// Prove that mangling only changed for noexcept types under /std:C++17, not all noexcept functions
+// CHECK-DAG: @"?nochange@@YAXXZ"
+void nochange() noexcept {}
+
+// CXX11-DAG: @"?a@@YAXP6AHXZ@Z"
+// NOCOMPAT-DAG: @"?a@@YAXP6AHXZ@Z"
+// CXX17-DAG: @"?a@@YAXP6AHX_E@Z"
+void a(int() noexcept) {}
+// CHECK-DAG: @"?b@@YAXP6AHXZ@Z"
+void b(int() noexcept(false)) {}
+// CXX11-DAG: @"?c@@YAXP6AHXZ@Z"
+// NOCOMPAT-DAG: @"?c@@YAXP6AHXZ@Z"
+// CXX17-DAG: @"?c@@YAXP6AHX_E@Z"
+void c(int() noexcept(true)) {}
+// CHECK-DAG: @"?d@@YAXP6AHXZ@Z"
+void d(int()) {}
+
+template <typename T>
+class e;
+template <typename T, typename... U>
+class e<T(U...) noexcept> {
+  // CXX11-DAG: @"?ee@?$e@$$A6AXXZ@@EEAAXXZ"
+  // NOCOMPAT-DAG: @"?ee@?$e@$$A6AXXZ@@EEAAXXZ"
+  // CXX17-DAG: @"?ee@?$e@$$A6AXX_E@@EEAAXXZ"
+  virtual T ee(U &&...) noexcept {};
+};
+
+e<void() noexcept> e1;
+
+template <typename T>
+class f;
+template <typename T, typename... U>
+class f<T(U...)> {
+  // CHECK-DAG: @"?ff@?$f@$$A6AXXZ@@EEAAXXZ"
+  virtual T ff(U &&...) noexcept {};
+};
+
+f<void()> f1;