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 ↗

(On Diff #178166)

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 ↗

(On Diff #178166)

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 ↗

(On Diff #178166)

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 ↗

(On Diff #178166)

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.

cfe/trunk/include/clang/Basic/LangOptions.h

enum DefaultCallingConvention {
     DCC_FastCall,
     DCC_StdCall,
     DCC_VectorCall,
     DCC_RegCall
   };
 
   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.
   enum class ClangABI {
     /// Attempt to be ABI-compatible with code generated by Clang 3.8.x
     /// (SVN r257626). This causes <1 x long long> to be passed in an
     /// integer register instead of an SSE register on x64_64.
     Ver3_8,
#include "clang/Basic/LangOptions.def"
   }
 
   bool isSubscriptPointerArithmetic() const {
     return ObjCRuntime.isSubscriptPointerArithmetic() &&
            !ObjCSubscriptingLegacyRuntime;
   }
 
   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
   /// module.
   void resetNonModularOptions();
 
   /// Is this a libc/libm function that is no longer recognized as a
   /// builtin because a -fno-builtin-* option has been specified?

cfe/trunk/lib/AST/MicrosoftMangle.cpp

public:
   void mangleTagTypeKind(TagTypeKind TK);
   void mangleArtificialTagType(TagTypeKind TK, StringRef UnqualifiedName,
                               ArrayRef<StringRef> NestedNames = None);
   void mangleAddressSpaceType(QualType T, Qualifiers Quals, SourceRange Range);
   void mangleType(QualType T, SourceRange Range,
                   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:
   bool isStructorDecl(const NamedDecl *ND) const {
     return ND == Structor || getStructor(ND) == Structor;
   }
 
   void mangleUnqualifiedName(const NamedDecl *ND) {
if (ShouldMangle) {
     // component but this would break compatibility with MSVC's behavior.
     // Instead, do this when we know that compatibility isn't important (in
     // other words, when it is an overloaded extern "C" function).
     if (FD->isExternC() && FD->hasAttr<OverloadableAttr>())
       Out << "$$J0";
 
     mangleFunctionClass(FD);
 
-    mangleFunctionType(FT, FD);
+    mangleFunctionType(FT, FD, false, false);
   } else {
     Out << '9';
   }
 }
 
 void MicrosoftCXXNameMangler::mangleVariableEncoding(const VarDecl *VD) {
   // <type-encoding> ::= <storage-class> <variable-type>
   // <storage-class> ::= 0  # private static member

 void MicrosoftCXXNameMangler::mangleType(const FunctionNoProtoType *T,
                                          Qualifiers, SourceRange) {
   Out << "$$A6";
   mangleFunctionType(T);
 }
 
 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);
 
   SourceRange Range;
   if (D) Range = D->getSourceRange();
 
   bool IsInLambda = false;
if (!Proto) {
     }
     // <builtin-type>      ::= Z  # ellipsis
     if (Proto->isVariadic())
       Out << 'Z';
     else
       Out << '@';
   }
 
+  if (MangleExceptionSpec && getASTContext().getLangOpts().CPlusPlus17 &&
+      getASTContext().getLangOpts().isCompatibleWithMSVC(
+          LangOptions::MSVC2017_5))
     mangleThrowSpecification(Proto);
+  else
+    Out << 'Z';
 }
 
 void MicrosoftCXXNameMangler::mangleFunctionClass(const FunctionDecl *FD) {
   // <function-class>  ::= <member-function> E? # E designates a 64-bit 'this'
   //                                            # pointer. in 64-bit mode *all*
   //                                            # 'this' pointers are 64-bit.
   //                   ::= <global-function>
   // <member-function> ::= A # private: near
switch (CC) {
     case CC_Swift: Out << 'S'; break;
     case CC_PreserveMost: Out << 'U'; break;
     case CC_X86RegCall: Out << 'w'; break;
   }
 }
 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.)
+  // <throw-spec> ::= Z # (default)
+  //              ::= _E # noexcept
+  if (FT->canThrow())
     Out << 'Z';
+  else
+    Out << "_E";
 }
 
 void MicrosoftCXXNameMangler::mangleType(const UnresolvedUsingType *T,
                                          Qualifiers, SourceRange Range) {
   // Probably should be mangled as a template instantiation; need to see what
   // VC does first.
   DiagnosticsEngine &Diags = Context.getDiags();
   unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,

cfe/trunk/lib/Driver/ToolChains/MSVC.cpp

VersionTuple MSVCToolChain::computeMSVCVersion(const Driver *D,
   VersionTuple MSVT = ToolChain::computeMSVCVersion(D, Args);
   if (MSVT.empty())
     MSVT = getMSVCVersionFromTriple(getTriple());
   if (MSVT.empty() && IsWindowsMSVC)
     MSVT = getMSVCVersionFromExe(getSubDirectoryPath(SubDirectoryType::Bin));
   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;
 }
 
 std::string
 MSVCToolChain::ComputeEffectiveClangTriple(const ArgList &Args,
                                            types::ID InputType) const {

cfe/trunk/lib/Sema/SemaDeclAttr.cpp

 }
 
 static void handleLayoutVersion(Sema &S, Decl *D, const ParsedAttr &AL) {
   uint32_t Version;
   Expr *VersionExpr = static_cast<Expr *>(AL.getArgAsExpr(0));
   if (!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), Version))
     return;
 
+  // The attribute expects a "major" version number like 19, but new versions of
+  // MSVC have moved to updating the "minor", or less significant numbers, so we
+  // have to multiply by 100 now.
+  Version *= 100;
+
   // TODO: Investigate what happens with the next major version of MSVC.
   if (Version != LangOptions::MSVC2015) {
     S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_bounds)
         << AL << Version << VersionExpr->getSourceRange();
     return;
   }
 
   D->addAttr(::new (S.Context)