delocate: remove the need to demangle local variables We need to demangle because the backing storage for a delocated BSS symbol is declared static. The compiler seems to be always give it a C++-mangled name. But we relied on those names to infer the names of the bss_get functions that the rest of the code wanted. Instead, if we make the backing storage non-static, we get the name we want. Ideally we would avoid having to extract those names (see https://boringssl-review.googlesource.com/c/boringssl/+/70849), but faking the linker script behavior in delocate seemed tricky, so I opted to do this and then we can finish up the linker script version later. While I'm here, prefix the underlying names with 'bcm_' so we're not squatting as much of the namespace. Change-Id: I1ce9a189e620c872511dbce09d23e655f1593d34 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/76847 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com>
diff --git a/crypto/fipsmodule/delocate.h b/crypto/fipsmodule/delocate.h index 26d3d41..e104d6e 100644 --- a/crypto/fipsmodule/delocate.h +++ b/crypto/fipsmodule/delocate.h
@@ -22,11 +22,18 @@ #if !defined(BORINGSSL_SHARED_LIBRARY) && defined(BORINGSSL_FIPS) && \ !defined(OPENSSL_ASAN) && !defined(OPENSSL_MSAN) -#define DEFINE_BSS_GET(type, name, init_value) \ - static type name __attribute__((used)) = init_value; \ - extern "C" { \ - type *name##_bss_get(void) __attribute__((const)); \ - } +#define DEFINE_BSS_GET(type, name, init_value) \ + /* delocate needs C linkage and for |name| to be unique across BCM. */ \ + extern "C" { \ + extern type bcm_##name; \ + type bcm_##name = init_value; \ + type *bcm_##name##_bss_get(void) __attribute__((const)); \ + } /* extern "C" */ \ + \ + /* The getter functions are exported, but static variables are usually named \ + * with short names. Define a static wrapper function so the caller can use \ + * a short name, while the symbol itself is prefixed. */ \ + static type *name##_bss_get(void) { return bcm_##name##_bss_get(); } // For FIPS builds we require that CRYPTO_ONCE_INIT be zero. #define DEFINE_STATIC_ONCE(name) \ DEFINE_BSS_GET(CRYPTO_once_t, name, CRYPTO_ONCE_INIT)
diff --git a/util/fipstools/delocate/delocate.go b/util/fipstools/delocate/delocate.go index a1a990e..e4b6111 100644 --- a/util/fipstools/delocate/delocate.go +++ b/util/fipstools/delocate/delocate.go
@@ -195,7 +195,7 @@ if len(args) < 1 { return nil, errors.New("comm directive has no arguments") } - d.bssAccessorsNeeded[demangle(args[0])] = args[0] + d.bssAccessorsNeeded[args[0]] = args[0] d.writeNode(statement) case "data": @@ -204,6 +204,10 @@ // will have to work around this in the future. return nil, errors.New(".data section found in module") + case "bss": + d.writeNode(statement) + return d.handleBSS(statement) + case "section": section := args[0] @@ -1259,7 +1263,7 @@ localSymbol := localTargetName(symbol) d.output.WriteString(fmt.Sprintf("\n%s:\n", localSymbol)) - d.bssAccessorsNeeded[demangle(symbol)] = localSymbol + d.bssAccessorsNeeded[symbol] = localSymbol } case ruleLabelContainingDirective: @@ -1746,42 +1750,6 @@ return "bcm_redirector_" + symbol } -// Optionally demangle C++ local variable names. -func demangle(symbol string) string { - if !strings.HasPrefix(symbol, "_Z") { - return symbol - } - - // The names must have the form "_ZL", followed by the length of the name - // in base 10, followed by the name. - if !strings.HasPrefix(symbol, "_ZL") { - panic("malformed symbol: starts with _Z but not _ZL") - } - - if len(symbol) < 4 { - panic("malformed symbol: too short") - } - - pos := 3 - for pos < len(symbol) && '0' <= symbol[pos] && symbol[pos] <= '9' { - pos++ - } - if pos == 3 { - panic("malformed symbol: no length digits") - } - - length, err := strconv.Atoi(symbol[3:pos]) - if err != nil { - panic("malformed symbol: invalid length") - } - - if len(symbol[pos:]) != length { - panic("malformed symbol: length mismatch") - } - - return symbol[pos:] -} - // sectionType returns the type of a section. I.e. a section called “.text.foo” // is a “.text” section. func sectionType(section string) (string, bool) {
diff --git a/util/fipstools/delocate/testdata/x86_64-BSS/in.s b/util/fipstools/delocate/testdata/x86_64-BSS/in.s index 2d31363..1dceb5b 100644 --- a/util/fipstools/delocate/testdata/x86_64-BSS/in.s +++ b/util/fipstools/delocate/testdata/x86_64-BSS/in.s
@@ -19,15 +19,26 @@ # .bss handling is terminated by a .text directive. .text +not_bss1: + ret + + # The .bss directive can introduce BSS. + .bss +test: + .quad 0 + .text +not_bss2: + ret + .section .bss,"awT",@nobits y: .quad 0 - # Or a .section directive. + # A .section directive also terminates BSS. .section .rodata .quad 0 - # Or the end of the file. + # The end of the file terminates BSS. .section .bss,"awT",@nobits z: .quad 0
diff --git a/util/fipstools/delocate/testdata/x86_64-BSS/out.s b/util/fipstools/delocate/testdata/x86_64-BSS/out.s index e768027..4a5be91 100644 --- a/util/fipstools/delocate/testdata/x86_64-BSS/out.s +++ b/util/fipstools/delocate/testdata/x86_64-BSS/out.s
@@ -25,18 +25,33 @@ # .bss handling is terminated by a .text directive. .text +.Lnot_bss1_local_target: +not_bss1: + ret + + # The .bss directive can introduce BSS. + .bss +test: +.Ltest_local_target: + + .quad 0 + .text +.Lnot_bss2_local_target: +not_bss2: + ret + .section .bss,"awT",@nobits y: .Ly_local_target: .quad 0 - # Or a .section directive. + # A .section directive also terminates BSS. # WAS .section .rodata .text .quad 0 - # Or the end of the file. + # The end of the file terminates BSS. .section .bss,"awT",@nobits z: .Lz_local_target: @@ -53,6 +68,10 @@ aes_128_ctr_generic_storage2_bss_get: leaq aes_128_ctr_generic_storage2(%rip), %rax ret +.type test_bss_get, @function +test_bss_get: + leaq .Ltest_local_target(%rip), %rax + ret .type x_bss_get, @function x_bss_get: leaq .Lx_local_target(%rip), %rax