Make Windows symbol-prefixing work.

This teaches read_symbols.go to use debug/pe, and fixes miscellaneous
issues with NASM. It also reveals a problem with this strategy of
getting symbols out at the linker level: inline functions.  I'm thinking
a better long-term mechanism may be to parse our header files.

Change-Id: I11b008543a7a97db3db9d4062ee4ddb910d174b7
Reviewed-on: https://boringssl-review.googlesource.com/c/33349
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>

CMakeLists.txt

@@ -53,6 +53,9 @@ endif()
 
 if(BORINGSSL_PREFIX AND BORINGSSL_PREFIX_SYMBOLS)
   add_definitions(-DBORINGSSL_PREFIX=${BORINGSSL_PREFIX})
+  # CMake automatically connects include_directories to the NASM command-line,
+  # but not add_definitions.
+  set(CMAKE_ASM_NASM_FLAGS "${CMAKE_ASM_NASM_FLAGS} -DBORINGSSL_PREFIX=${BORINGSSL_PREFIX}")
 
   # Use "symbol_prefix_include" to store generated header files
   include_directories(${CMAKE_CURRENT_BINARY_DIR}/symbol_prefix_include)

crypto/CMakeLists.txt

@@ -53,7 +53,7 @@ if(NOT OPENSSL_NO_ASM)
       set(PERLASM_STYLE win32n)
       set(PERLASM_FLAGS "-DOPENSSL_IA32_SSE2")
     endif()
-    set(CMAKE_ASM_NASM_FLAGS "-gcv8")
+    set(CMAKE_ASM_NASM_FLAGS "${CMAKE_ASM_NASM_FLAGS} -gcv8")
 
     # On Windows, we use the NASM output, specifically built with Yasm.
     set(ASM_EXT asm)

crypto/thread_win.c

