From 8c23d3a5dfc72c5cb60c90d923a7acdfa791ff50 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sun, 25 Nov 2018 15:58:02 -0600 Subject: [PATCH] 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 Reviewed-by: Adam Langley --- CMakeLists.txt | 3 ++ crypto/CMakeLists.txt | 2 +- crypto/thread_win.c | 14 ++++-- util/make_prefix_headers.go | 18 +++++++- util/read_symbols.go | 89 ++++++++++++++++++++++++++++++++++--- 5 files changed, 115 insertions(+), 11 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index dd2d9378..bfde5d58 100644 --- a/CMakeLists.txt +++ b/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) diff --git a/crypto/CMakeLists.txt b/crypto/CMakeLists.txt index ee9626a1..b1ca70e1 100644 --- a/crypto/CMakeLists.txt +++ b/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) diff --git a/crypto/thread_win.c b/crypto/thread_win.c index 8b2b2da5..45011650 100644 --- a/crypto/thread_win.c +++ b/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:p_thread_callback_boringssl") +__pragma(comment(linker, "/INCLUDE:_tls_used")) +__pragma(comment( + linker, "/INCLUDE:" EXPAND_AND_STRINGIFY(p_thread_callback_boringssl))) #else -#pragma comment(linker, "/INCLUDE:__tls_used") -#pragma comment(linker, "/INCLUDE:_p_thread_callback_boringssl") +__pragma(comment(linker, "/INCLUDE:__tls_used")) +__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 diff --git a/util/make_prefix_headers.go b/util/make_prefix_headers.go index a5e5441f..b536f14c 100644 --- a/util/make_prefix_headers.go +++ b/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 } diff --git a/util/read_symbols.go b/util/read_symbols.go index fc3e0693..d6d5a9ae 100644 --- a/util/read_symbols.go +++ b/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. - if !strings.HasPrefix(s, "_Z") { - if _, err := fmt.Fprintln(out, s); err != nil { - printAndExit("Error writing to %s: %s", *outFlag, err) + var skipSymbols = []string{ + // Inline functions, etc., from the compiler or language + // runtime will naturally end up in the library, to be + // 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 +}