Bug 1608460 - Redefine MIDL names to be unique to work around a clang-cl bug r=froydnj
authorDavid Major <dmajor@mozilla.com>
Fri, 10 Jan 2020 17:53:29 +0000
changeset 509863 faddb73036a34c0f0b182017a7ece2dfd9244546
parent 509862 6c9f42c650c19e798ea9f61344a1ac3f48934891
child 509864 84052ee4136c7d73ed83e2d3134294666b0ad9b2
push id37005
push usernerli@mozilla.com
push dateSat, 11 Jan 2020 21:51:30 +0000
treeherdermozilla-central@408f6c0f9814 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1608460
milestone74.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1608460 - Redefine MIDL names to be unique to work around a clang-cl bug r=froydnj This is an alternative to carrying a revert of the offending changeset, since the bug doesn't show hope of a fix anytime soon, or maybe ever. This way we don't have to keep rebasing the patch as we pick up new clangs, and developers don't have to remember to apply the patch when building a local compiler. Differential Revision: https://phabricator.services.mozilla.com/D59200
accessible/interfaces/ia2/moz.build
accessible/ipc/win/handler/moz.build
accessible/ipc/win/moz.build
build/build-clang/clang-linux64-cross.json
build/build-clang/clang-win64.json
build/build-clang/revert-r359260-due-to-bug41817.patch
--- a/accessible/interfaces/ia2/moz.build
+++ b/accessible/interfaces/ia2/moz.build
@@ -79,19 +79,25 @@ GENERATED_FILES += [
     'AccessibleValue.h',
     'AccessibleValue_i.c',
     'AccessibleValue_p.c',
     'dlldata.c',
     'IA2CommonTypes.h',
     'IA2Typelib.tlb',
 ]
 
-SOURCES += [
-    '!%s' % p for p in GENERATED_FILES if p.endswith('.c')
-]
+for p in GENERATED_FILES:
+    if p.endswith('.c'):
+        SOURCES += ['!%s' % p]
+
+        # Give some symbols a unique name in each translation unit, to avoid
+        # collisions caused by https://llvm.org/pr41817.
+        if CONFIG['CC_TYPE'] == 'clang-cl':
+            SOURCES['!%s' % p].flags += ['-DObject_StubDesc=Object_StubDesc__%s' % p[:-2]]
+            SOURCES['!%s' % p].flags += ['-DUserMarshalRoutines=UserMarshalRoutines__%s' % p[:-2]]
 
 RCINCLUDE = 'IA2Marshal.rc'
 
 # Suppress warnings from the MIDL generated code.
 if CONFIG['CC_TYPE'] == 'clang-cl':
     CFLAGS += [
         '-Wno-extern-initializer',
         '-Wno-incompatible-pointer-types',
--- a/accessible/ipc/win/handler/moz.build
+++ b/accessible/ipc/win/handler/moz.build
@@ -32,16 +32,25 @@ GENERATED_FILES += [
     'dlldata.c',
     'HandlerData.h',
     'HandlerData.tlb',
     'HandlerData_c.c',
     'HandlerData_i.c',
     'HandlerData_p.c',
 ]
 
+# Give some symbols a unique name in each translation unit, to avoid
+# collisions caused by https://llvm.org/pr41817.
+if CONFIG['CC_TYPE'] == 'clang-cl':
+    SOURCES['!HandlerData_p.c'].flags += ['-DHandlerData__MIDL_ProcFormatString=HandlerData__MIDL_ProcFormatString__HandlerData_p']
+    SOURCES['!HandlerData_p.c'].flags += ['-DHandlerData__MIDL_TypeFormatString=HandlerData__MIDL_TypeFormatString__HandlerData_p']
+    for p in GENERATED_FILES:
+        if p.endswith('.c'):
+            SOURCES['!%s' % p].flags += ['-DUserMarshalRoutines=UserMarshalRoutines__%s' % p[:-2]]
+
 DEFFILE = 'AccessibleHandler.def'
 
 USE_LIBS += [
     'mscom_oop',
 ]
 
 OS_LIBS += [
     'rpcrt4',
--- a/accessible/ipc/win/moz.build
+++ b/accessible/ipc/win/moz.build
@@ -34,16 +34,21 @@ if CONFIG['ACCESSIBILITY']:
         '!./handler/HandlerData_c.c',
         'COMPtrTypes.cpp',
         'DocAccessibleChild.cpp',
         'HandlerProvider.cpp',
         'PlatformChild.cpp',
         'ProxyAccessible.cpp',
     ]
 
+    # Give some symbols a unique name in each translation unit, to avoid
+    # collisions caused by https://llvm.org/pr41817.
+    if CONFIG['CC_TYPE'] == 'clang-cl':
+        SOURCES['!./handler/HandlerData_c.c'].flags += ['-DUserMarshalRoutines=UserMarshalRoutines__HandlerData_c']
+
     LOCAL_INCLUDES += [
         '/accessible/base',
         '/accessible/generic',
         '/accessible/windows/ia2',
         '/accessible/windows/msaa',
         '/accessible/xpcom',
     ]
 
--- a/build/build-clang/clang-linux64-cross.json
+++ b/build/build-clang/clang-linux64-cross.json
@@ -10,12 +10,11 @@
     "cxx": "{MOZ_FETCHES_DIR}/gcc/bin/g++",
     "as": "{MOZ_FETCHES_DIR}/gcc/bin/gcc",
     "patches": [
       "static-llvm-symbolizer.patch",
       "find_symbolizer_linux.patch",
       "rename_gcov_flush.patch",
       "critical_section_on_gcov_flush-rG02ce9d8ef5a8.patch",
       "android-mangling-error.patch",
-      "revert-r359260-due-to-bug41817.patch",
       "rG7e18aeba5062.patch"
     ]
 }
