From 40cbc25b9a406becc3fbf7040eb375b91949ebe9 Mon Sep 17 00:00:00 2001 From: Sean McGrail <549813+skmcgrail@users.noreply.github.com> Date: Thu, 21 Dec 2023 14:33:23 -0800 Subject: [PATCH] Modify prefixing behavior on aarch64 to allow delocator to handle OPENSSL_armcap_P correctly (#1342) --- crypto/fipsmodule/CMakeLists.txt | 62 +++++++---------- util/fipstools/delocate/delocate.go | 66 +++++++++++++------ util/fipstools/delocate/delocate_test.go | 40 +++++------ .../delocate/testdata/generic-Includes/in.s | 6 ++ .../delocate/testdata/generic-Includes/out.s | 56 ++++++++++++++++ util/make_prefix_headers.go | 49 ++++++++++---- 6 files changed, 190 insertions(+), 89 deletions(-) create mode 100644 util/fipstools/delocate/testdata/generic-Includes/in.s create mode 100644 util/fipstools/delocate/testdata/generic-Includes/out.s diff --git a/crypto/fipsmodule/CMakeLists.txt b/crypto/fipsmodule/CMakeLists.txt index df24a6b643..221d5f0a03 100644 --- a/crypto/fipsmodule/CMakeLists.txt +++ b/crypto/fipsmodule/CMakeLists.txt @@ -270,29 +270,6 @@ if((((ARCH STREQUAL "x86_64") AND NOT MY_ASSEMBLER_IS_TOO_OLD_FOR_AVX) OR endif() endif() -function(cpreprocess dest src) - set(TARGET "") - if(CMAKE_ASM_COMPILER_TARGET) - set(TARGET "--target=${CMAKE_ASM_COMPILER_TARGET}") - endif() - - if(BORINGSSL_PREFIX) - set(PREFIX_INCLUDE "-I${PROJECT_BINARY_DIR}/symbol_prefix_include") - endif() - - string(REGEX REPLACE "[ ]+" ";" CMAKE_ASM_FLAGS "${CMAKE_ASM_FLAGS}") - add_custom_command( - OUTPUT ${dest} - COMMAND ${CMAKE_ASM_COMPILER} ${TARGET} ${CMAKE_ASM_FLAGS} -E ${src} ${PREFIX_INCLUDE} -I${PROJECT_SOURCE_DIR}/include > ${dest} - DEPENDS - ${src} - ${PROJECT_SOURCE_DIR}/include/openssl/arm_arch.h - ${PROJECT_SOURCE_DIR}/include/openssl/asm_base.h - ${PROJECT_SOURCE_DIR}/include/openssl/target.h - WORKING_DIRECTORY . - ) -endfunction() - function(s2n_asm_cpreprocess dest src) # s2n_asm_cpreprocess differs from cpreprocess in that is does additional post-processing # based on s2n-bignum https://github.com/awslabs/s2n-bignum/blob/main/x86/Makefile#L264 @@ -347,29 +324,38 @@ if(FIPS_DELOCATE) ) target_compile_definitions(bcm_c_generated_asm PRIVATE BORINGSSL_IMPLEMENTATION) - if(ARCH STREQUAL "aarch64") - # Perlasm output on Aarch64 needs to pass through the C preprocessor - # before it can be parsed by delocate. - foreach(asm ${BCM_ASM_SOURCES}) - cpreprocess(${asm}.s ${asm}) - list(APPEND BCM_ASM_PROCESSED_SOURCES "${asm}.s") - endforeach() - else() - # No preprocessing is required on other platforms. - set(BCM_ASM_PROCESSED_SOURCES ${BCM_ASM_SOURCES}) - endif() - add_dependencies(bcm_c_generated_asm boringssl_prefix_symbols) - target_include_directories(bcm_c_generated_asm BEFORE PRIVATE ${PROJECT_BINARY_DIR}/symbol_prefix_include) + # Important: We do not want to add the generated prefix symbols to the include path here! + # Delocator expects symbols to not be prefixed. target_include_directories(bcm_c_generated_asm PRIVATE ${PROJECT_SOURCE_DIR}/include) set_target_properties(bcm_c_generated_asm PROPERTIES COMPILE_OPTIONS "-S") set_target_properties(bcm_c_generated_asm PROPERTIES POSITION_INDEPENDENT_CODE ON) + set(TARGET "") + if(CMAKE_ASM_COMPILER_TARGET) + set(TARGET "--target=${CMAKE_ASM_COMPILER_TARGET}") + endif() + go_executable(delocate boringssl.googlesource.com/boringssl/util/fipstools/delocate) add_custom_command( OUTPUT bcm-delocated.S - COMMAND ./delocate -a $ -o bcm-delocated.S ${BCM_ASM_PROCESSED_SOURCES} - DEPENDS bcm_c_generated_asm delocate ${BCM_ASM_PROCESSED_SOURCES} + COMMAND + ./delocate + -a $ + -o bcm-delocated.S + -cc ${CMAKE_ASM_COMPILER} + -cc-flags "${TARGET} ${CMAKE_ASM_FLAGS}" + ${PROJECT_SOURCE_DIR}/include/openssl/arm_arch.h + ${PROJECT_SOURCE_DIR}/include/openssl/asm_base.h + ${PROJECT_SOURCE_DIR}/include/openssl/target.h + ${BCM_ASM_SOURCES} + DEPENDS + bcm_c_generated_asm + delocate + ${BCM_ASM_SOURCES} + ${PROJECT_SOURCE_DIR}/include/openssl/arm_arch.h + ${PROJECT_SOURCE_DIR}/include/openssl/asm_base.h + ${PROJECT_SOURCE_DIR}/include/openssl/target.h WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} ) diff --git a/util/fipstools/delocate/delocate.go b/util/fipstools/delocate/delocate.go index b33ff266df..f8573e3055 100644 --- a/util/fipstools/delocate/delocate.go +++ b/util/fipstools/delocate/delocate.go @@ -61,7 +61,7 @@ const ( // represents a unique symbol for an occurrence of OPENSSL_ia32cap_P. type cpuCapUniqueSymbol struct { - registerName string + registerName string suffixUniqueness string } @@ -78,8 +78,8 @@ func (uniqueSymbol cpuCapUniqueSymbol) getx86SymbolReturn() string { // uniqueness must be a globally unique integer value. func newCpuCapUniqueSymbol(uniqueness int, registerName string) *cpuCapUniqueSymbol { return &cpuCapUniqueSymbol{ - registerName: strings.Trim(registerName, "%"), // should work with both AT&T and Intel syntax. - suffixUniqueness: strconv.Itoa(uniqueness), + registerName: strings.Trim(registerName, "%"), // should work with both AT&T and Intel syntax. + suffixUniqueness: strconv.Itoa(uniqueness), } } @@ -1727,7 +1727,7 @@ func writeAarch64Function(w stringWriter, funcName string, writeContents func(st w.WriteString(".size " + funcName + ", .-" + funcName + "\n") } -func transform(w stringWriter, inputs []inputFile) error { +func transform(w stringWriter, includes []string, inputs []inputFile) error { // symbols contains all defined symbols. symbols := make(map[string]struct{}) // localEntrySymbols contains all symbols with a .localentry directive. @@ -1745,6 +1745,14 @@ func transform(w stringWriter, inputs []inputFile) error { // OPENSSL_ia32cap_get will be synthesized by this script. symbols["OPENSSL_ia32cap_get"] = struct{}{} + for _, include := range includes { + relative, err := relativeHeaderIncludePath(include) + if err != nil { + return err + } + w.WriteString(fmt.Sprintf("#include <%s>\n", relative)) + } + for _, input := range inputs { forEachPath(input.ast.up, func(node *node32) { symbol := input.contents[node.begin:node.end] @@ -1820,18 +1828,18 @@ func transform(w stringWriter, inputs []inputFile) error { } d := &delocation{ - symbols: symbols, - localEntrySymbols: localEntrySymbols, - processor: processor, - commentIndicator: commentIndicator, - output: w, - cpuCapUniqueSymbols: []*cpuCapUniqueSymbol{}, - redirectors: make(map[string]string), - bssAccessorsNeeded: make(map[string]string), - tocLoaders: make(map[string]struct{}), - gotExternalsNeeded: make(map[string]struct{}), - gotOffsetsNeeded: make(map[string]struct{}), - gotOffOffsetsNeeded: make(map[string]struct{}), + symbols: symbols, + localEntrySymbols: localEntrySymbols, + processor: processor, + commentIndicator: commentIndicator, + output: w, + cpuCapUniqueSymbols: []*cpuCapUniqueSymbol{}, + redirectors: make(map[string]string), + bssAccessorsNeeded: make(map[string]string), + tocLoaders: make(map[string]struct{}), + gotExternalsNeeded: make(map[string]struct{}), + gotOffsetsNeeded: make(map[string]struct{}), + gotOffOffsetsNeeded: make(map[string]struct{}), } w.WriteString(".text\n") @@ -2123,6 +2131,21 @@ func includePathFromHeaderFilePath(path string) (string, error) { return "", fmt.Errorf("failed to find 'openssl' path element in header file path %q", path) } +// relativeHeaderIncludePath returns the relative header path for usage in #include statements. +func relativeHeaderIncludePath(path string) (string, error) { + dir, err := includePathFromHeaderFilePath(path) + if err != nil { + return "", err + } + + relative, err := filepath.Rel(dir, path) + if err != nil { + return "", err + } + + return relative, nil +} + func main() { // The .a file, if given, is expected to be an archive of textual // assembly sources. That's odd, but CMake really wants to create @@ -2148,6 +2171,7 @@ func main() { }) } + var includes []string includePaths := make(map[string]struct{}) for i, path := range flag.Args() { @@ -2163,6 +2187,7 @@ func main() { fmt.Fprintf(os.Stderr, "%s\n", err) os.Exit(1) } + includes = append(includes, path) includePaths[dir] = struct{}{} continue } @@ -2188,6 +2213,9 @@ func main() { // -E requests only preprocessing. cppCommand = append(cppCommand, "-E") + + // Output ‘#include’ directives in addition to the result of preprocessing. + cppCommand = append(cppCommand, "-dI") } if err := parseInputs(inputs, cppCommand); err != nil { @@ -2201,7 +2229,7 @@ func main() { } defer out.Close() - if err := transform(out, inputs); err != nil { + if err := transform(out, includes, inputs); err != nil { fmt.Fprintf(os.Stderr, "%s\n", err) os.Exit(1) } @@ -2281,7 +2309,7 @@ func isSynthesized(symbol string, processor processorType) bool { // While BORINGSSL_bcm_text_[start,end] are known symbols, on aarch64 we go // through the GOT because adr doesn't have adequate reach. - if (processor != aarch64) { + if processor != aarch64 { SymbolisSynthesized = SymbolisSynthesized || strings.HasPrefix(symbol, "BORINGSSL_bcm_text_") } @@ -2289,7 +2317,7 @@ func isSynthesized(symbol string, processor processorType) bool { } func isFipsScopeMarkers(symbol string) bool { - return symbol == "BORINGSSL_bcm_text_start" || + return symbol == "BORINGSSL_bcm_text_start" || symbol == "BORINGSSL_bcm_text_end" } diff --git a/util/fipstools/delocate/delocate_test.go b/util/fipstools/delocate/delocate_test.go index b9d514eac1..04290cb823 100644 --- a/util/fipstools/delocate/delocate_test.go +++ b/util/fipstools/delocate/delocate_test.go @@ -28,9 +28,10 @@ var ( ) type delocateTest struct { - name string - in []string - out string + name string + includes []string + inputs []string + out string } func (test *delocateTest) Path(file string) string { @@ -38,27 +39,28 @@ func (test *delocateTest) Path(file string) string { } var delocateTests = []delocateTest{ - {"generic-FileDirectives", []string{"in.s"}, "out.s"}, - {"ppc64le-GlobalEntry", []string{"in.s"}, "out.s"}, - {"ppc64le-LoadToR0", []string{"in.s"}, "out.s"}, - {"ppc64le-Sample2", []string{"in.s"}, "out.s"}, - {"ppc64le-Sample", []string{"in.s"}, "out.s"}, - {"ppc64le-TOCWithOffset", []string{"in.s"}, "out.s"}, - {"x86_64-Basic", []string{"in.s"}, "out.s"}, - {"x86_64-BSS", []string{"in.s"}, "out.s"}, - {"x86_64-GOTRewrite", []string{"in.s"}, "out.s"}, - {"x86_64-LargeMemory", []string{"in.s"}, "out.s"}, - {"x86_64-LabelRewrite", []string{"in1.s", "in2.s"}, "out.s"}, - {"x86_64-Sections", []string{"in.s"}, "out.s"}, - {"x86_64-ThreeArg", []string{"in.s"}, "out.s"}, - {"aarch64-Basic", []string{"in.s"}, "out.s"}, + {"generic-FileDirectives", nil, []string{"in.s"}, "out.s"}, + {"generic-Includes", []string{"/some/include/path/openssl/foo.h", "/some/include/path/openssl/bar.h"}, []string{"in.s"}, "out.s"}, + {"ppc64le-GlobalEntry", nil, []string{"in.s"}, "out.s"}, + {"ppc64le-LoadToR0", nil, []string{"in.s"}, "out.s"}, + {"ppc64le-Sample2", nil, []string{"in.s"}, "out.s"}, + {"ppc64le-Sample", nil, []string{"in.s"}, "out.s"}, + {"ppc64le-TOCWithOffset", nil, []string{"in.s"}, "out.s"}, + {"x86_64-Basic", nil, []string{"in.s"}, "out.s"}, + {"x86_64-BSS", nil, []string{"in.s"}, "out.s"}, + {"x86_64-GOTRewrite", nil, []string{"in.s"}, "out.s"}, + {"x86_64-LargeMemory", nil, []string{"in.s"}, "out.s"}, + {"x86_64-LabelRewrite", nil, []string{"in1.s", "in2.s"}, "out.s"}, + {"x86_64-Sections", nil, []string{"in.s"}, "out.s"}, + {"x86_64-ThreeArg", nil, []string{"in.s"}, "out.s"}, + {"aarch64-Basic", nil, []string{"in.s"}, "out.s"}, } func TestDelocate(t *testing.T) { for _, test := range delocateTests { t.Run(test.name, func(t *testing.T) { var inputs []inputFile - for i, in := range test.in { + for i, in := range test.inputs { inputs = append(inputs, inputFile{ index: i, path: test.Path(in), @@ -70,7 +72,7 @@ func TestDelocate(t *testing.T) { } var buf bytes.Buffer - if err := transform(&buf, inputs); err != nil { + if err := transform(&buf, test.includes, inputs); err != nil { t.Fatalf("transform failed: %s", err) } diff --git a/util/fipstools/delocate/testdata/generic-Includes/in.s b/util/fipstools/delocate/testdata/generic-Includes/in.s new file mode 100644 index 0000000000..ca5f8c291e --- /dev/null +++ b/util/fipstools/delocate/testdata/generic-Includes/in.s @@ -0,0 +1,6 @@ +.file 10 "some/path/file.c" "file.c" +.file 1000 "some/path/file2.c" "file2.c" +.file 1001 "some/path/file_with_md5.c" "other_name.c" md5 0x5eba7844df6449a7f2fff1556fe7ba8d239f8e2f + +# An instruction is needed to satisfy the architecture auto-detection. + movq %rax, %rbx diff --git a/util/fipstools/delocate/testdata/generic-Includes/out.s b/util/fipstools/delocate/testdata/generic-Includes/out.s new file mode 100644 index 0000000000..c835dfe734 --- /dev/null +++ b/util/fipstools/delocate/testdata/generic-Includes/out.s @@ -0,0 +1,56 @@ +#include +#include +.text +.file 1002 "inserted_by_delocate.c" md5 0x00000000000000000000000000000000 +.loc 1002 1 0 +BORINGSSL_bcm_text_start: +.file 10 "some/path/file.c" "file.c" +.file 1000 "some/path/file2.c" "file2.c" +.file 1001 "some/path/file_with_md5.c" "other_name.c" md5 0x5eba7844df6449a7f2fff1556fe7ba8d239f8e2f + +# An instruction is needed to satisfy the architecture auto-detection. + movq %rax, %rbx +.text +.loc 1002 2 0 +BORINGSSL_bcm_text_end: +.type OPENSSL_ia32cap_get, @function +.globl OPENSSL_ia32cap_get +.LOPENSSL_ia32cap_get_local_target: +OPENSSL_ia32cap_get: + leaq OPENSSL_ia32cap_P(%rip), %rax + ret +.type BORINGSSL_bcm_text_hash, @object +.size BORINGSSL_bcm_text_hash, 32 +BORINGSSL_bcm_text_hash: +.byte 0xae +.byte 0x2c +.byte 0xea +.byte 0x2a +.byte 0xbd +.byte 0xa6 +.byte 0xf3 +.byte 0xec +.byte 0x97 +.byte 0x7f +.byte 0x9b +.byte 0xf6 +.byte 0x94 +.byte 0x9a +.byte 0xfc +.byte 0x83 +.byte 0x68 +.byte 0x27 +.byte 0xcb +.byte 0xa0 +.byte 0xa0 +.byte 0x9f +.byte 0x6b +.byte 0x6f +.byte 0xde +.byte 0x52 +.byte 0xcd +.byte 0xe2 +.byte 0xcd +.byte 0xff +.byte 0x31 +.byte 0x80 diff --git a/util/make_prefix_headers.go b/util/make_prefix_headers.go index 9ed674e5e4..9b2c4cf2b1 100644 --- a/util/make_prefix_headers.go +++ b/util/make_prefix_headers.go @@ -93,9 +93,14 @@ func writeCHeader(symbols []string, path string) error { } if _, err := fmt.Fprintf(f, ` +#ifndef BORINGSSL_PREFIX_SYMBOLS_H + +#define BORINGSSL_PREFIX_SYMBOLS_H + #ifndef BORINGSSL_PREFIX #define BORINGSSL_PREFIX %s -#endif +#endif // BORINGSSL_PREFIX + `, *prefix); err != nil { return err } @@ -116,6 +121,10 @@ func writeCHeader(symbols []string, path string) error { } } + if _, err := f.WriteString("\n#endif // BORINGSSL_PREFIX_SYMBOLS_H\n"); err != nil { + return err + } + return nil } @@ -144,17 +153,17 @@ func writeASMHeader(symbols []string, path string) error { } if _, err := fmt.Fprintf(f, ` -#ifndef BORINGSSL_PREFIX -#define BORINGSSL_PREFIX %s -#endif - `, *prefix); err != nil { - return err - } - - if _, err := f.WriteString(` #if !defined(__APPLE__) #include #else +#ifndef BORINGSSL_PREFIX_SYMBOLS_ASM_H + +#define BORINGSSL_PREFIX_SYMBOLS_ASM_H + +#ifndef BORINGSSL_PREFIX +#define BORINGSSL_PREFIX %s +#endif // BORINGSSL_PREFIX + // On iOS and macOS, we need to treat assembly symbols differently from other // symbols. The linker expects symbols to be prefixed with an underscore. // Perlasm thus generates symbol with this underscore applied. Our macros must, @@ -162,7 +171,7 @@ func writeASMHeader(symbols []string, path string) error { #define BORINGSSL_ADD_PREFIX_MAC_ASM(a, b) BORINGSSL_ADD_PREFIX_INNER_MAC_ASM(a, b) #define BORINGSSL_ADD_PREFIX_INNER_MAC_ASM(a, b) _ ## a ## _ ## b -`); err != nil { +`, *prefix); err != nil { return err } @@ -172,7 +181,13 @@ func writeASMHeader(symbols []string, path string) error { } } - _, err = fmt.Fprintf(f, "#endif\n") + if _, err := f.WriteString(` +#endif // BORINGSSL_PREFIX_SYMBOLS_ASM_H +#endif // !defined(__APPLE__) +`); err != nil { + return nil + } + return nil } @@ -202,9 +217,13 @@ func writeNASMHeader(symbols []string, path string) error { } if _, err := fmt.Fprintf(f, ` +%%ifndef BORINGSSL_PREFIX_SYMBOLS_NASM_INC + +%%define BORINGSSL_PREFIX_SYMBOLS_NASM_INC + %%ifndef BORINGSSL_PREFIX %%define BORINGSSL_PREFIX %s -%%endif +%%endif ; BORINGSSL_PREFIX `, *prefix); err != nil { return err } @@ -212,6 +231,7 @@ func writeNASMHeader(symbols []string, path string) error { if _, err := f.WriteString(` ; 32-bit Windows adds underscores to C functions, while 64-bit Windows does not. %ifidn __OUTPUT_FORMAT__, win32 + `); err != nil { return err } @@ -232,7 +252,10 @@ func writeNASMHeader(symbols []string, path string) error { } } - if _, err := fmt.Fprintf(f, "%%endif\n"); err != nil { + if _, err := fmt.Fprintf(f, ` +%%endif ; __OUTPUT_FORMAT__ +%%endif ; BORINGSSL_PREFIX_SYMBOLS_NASM_INC +`); err != nil { return err }