bindexplib: Export the same symbols on every host

WINDOWS_EXPORT_ALL_SYMBOLS reads object files with two backends: on
Windows hosts CMake walks the COFF symbol table itself, elsewhere (and
for LLVM bitcode) it parses `nm` output.  Only the walker applied the
rules the MSVC ABI needs, so a DLL cross-compiled from Linux or macOS
exported a different set of symbols than the same sources built on
Windows.

Move every rule that depends only on the symbol name into one place
used by both backends, and teach the nm backend the rest: skip
deleting destructors (`??_G`, `??_E`), export vftables (`??_7`) as code
except on ARM64EC where they are DATA, ignore common symbols, and strip
i386 decorations.  The machine type these need is read from the COFF
file header, once per object file.

The expected output of the nm symbol-type test changes accordingly.
Add a test running `cmake -E __create_def` over checked-in objects for
x86_64, i386 and ARM64EC against expected .def files, so one
expectation pins both backends.

Fixes: #28103

.gitattributes

@@ -51,6 +51,7 @@ coverage.xml.in  tab-indent
 *.make           tab-indent
 .hooks-config    tab-indent
 
+*.obj            -text
 *.pfx            -text
 *.png            -text
 *.png.in         -text

Source/bindexplib.cxx

@@ -67,14 +67,16 @@
 #include <algorithm>
 #include <cstddef> // IWYU pragma: keep
 #include <iostream>
+#include <map>
 #include <sstream>
+#include <string>
+#include <utility>
 #include <vector>
 
 #ifdef _WIN32
 #  include <windows.h>
 
 #  include "cmsys/Encoding.hxx"
-#  include "cmsys/String.h"
 #endif
 
 #include "cmsys/FStream.hxx"
@@ -84,6 +86,133 @@
 #include "cmStringAlgorithms.h"
 #include "cmSystemTools.h"
 
+namespace {
+
+enum class Arch
+{
+  Generic,
+  I386,
+  ARM64EC,
+};
+
+/*
+ * The two backends below, the COFF symbol table walker used on Windows
+ * hosts and the nm reader used everywhere else, must select the same
+ * symbols, so every rule that depends only on the symbol name is shared.
+ */
+
+bool SymbolIsFromManagedCode(std::string const& symbol)
+{
+  return symbol == "__t2m" || symbol == "__m2mep" || symbol == "__mep" ||
+    symbol.find("$$F") != std::string::npos ||
+    symbol.find("$$J") != std::string::npos;
+}
+
+bool SymbolIsOperatorExternC(std::string const& symbol)
+{
+  return symbol.find_first_not_of("=<>+-*/%,?|~!^&[]()") == std::string::npos;
+}
+
+/*
+ * The "(vector) deleting destructor" must not be exported:
+ *  "??_G"  scalar deleting dtor
+ *  "??_E"  vector deleting dtor
+ */
+bool SymbolIsDeletingDestructor(std::string const& symbol)
+{
+  return cmHasLiteralPrefix(symbol, "??_G") ||
+    cmHasLiteralPrefix(symbol, "??_E");
+}
+
+bool SymbolIsVftable(std::string const& symbol)
+{
+  return cmHasLiteralPrefix(symbol, "??_7");
+}
+
+bool SymbolIsArm64ECThunk(std::string const& symbol)
+{
+  return symbol.find("$ientry_thunk") != std::string::npos ||
+    symbol.find("$entry_thunk") != std::string::npos ||
+    symbol.find("$iexit_thunk") != std::string::npos ||
+    symbol.find("$exit_thunk") != std::string::npos;
+}
+
+/*
+ * Strip the decorations that must not appear in the generated .def file.
+ */
+void NormalizeSymbolName(std::string& symbol, Arch arch)
+{
+  // clear out any leading spaces
+  symbol.erase(0, symbol.find_first_not_of(" \t\n\v\f\r"));
+  // if it starts with _ and has an @ then it is a __cdecl
+  // so remove the @ stuff for the export
+  if (!symbol.empty() && symbol[0] == '_') {
+    std::string::size_type posAt = symbol.find('@');
+    if (posAt != std::string::npos) {
+      symbol.erase(posAt);
+    }
+  }
+  // For i386 builds we need to remove _
+  if (arch == Arch::I386 && !symbol.empty() && symbol[0] == '_') {
+    symbol.erase(0, 1);
+  }
+}
+
+/*
+ * Symbols that are never exported, whatever section they are defined in.
+ */
+bool SymbolIsExcluded(std::string const& symbol, Arch arch)
+{
+  // The original code had a check for
+  //     symbol.find("real@") == std::string::npos)
+  // but this disallows member functions with the name "real".
+  return symbol.empty() || SymbolIsDeletingDestructor(symbol) ||
+    // Skip symbols containing a dot, that are from managed code,
+    // or that are C++ operators incorrectly declared extern "C".
+    symbol.find('.') != std::string::npos || SymbolIsFromManagedCode(symbol) ||
+    SymbolIsOperatorExternC(symbol) ||
+    // Skip arm64ec thunk symbols.
+    (arch == Arch::ARM64EC && SymbolIsArm64ECThunk(symbol));
+}
+
+/*
+ * Read the machine type out of a COFF object file header so that the nm
+ * backend can apply the same architecture-specific rules as the COFF
+ * symbol table walker.  Inputs that are not COFF object files, such as
+ * LLVM bitcode, are reported as generic.
+ */
+Arch GetObjectArch(std::string const& filename)
+{
+  cmsys::ifstream fin(filename.c_str(), std::ios::in | std::ios::binary);
+  if (!fin) {
+    return Arch::Generic;
+  }
+  unsigned char header[8] = { 0 };
+  fin.read(reinterpret_cast<char*>(header), sizeof(header));
+  if (!fin) {
+    return Arch::Generic;
+  }
+  auto readWord = [&header](size_t i) -> unsigned int {
+    return static_cast<unsigned int>(header[i]) |
+      (static_cast<unsigned int>(header[i + 1]) << 8);
+  };
+  unsigned int machine = readWord(0);
+  if (machine == 0x0000 && readWord(2) == 0xffff) {
+    // In a /bigobj file the machine type follows Sig1, Sig2 and Version.
+    machine = readWord(6);
+  }
+  switch (machine) {
+    case 0x014c: // IMAGE_FILE_MACHINE_I386
+      return Arch::I386;
+    case 0xa641: // IMAGE_FILE_MACHINE_ARM64EC
+      return Arch::ARM64EC;
+    default:
+      return Arch::Generic;
+  }
+}
+
+} // namespace
+
 #ifdef _WIN32
 #  ifndef IMAGE_FILE_MACHINE_ARM
 #    define IMAGE_FILE_MACHINE_ARM 0x01c0 // ARM Little-Endian