--- a/build/build-clang/clang-win64.json
+++ b/build/build-clang/clang-win64.json
@@ -5,13 +5,12 @@
     "assertions": false,
     "python_path": "c:/mozilla-build/python/python.exe",
     "cc": "cl.exe",
     "cxx": "cl.exe",
     "ml": "ml64.exe",
     "patches": [
       "unpoison-thread-stacks.patch",
       "downgrade-mangling-error.patch",
-      "revert-r359260-due-to-bug41817.patch",
       "rG7e18aeba5062.patch",
       "loosen-msvc-detection.patch"
     ]
 }
deleted file mode 100644
--- a/build/build-clang/revert-r359260-due-to-bug41817.patch
+++ /dev/null
@@ -1,341 +0,0 @@
-https://llvm.org/pr41817 "The linkage changes in r359260 break MIDL code"
-
-MIDL produces non-standards-compliant C code for some of our .idl files. MSVC
-and older versions of clang-cl used to accept it, but r359260 inadvertently
-caused such code to break. Until a fix is available upstream, let's revert
-r359260 in the meantime.
-
-Index: lib/AST/Decl.cpp
-===================================================================
---- a/clang/lib/AST/Decl.cpp
-+++ b/clang/lib/AST/Decl.cpp
-@@ -610,18 +610,6 @@
-   return LinkageInfo::external();
- }
- 
--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();
--  }
--  return SC_None;
--}
--
- LinkageInfo
- LinkageComputer::getLVForNamespaceScopeDecl(const NamedDecl *D,
-                                             LVComputationKind computation,
-@@ -633,28 +621,24 @@
-   // C++ [basic.link]p3:
-   //   A name having namespace scope (3.3.6) has internal linkage if it
-   //   is the name of
-+  //     - an object, reference, function or function template that is
-+  //       explicitly declared static; or,
-+  // (This bullet corresponds to C99 6.2.2p3.)
-+  if (const auto *Var = dyn_cast<VarDecl>(D)) {
-+    // Explicitly declared static.
-+    if (Var->getStorageClass() == SC_Static)
-+      return getInternalLinkageFor(Var);
- 
--  if (getStorageClass(D->getCanonicalDecl()) == SC_Static) {
--    // - a variable, variable template, function, or function template
--    //   that is explicitly declared static; or
--    // (This bullet corresponds to C99 6.2.2p3.)
--    return getInternalLinkageFor(D);
--  }
--
--  if (const auto *Var = dyn_cast<VarDecl>(D)) {
--    // - a non-template variable of non-volatile const-qualified type, unless
--    //   - it is explicitly declared extern, or
--    //   - it is inline or exported, or
--    //   - it was previously declared and the prior declaration did not have
--    //     internal linkage
--    // (There is no equivalent in C99.)
-+    // - a non-inline, non-volatile object or reference that is explicitly
-+    //   declared const or constexpr and neither explicitly declared extern
-+    //   nor previously declared to have external linkage; or (there is no
-+    //   equivalent in C99)
-+    // The C++ modules TS adds "non-exported" to this list.
-     if (Context.getLangOpts().CPlusPlus &&
-         Var->getType().isConstQualified() &&
-         !Var->getType().isVolatileQualified() &&
-         !Var->isInline() &&
--        !isExportedFromModuleInterfaceUnit(Var) &&
--        !isa<VarTemplateSpecializationDecl>(Var) &&
--        !Var->getDescribedVarTemplate()) {
-+        !isExportedFromModuleInterfaceUnit(Var)) {
-       const VarDecl *PrevVar = Var->getPreviousDecl();
-       if (PrevVar)
-         return getLVForDecl(PrevVar, computation);
-@@ -674,6 +658,14 @@
-       if (PrevVar->getStorageClass() == SC_Static)
-         return getInternalLinkageFor(Var);
-     }
-+  } else if (const FunctionDecl *Function = D->getAsFunction()) {
-+    // C++ [temp]p4:
-+    //   A non-member function template can have internal linkage; any
-+    //   other template name shall have external linkage.
-+
-+    // Explicitly declared static.
-+    if (Function->getCanonicalDecl()->getStorageClass() == SC_Static)
-+      return getInternalLinkageFor(Function);
-   } else if (const auto *IFD = dyn_cast<IndirectFieldDecl>(D)) {
-     //   - a data member of an anonymous union.
-     const VarDecl *VD = IFD->getVarDecl();
-@@ -682,8 +674,6 @@
-   }
-   assert(!isa<FieldDecl>(D) && "Didn't expect a FieldDecl!");
- 
--  // FIXME: This gives internal linkage to names that should have no linkage
--  // (those not covered by [basic.link]p6).
-   if (D->isInAnonymousNamespace()) {
-     const auto *Var = dyn_cast<VarDecl>(D);
-     const auto *Func = dyn_cast<FunctionDecl>(D);
-@@ -743,20 +733,10 @@
- 
-   // C++ [basic.link]p4:
- 
--  //   A name having namespace scope that has not been given internal linkage
--  //   above and that is the name of
--  //   [...bullets...]
--  //   has its linkage determined as follows:
--  //     - if the enclosing namespace has internal linkage, the name has
--  //       internal linkage; [handled above]
--  //     - otherwise, if the declaration of the name is attached to a named
--  //       module and is not exported, the name has module linkage;
--  //     - otherwise, the name has external linkage.
--  // LV is currently set up to handle the last two bullets.
-+  //   A name having namespace scope has external linkage if it is the
-+  //   name of
-   //
--  //   The bullets are:
--
--  //     - a variable; or
-+  //     - an object or reference, unless it has internal linkage; or
-   if (const auto *Var = dyn_cast<VarDecl>(D)) {
-     // GCC applies the following optimization to variables and static
-     // data members, but not to functions:
-@@ -802,7 +782,7 @@
-       mergeTemplateLV(LV, spec, computation);
-     }
- 
--  //     - a function; or
-+  //     - a function, unless it has internal linkage; or
-   } else if (const auto *Function = dyn_cast<FunctionDecl>(D)) {
-     // In theory, we can modify the function's LV by the LV of its
-     // type unless it has C linkage (see comment above about variables
-@@ -856,8 +836,7 @@
-       mergeTemplateLV(LV, spec, computation);
-     }
- 
--  // FIXME: This is not part of the C++ standard any more.
--  //     - an enumerator belonging to an enumeration with external linkage; or
-+  //     - an enumerator belonging to an enumeration with external linkage;
-   } else if (isa<EnumConstantDecl>(D)) {
-     LinkageInfo EnumLV = getLVForDecl(cast<NamedDecl>(D->getDeclContext()),
-                                       computation);
-@@ -865,7 +844,8 @@
-       return LinkageInfo::none();
-     LV.merge(EnumLV);
- 
--  //     - a template
-+  //     - a template, unless it is a function template that has
-+  //       internal linkage (Clause 14);
-   } else if (const auto *temp = dyn_cast<TemplateDecl>(D)) {
-     bool considerVisibility = !hasExplicitVisibilityAlready(computation);
-     LinkageInfo tempLV =
-@@ -872,9 +852,8 @@
-       getLVForTemplateParameterList(temp->getTemplateParameters(), computation);
-     LV.mergeMaybeWithVisibility(tempLV, considerVisibility);
- 
--  //     An unnamed namespace or a namespace declared directly or indirectly
--  //     within an unnamed namespace has internal linkage. All other namespaces
--  //     have external linkage.
-+  //     - a namespace (7.3), unless it is declared within an unnamed
-+  //       namespace.
-   //
-   // We handled names in anonymous namespaces above.
-   } else if (isa<NamespaceDecl>(D)) {
-Index: test/CXX/drs/dr23xx.cpp
-===================================================================
---- a/clang/test/CXX/drs/dr23xx.cpp
-+++ b/clang/test/CXX/drs/dr23xx.cpp
-@@ -60,20 +60,3 @@
- void wrong_value() { auto [x, y] = Bad2(); } // expected-error {{decomposes into 42 elements}}
- } // namespace dr2386
- #endif
--
--namespace dr2387 { // dr2387: 9
--#if __cplusplus >= 201402L
--  template<int> int a = 0;
--  extern template int a<0>; // ok
--
--  template<int> static int b = 0;
--  extern template int b<0>; // expected-error {{internal linkage}}
--
--  template<int> const int c = 0;
--  extern template const int c<0>; // ok, has external linkage despite 'const'
--
--  template<typename T> T d = 0;
--  extern template int d<int>;
--  extern template const int d<const int>;
--#endif
--}
-Index: test/CXX/module/module.interface/p3.cpp
-===================================================================
---- a/clang/test/CXX/module/module.interface/p3.cpp
-+++ b/clang/test/CXX/module/module.interface/p3.cpp
-@@ -48,7 +48,7 @@
- namespace { // expected-note {{here}}
-   export int d; // expected-error {{export declaration appears within anonymous namespace}}
- }
--export template<typename> static int e; // expected-error {{declaration of 'e' with internal linkage cannot be exported}}
-+export template<typename> static int e; // FIXME
- export template<typename> static int f(); // expected-error {{declaration of 'f' with internal linkage cannot be exported}}
- export const int k = 5;
- export static union { int n; }; // expected-error {{declaration of 'n' with internal linkage cannot be exported}}
-Index: test/CXX/module/module.interface/p5.cpp
-===================================================================
---- a/clang/test/CXX/module/module.interface/p5.cpp
-+++ b/clang/test/CXX/module/module.interface/p5.cpp
-@@ -14,7 +14,7 @@
- namespace NS {}
- 
- template<typename> int ta;
--template<typename> static int sta; // expected-note {{target}}
-+template<typename> static int sta;
- template<typename> void tb();
- template<typename> static void stb(); // expected-note {{target}}
- template<typename> struct tc {};
-@@ -44,7 +44,7 @@
-   }
- }
- 
--export { // expected-note 19{{here}}
-+export { // expected-note 18{{here}}
-   using ::a;
-   using ::sa; // expected-error {{using declaration referring to 'sa' with internal linkage}}
-   using ::b;
-@@ -56,7 +56,7 @@
-   using ::sg1; // expected-error {{using declaration referring to 'sg1' with internal linkage}}
- 
-   using ::ta;
--  using ::sta; // expected-error {{using declaration referring to 'sta' with internal linkage}}
-+  using ::sta; // FIXME {{using declaration referring to 'sta' with internal linkage}}
-   using ::tb;
-   using ::stb; // expected-error {{using declaration referring to 'stb' with internal linkage}}
-   using ::tc;
-Index: test/CodeGenCXX/cxx1y-variable-template-linkage.cpp
-===================================================================
---- a/clang/test/CodeGenCXX/cxx1y-variable-template-linkage.cpp
-+++ b/clang/test/CodeGenCXX/cxx1y-variable-template-linkage.cpp
-@@ -6,61 +6,21 @@
- // should be 'internal global' and not 'linkonce_odr global'.
- 
- template <typename T> int x = 42;
--// CHECK-DAG: @_Z1xIiE = linkonce_odr global
-+
- // CHECK-DAG: @_Z1xIZL3foovE3FooE = internal global
- 
--// 'static' affects the linkage of the global
--template <typename T> static int y = 42;
--// CHECK-DAG: @_ZL1yIiE = internal global
--// CHECK-DAG: @_ZL1yIZL3foovE3FooE = internal global
--
--// 'const' does not
--template <typename T> const int z = 42;
--// CHECK-DAG: @_Z1zIiE = linkonce_odr constant
--// CHECK-DAG: @_Z1zIZL3foovE3FooE = internal constant
--
--template <typename T> T t = 42;
--// CHECK-DAG: @_Z1tIiE = linkonce_odr global
--// CHECK-DAG: @_Z1tIKiE = linkonce_odr constant
--
--int mode;
--
- // CHECK-DAG: define internal dereferenceable(4) i32* @_ZL3foov(
--static const int &foo() {
-+static int &foo() {
-    struct Foo { };
--
--   switch (mode) {
--   case 0:
--     // CHECK-DAG: @_Z1xIiE
--     return x<int>;
--   case 1:
--     // CHECK-DAG: @_Z1xIZL3foovE3FooE
--     return x<Foo>;
--   case 2:
--     // CHECK-DAG: @_ZL1yIiE
--     return y<int>;
--   case 3:
--     // CHECK-DAG: @_ZL1yIZL3foovE3FooE
--     return y<Foo>;
--   case 4:
--     // CHECK-DAG: @_Z1zIiE
--     return z<int>;
--   case 5:
--     // CHECK-DAG: @_Z1zIZL3foovE3FooE
--     return z<Foo>;
--   case 6:
--     // CHECK-DAG: @_Z1tIiE
--     return t<int>;
--   case 7:
--     // CHECK-DAG: @_Z1tIKiE
--     return t<const int>;
--   }
-+   
-+   // CHECK-DAG: ret i32* @_Z1xIZL3foovE3FooE
-+   return x<Foo>;
- }
- 
- 
- #if !__has_feature(cxx_exceptions) // File A
- // CHECKA-DAG: define dereferenceable(4) i32* @_Z3barv(
--const int &bar() {
-+int &bar() { 
- 	// CHECKA-DAG: call dereferenceable(4) i32* @_ZL3foov()
- 	return foo();
- }
-@@ -68,7 +28,7 @@
- #else // File B
- 
- // CHECKB-DAG: declare dereferenceable(4) i32* @_Z3barv(
--const int &bar();
-+int &bar();
- 
- int main() {
- 	// CHECKB-DAG: call dereferenceable(4) i32* @_Z3barv()
-Index: test/SemaCXX/warn-unused-filescoped.cpp
-===================================================================
---- a/clang/test/SemaCXX/warn-unused-filescoped.cpp
-+++ b/clang/test/SemaCXX/warn-unused-filescoped.cpp
-@@ -207,9 +207,8 @@
- 
- namespace test10 {
- #if __cplusplus >= 201103L
--// FIXME: Warn on template definitions with no instantiations?
- template<class T>
--constexpr T pi = T(3.14);
-+constexpr T pi = T(3.14); // expected-warning {{unused}}
- #endif
- }
- 
-Index: test/SemaCXX/warn-unused-variables.cpp
-===================================================================
---- a/clang/test/SemaCXX/warn-unused-variables.cpp
-+++ b/clang/test/SemaCXX/warn-unused-variables.cpp
-@@ -135,9 +135,7 @@
-   template<typename T> int m = 0;
-   template<typename T> int m<T*> = 0;
- 
--  // This has external linkage, so could be referenced by a declaration in a
--  // different translation unit.
--  template<> const int m<void> = 0; // no warning
-+  template<> const int m<void> = 0; // expected-warning {{unused variable}}
- }
- 
- namespace ctor_with_cleanups {