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>
google · Feb 28, 2025 · 673e61f · 673e61f
1 parent ad62e9c
commit 673e61f
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 47 deletions.
diff --git 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
@@ -195,7 +195,7 @@ func (d *delocation) processDirective(statement, directive *node32) (*node32, er
 		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 @@ func (d *delocation) processDirective(statement, directive *node32) (*node32, er
 		// 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 @@ func (d *delocation) handleBSS(statement *node32) (*node32, error) {
 				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 @@ func redirectorName(symbol string) string {
 	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
@@ -19,15 +19,26 @@ x:
 
 	# .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
@@ -25,18 +25,33 @@ x:
 
 	# .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_storage_bss_get:
 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