@@ -145,13 +274,6 @@ typedef struct _cmIMAGE_SYMBOL_EX
 } cmIMAGE_SYMBOL_EX;
 typedef cmIMAGE_SYMBOL_EX UNALIGNED* cmPIMAGE_SYMBOL_EX;
 
-enum class Arch
-{
-  Generic,
-  I386,
-  ARM64EC,
-};
-
 PIMAGE_SECTION_HEADER GetSectionHeaderOffset(
   PIMAGE_FILE_HEADER pImageFileHeader)
 {
@@ -266,67 +388,28 @@ public:
             symbol = stringTable + pSymbolTable->N.Name.Long;
           }
 
-          // clear out any leading spaces
-          while (cmsysString_isspace(symbol[0]))
-            symbol.erase(0, 1);
-          // if it starts with _ and has an @ then it is a __cdecl
-          // so remove the @ stuff for the export
-          if (symbol[0] == '_') {
-            std::string::size_type posAt = symbol.find('@');
-            if (posAt != std::string::npos) {
-              symbol.erase(posAt);
-            }
-          }
-          // For i386 builds we need to remove _
-          if (this->SymbolArch == Arch::I386 && symbol[0] == '_') {
-            symbol.erase(0, 1);
-          }
-
-          // Check whether it is "Scalar deleting destructor" and "Vector
-          // deleting destructor"
-          // if scalarPrefix and vectorPrefix are not found then print
-          // the symbol
-          char const* scalarPrefix = "??_G";
-          char const* vectorPrefix = "??_E";
-          char const* vftablePrefix = "??_7";
-          // The original code had a check for
-          //     symbol.find("real@") == std::string::npos)
-          // but this disallows member functions with the name "real".
-          if (symbol.compare(0, 4, scalarPrefix) &&
-              symbol.compare(0, 4, vectorPrefix)) {
+          NormalizeSymbolName(symbol, this->SymbolArch);
+          if (!SymbolIsExcluded(symbol, this->SymbolArch)) {
             SectChar = this->SectionHeaders[pSymbolTable->SectionNumber - 1]
                          .Characteristics;
-            // Skip symbols containing a dot, are from managed code,
-            // or are C++ operators incorrectly declared extern "C".
-            if (symbol.find('.') == std::string::npos &&
-                !SymbolIsFromManagedCode(symbol) &&
-                !SymbolIsOperatorExternC(symbol)) {
-              // skip arm64ec thunk symbols
-              if (this->SymbolArch != Arch::ARM64EC ||
-                  (symbol.find("$ientry_thunk") == std::string::npos &&
-                   symbol.find("$entry_thunk") == std::string::npos &&
-                   symbol.find("$iexit_thunk") == std::string::npos &&
-                   symbol.find("$exit_thunk") == std::string::npos)) {
             if ((!pSymbolTable->Type &&
                  // Read only (i.e. constants) must be excluded
                  (SectChar & IMAGE_SCN_MEM_WRITE)) ||
                 (this->SymbolArch == Arch::ARM64EC &&
                  // vftable symbols are DATA on ARM64EC
-                     symbol.compare(0, 4, vftablePrefix) == 0)) {
+                 SymbolIsVftable(symbol))) {
               this->DataSymbols.insert(symbol);
             } else if (pSymbolTable->Type ||
                        !(SectChar & IMAGE_SCN_MEM_READ) ||
                        (SectChar & IMAGE_SCN_MEM_EXECUTE) ||
                        (this->SymbolArch != Arch::ARM64EC &&
                         // vftable symbols fail if marked as DATA
-                            symbol.compare(0, 4, vftablePrefix) == 0)) {
+                        SymbolIsVftable(symbol))) {
               this->Symbols.insert(symbol);
             }
           }
         }
       }