@@ -146,12 +146,18 @@ static void NTAPI thread_local_destructor(PVOID module, DWORD reason,
 // if it's not already there. (E.g. if __declspec(thread) is not used). Force
 // a reference to p_thread_callback_boringssl to prevent whole program
 // optimization from discarding the variable.
+//
+// Note, in the prefixed build, |p_thread_callback_boringssl| may be a macro.
+#define STRINGIFY(x) #x
+#define EXPAND_AND_STRINGIFY(x) STRINGIFY(x)
 #ifdef _WIN64
-#pragma comment(linker, "/INCLUDE:_tls_used")
+__pragma(comment(linker, "/INCLUDE:_tls_used"))
-#pragma comment(linker, "/INCLUDE:p_thread_callback_boringssl")
+__pragma(comment(
+    linker, "/INCLUDE:" EXPAND_AND_STRINGIFY(p_thread_callback_boringssl)))
 #else
-#pragma comment(linker, "/INCLUDE:__tls_used")
+__pragma(comment(linker, "/INCLUDE:__tls_used"))
-#pragma comment(linker, "/INCLUDE:_p_thread_callback_boringssl")
+__pragma(comment(
+    linker, "/INCLUDE:_" EXPAND_AND_STRINGIFY(p_thread_callback_boringssl)))
 #endif
 
 // .CRT$XLA to .CRT$XLZ is an array of PIMAGE_TLS_CALLBACK pointers that are

util/make_prefix_headers.go

@@ -172,16 +172,32 @@ func writeNASMHeader(symbols []string, path string) error {
 ; OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
 ; CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
+; 32-bit Windows adds underscores to C functions, while 64-bit Windows does not.
+%ifidn __OUTPUT_FORMAT__, win32
 `); err != nil {
 		return err
 	}
 
 	for _, symbol := range symbols {
-		if _, err := fmt.Fprintf(f, "%%define %s BORINGSSL_PREFIX %%+ %s\n", symbol, symbol); err != nil {
+		if _, err := fmt.Fprintf(f, "%%xdefine _%s _ %%+ BORINGSSL_PREFIX %%+ _%s\n", symbol, symbol); err != nil {
 			return err
 		}
 	}
 
+	if _, err := fmt.Fprintf(f, "%%else\n"); err != nil {
+		return err
+	}
+
+	for _, symbol := range symbols {
+		if _, err := fmt.Fprintf(f, "%%xdefine %s BORINGSSL_PREFIX %%+ _%s\n", symbol, symbol); err != nil {
+			return err
+		}
+	}
+
+	if _, err := fmt.Fprintf(f, "%%endif\n"); err != nil {
+		return err
+	}
+
 	return nil
 }
 

util/read_symbols.go

@@ -20,6 +20,7 @@ import (
 	"bytes"
 	"debug/elf"
 	"debug/macho"
+	"debug/pe"
 	"flag"
 	"fmt"
 	"os"
@@ -33,11 +34,12 @@ import (
 const (
 	ObjFileFormatELF   = "elf"
 	ObjFileFormatMachO = "macho"
+	ObjFileFormatPE    = "pe"
 )
 
 var (
 	outFlag       = flag.String("out", "-", "File to write output symbols")
-	objFileFormat = flag.String("obj-file-format", defaultObjFileFormat(runtime.GOOS), "Object file format to expect (options are elf, macho)")
+	objFileFormat = flag.String("obj-file-format", defaultObjFileFormat(runtime.GOOS), "Object file format to expect (options are elf, macho, pe)")
 )
 
 func defaultObjFileFormat(goos string) string {
@@ -46,6 +48,8 @@ func defaultObjFileFormat(goos string) string {
 		return ObjFileFormatELF
 	case "darwin":
 		return ObjFileFormatMachO
+	case "windows":
+		return ObjFileFormatPE
 	default:
 		// By returning a value here rather than panicking, the user can still
 		// cross-compile from an unsupported platform to a supported platform by
@@ -105,17 +109,53 @@ func main() {
 			}
 		}
 	}
+
 	sort.Strings(symbols)
 	for _, s := range symbols {
-		// Filter out C++ mangled names.
+		var skipSymbols = []string{
-		if !strings.HasPrefix(s, "_Z") {
+			// Inline functions, etc., from the compiler or language
-			if _, err := fmt.Fprintln(out, s); err != nil {
+			// runtime will naturally end up in the library, to be
-				printAndExit("Error writing to %s: %s", *outFlag, err)
+			// deduplicated against other object files. Such symbols
+			// should not be prefixed. It is a limitation of this
+			// symbol-prefixing strategy that we cannot distinguish
+			// our own inline symbols (which should be prefixed)
+			// from the system's (which should not), so we blacklist
+			// known system symbols.
+			"__local_stdio_printf_options",
+			"__local_stdio_scanf_options",
+			"_vscprintf",
+			"_vscprintf_l",
+			"_vsscanf_l",
+			"_xmm",
+			"sscanf",
+			"vsnprintf",
+			// sdallocx is a weak symbol and intended to merge with
+			// the real one, if present.
+			"sdallocx",
+		}
+		var skip bool
+		for _, sym := range skipSymbols {
+			if sym == s {
+				skip = true
+				break
 			}
 		}
+		if skip || isCXXSymbol(s) || strings.HasPrefix(s, "__real@") {
+			continue
+		}
+		if _, err := fmt.Fprintln(out, s); err != nil {
+			printAndExit("Error writing to %s: %s", *outFlag, err)
+		}
 	}
 }
 
+func isCXXSymbol(s string) bool {
+	if *objFileFormat == ObjFileFormatPE {
+		return strings.HasPrefix(s, "?")
+	}
+	return strings.HasPrefix(s, "_Z")
+}
+
 // listSymbols lists the exported symbols from an object file.
 func listSymbols(contents []byte) ([]string, error) {
 	switch *objFileFormat {
@@ -123,6 +163,8 @@ func listSymbols(contents []byte) ([]string, error) {
 		return listSymbolsELF(contents)
 	case ObjFileFormatMachO:
 		return listSymbolsMachO(contents)
+	case ObjFileFormatPE:
+		return listSymbolsPE(contents)
 	default:
 		return nil, fmt.Errorf("unsupported object file format %q", *objFileFormat)
 	}
@@ -181,3 +223,40 @@ func listSymbolsMachO(contents []byte) ([]string, error) {
 	}
 	return names, nil
 }
+
+func listSymbolsPE(contents []byte) ([]string, error) {
+	f, err := pe.NewFile(bytes.NewReader(contents))
+	if err != nil {
+		return nil, err
+	}
+	var ret []string
+	for _, sym := range f.Symbols {
+		const (
+			// https://docs.microsoft.com/en-us/windows/desktop/debug/pe-format#section-number-values
+			IMAGE_SYM_UNDEFINED = 0
+			// https://docs.microsoft.com/en-us/windows/desktop/debug/pe-format#storage-class
+			IMAGE_SYM_CLASS_EXTERNAL = 2
+		)
+		if sym.SectionNumber != IMAGE_SYM_UNDEFINED && sym.StorageClass == IMAGE_SYM_CLASS_EXTERNAL {
+			name := sym.Name
+			if f.Machine == pe.IMAGE_FILE_MACHINE_I386 {
+				// On 32-bit Windows, C symbols are decorated by calling
+				// convention.
+				// https://msdn.microsoft.com/en-us/library/56h2zst2.aspx#FormatC
+				if strings.HasPrefix(name, "_") || strings.HasPrefix(name, "@") {
+					// __cdecl, __stdcall, or __fastcall. Remove the prefix and
+					// suffix, if present.
+					name = name[1:]
+					if idx := strings.LastIndex(name, "@"); idx >= 0 {
+						name = name[:idx]
+					}
+				} else if idx := strings.LastIndex(name, "@@"); idx >= 0 {
+					// __vectorcall. Remove the suffix.
+					name = name[:idx]
+				}
+			}
+			ret = append(ret, name)
+		}
+	}
+	return ret, nil
+}