-        }
-      }
 
       /*
        * Take into account any aux symbols
@@ -338,19 +421,6 @@ public:
   }
 
 private:
-  bool SymbolIsFromManagedCode(std::string const& symbol)
-  {
-    return symbol == "__t2m" || symbol == "__m2mep" || symbol == "__mep" ||
-      symbol.find("$$F") != std::string::npos ||
-      symbol.find("$$J") != std::string::npos;
-  }
-
-  bool SymbolIsOperatorExternC(std::string const& symbol)
-  {
-    return symbol.find_first_not_of("=<>+-*/%,?|~!^&[]()") ==
-      std::string::npos;
-  }
-
   std::set<std::string>& Symbols;
   std::set<std::string>& DataSymbols;
   DWORD_PTR SymbolCount;
@@ -415,6 +485,12 @@ static bool DumpFileWithNm(std::string const& nmPath,
     return false;
   }
 
+  // The COFF symbol table walker applies rules that depend on the object
+  // file's architecture, but nm does not report it.  Read it from the
+  // object files named in the output, remembering it because one nm run
+  // may cover many object files.
+  std::map<std::string, Arch> archOfObject;
+
   std::istringstream ss(output);
   std::string line;
   while (std::getline(ss, line)) {
@@ -440,10 +516,22 @@ static bool DumpFileWithNm(std::string const& nmPath,
       return false;
     }
     char const sym_type = line[sym_end + 1];
-    std::string const symbol = line.substr(sym_start, sym_end - sym_start);
+    std::string const objectFile = line.substr(0, filename_end);
+    std::string symbol = line.substr(sym_start, sym_end - sym_start);
+
+    auto archEntry = archOfObject.find(objectFile);
+    if (archEntry == archOfObject.end()) {
+      archEntry =
+        archOfObject.emplace(objectFile, GetObjectArch(objectFile)).first;
+    }
+    Arch const arch = archEntry->second;
+
+    NormalizeSymbolName(symbol, arch);
+    if (SymbolIsExcluded(symbol, arch)) {
+      continue;
+    }
     switch (sym_type) {
       case 'B':
-      case 'C':
       case 'D':
         dataSymbols.insert(symbol);
         continue;
@@ -451,11 +539,21 @@ static bool DumpFileWithNm(std::string const& nmPath,
         symbols.insert(symbol);
         continue;
       case 'R':
-        if (cmHasLiteralPrefix(symbol, "??_7")) {
+        // Read only (i.e. constants) must be excluded, but vftables live
+        // there too.  They fail if marked as DATA, except on ARM64EC where
+        // they must be marked as DATA.
+        if (SymbolIsVftable(symbol)) {
+          if (arch == Arch::ARM64EC) {
             dataSymbols.insert(symbol);
+          } else {
+            symbols.insert(symbol);
+          }
         }
         continue;
       case 'A':
+      case 'C':
+        // Common symbols have no section, and the COFF symbol table walker
+        // rejects symbols whose section number is not positive.
       case 'I':
       case 'N':
       case 'S':
@@ -477,7 +575,7 @@ static bool DumpFileWithNm(std::string const& nmPath,
       default:
         std::cerr << "Ignoring symbol '" << symbol
                   << "' with unrecognized type '" << sym_type
-                  << "' in object '" << line.substr(0, filename_end) << "'.\n";
+                  << "' in object '" << objectFile << "'.\n";
     }
   }
 

Tests/RunCMake/AutoExportDll/CreateDef.cmake

@@ -0,0 +1,46 @@
+# Run "cmake -E __create_def" on the checked-in object files and compare the
+# generated .def files with the expected ones.  The object files are compiled
+# for Windows but are read on every host, so this checks that the COFF symbol
+# table backend used on Windows hosts and the nm backend used on other
+# hosts export the same symbols.
+#
+# The object files were generated from CreateDef/fixture.cxx with
+#
+#   clang-cl --target=<arch>-pc-windows-msvc /c /GR- /EHs- /Os /Zl -nostdinc \
+#     CreateDef/fixture.cxx /Fo:CreateDef/fixture-<arch>.obj
+#
+# once per architecture.
+
+set(failed "")
+foreach(arch IN ITEMS x86_64 i386 arm64ec)
+  set(objlist "${bin}/${arch}-objects.txt")
+  set(def "${bin}/${arch}.def")
+  file(WRITE "${objlist}" "${src}/CreateDef/fixture-${arch}.obj\n")
+  file(REMOVE "${def}")
+  execute_process(
+    COMMAND ${cmake} -E __create_def "${def}" "${objlist}" ${nm}
+    RESULT_VARIABLE result
+    OUTPUT_VARIABLE output
+    ERROR_VARIABLE output
+    )
+  if(NOT result EQUAL 0)
+    string(APPEND failed
+      "__create_def failed for ${arch}: ${result}\n${output}")
+    continue()
+  endif()
+  file(READ "${def}" actual)
+  file(READ "${src}/CreateDef/fixture-${arch}.def" expect)
+  # Compare ignoring trailing whitespace, which __create_def emits.
+  string(REGEX REPLACE "[ \t\r]+\n" "\n" actual "${actual}")
+  string(REGEX REPLACE "[ \t\r]+\n" "\n" expect "${expect}")
+  if(NOT actual STREQUAL expect)
+    string(APPEND failed
+      "Wrong exports for ${arch}.\n"
+      "Expected:\n${expect}\n"
+      "Actual:\n${actual}\n")
+  endif()
+endforeach()
+
+if(failed)
+  message(FATAL_ERROR "${failed}")
+endif()

Tests/RunCMake/AutoExportDll/CreateDef/fixture-arm64ec.def

@@ -0,0 +1,13 @@
+EXPORTS
+	??_7Virt@@6B@ 	 DATA
+	?bssData@@3HA 	 DATA
+	?writableData@@3HA 	 DATA
+	cWritableData 	 DATA
+	#CFunction
+	??0Virt@@$$hQEAA@XZ
+	??1Virt@@$$hUEAA@XZ
+	?CodeFunction@@$$hYAHXZ
+	?DeleteOne@@$$hYAXPEAVVirt@@@Z
+	?Method@Virt@@$$hUEAAHXZ
+	?UseConstData@@$$hYAPEBHXZ
+	?UseLocalFunction@@$$hYAHXZ

Tests/RunCMake/AutoExportDll/CreateDef/fixture-i386.def

@@ -0,0 +1,14 @@
+EXPORTS
+	?bssData@@3HA 	 DATA
+	?writableData@@3HA 	 DATA
+	cWritableData 	 DATA
+	??0Virt@@QAE@XZ
+	??1Virt@@UAE@XZ
+	??_7Virt@@6B@
+	?CodeFunction@@YAHXZ
+	?DeleteOne@@YAXPAVVirt@@@Z
+	?Method@Virt@@UAEHXZ
+	?UseConstData@@YAPBHXZ
+	?UseLocalFunction@@YAHXZ
+	CFunction
+	StdcallFunction

Tests/RunCMake/AutoExportDll/CreateDef/fixture-x86_64.def

@@ -0,0 +1,13 @@
+EXPORTS
+	?bssData@@3HA 	 DATA
+	?writableData@@3HA 	 DATA
+	cWritableData 	 DATA
+	??0Virt@@QEAA@XZ
+	??1Virt@@UEAA@XZ
+	??_7Virt@@6B@
+	?CodeFunction@@YAHXZ
+	?DeleteOne@@YAXPEAVVirt@@@Z
+	?Method@Virt@@UEAAHXZ
+	?UseConstData@@YAPEBHXZ
+	?UseLocalFunction@@YAHXZ
+	CFunction

Tests/RunCMake/AutoExportDll/CreateDef/fixture.cxx

@@ -0,0 +1,64 @@
+/* Distributed under the OSI-approved BSD 3-Clause License.  See accompanying
+   file LICENSE.rst or https://cmake.org/licensing for details.  */
+
+/* Source of the checked-in object files used by the CreateDef test.
+   See CreateDef.cmake for the commands used to regenerate them.  */
+
+class Virt
+{
+public:
+  Virt();
+  virtual ~Virt();
+  virtual int Method();
+};
+Virt::Virt()
+{
+}
+Virt::~Virt()
+{
+}
+int Virt::Method()
+{
+  return 1;
+}
+/* Makes the compiler emit the scalar deleting destructor "??_G".  */
+void DeleteOne(Virt* v)
+{
+  delete v;
+}
+
+int writableData = 1;
+int bssData;
+extern int const constData;
+int const constData = 2;
+int const* UseConstData()
+{
+  return &constData;
+}
+
+int CodeFunction()
+{
+  return 3;
+}
+static int LocalFunction()
+{
+  return 4;
+}
+int UseLocalFunction()
+{
+  return LocalFunction();
+}
+
+/* On i386 these lose their leading underscore, and __stdcall loses its
+   "@<n>" suffix.  */
+extern "C" int CFunction(int a)
+{
+  return a;
+}
+extern "C" int cWritableData = 5;
+#ifdef _M_IX86
+extern "C" int __stdcall StdcallFunction(int a)
+{
+  return a;
+}
+#endif

Tests/RunCMake/AutoExportDll/RunCMakeTest.cmake

@@ -66,6 +66,38 @@ if (RunCMake_GENERATOR MATCHES "(Ninja|Makefiles|Visual Studio)")
   run_cmake_AutoExport(AutoExportShort ".o/0cb3d702")
 endif ()
 
+# Verify that the two bindexplib backends, the COFF symbol table walker used
+# on Windows hosts and the nm reader used on other hosts, export the same
+# symbols from a fixed set of object files.
+function(run_create_def)
+  set(nm_arg "")
+  if(NOT CMAKE_HOST_WIN32)
+    # Reading COFF object files on other hosts requires llvm-nm.
+    find_program(LLVM_NM NAMES llvm-nm)
+    if(NOT LLVM_NM)
+      return()
+    endif()
+    # llvm-nm 16 is the first version that reads ARM64EC object files.
+    execute_process(COMMAND "${LLVM_NM}" --version
+      OUTPUT_VARIABLE nm_version ERROR_QUIET)
+    if(NOT nm_version MATCHES "LLVM version ([0-9]+)" OR CMAKE_MATCH_1 LESS 16)
+      return()
+    endif()
+    set(nm_arg "--nm=${LLVM_NM}")
+  endif()
+  set(RunCMake_TEST_BINARY_DIR "${RunCMake_BINARY_DIR}/CreateDef-build")
+  file(REMOVE_RECURSE "${RunCMake_TEST_BINARY_DIR}")
+  file(MAKE_DIRECTORY "${RunCMake_TEST_BINARY_DIR}")
+  run_cmake_command(CreateDef ${CMAKE_COMMAND}
+    "-Dcmake=${CMAKE_COMMAND}"
+    "-Dnm=${nm_arg}"
+    "-Dsrc=${RunCMake_SOURCE_DIR}"
+    "-Dbin=${RunCMake_TEST_BINARY_DIR}"
+    -P "${RunCMake_SOURCE_DIR}/CreateDef.cmake"
+    )
+endfunction()
+run_create_def()
+
 function(run_AIXExportExplicit)
   set(RunCMake_TEST_BINARY_DIR "${RunCMake_BINARY_DIR}/AIXExportExplicit-build")
   run_cmake(AIXExportExplicit)

Tests/RunCMake/CommandLine/E___create_def-nm-symbol-types-expected.def

@@ -1,6 +1,5 @@
 EXPORTS 
-	??_7Example@@6B@ 	 DATA
 	bss 	 DATA
-	common 	 DATA
 	data 	 DATA
+	??_7Example@@6B@
 	text