Skip to content

Commit

Permalink
Make the clang-6 bcm-delocated.S directive behavior for start/end sym…
Browse files Browse the repository at this point in the history
…bols dynamic (aws#1456)

* Revert "Fix Clang-6 FIPS static build issue (aws#1424)"

This reverts commit f618701.

* Make start/end BCM symbol directives optional via delocator flag

* CMake compiler detection for Clang ASM is silly
  • Loading branch information
skmcgrail authored Feb 28, 2024
1 parent 35d9d65 commit 3f3f830
Show file tree
Hide file tree
Showing 20 changed files with 164 additions and 29 deletions.
17 changes: 13 additions & 4 deletions crypto/fipsmodule/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ endif()
# clang-6 (and older) knows how to compile AVX512 assembly instructions,
# but only if it's given the right flags (e.g. -mavx512*).
# The flags are not required for any other compiler we are running in the CI.
if ((CMAKE_ASM_COMPILER_ID MATCHES "Clang" OR CMAKE_ASM_COMPILER MATCHES "clang") AND
(CMAKE_ASM_COMPILER_VERSION VERSION_LESS "7.0.0") AND (ARCH STREQUAL "x86_64"))
if (CLANG AND (CMAKE_ASM_COMPILER_ID MATCHES "Clang" OR CMAKE_ASM_COMPILER MATCHES "clang") AND
(CMAKE_C_COMPILER_VERSION VERSION_LESS "7.0.0") AND (ARCH STREQUAL "x86_64"))
set_source_files_properties(${CMAKE_CURRENT_BINARY_DIR}/aesni-gcm-avx512.${ASM_EXT} PROPERTIES COMPILE_FLAGS "-mavx512f -mavx512bw -mavx512dq -mavx512vl")
set_source_files_properties(${CMAKE_CURRENT_BINARY_DIR}/aesni-xts-avx512.${ASM_EXT} PROPERTIES COMPILE_FLAGS "-mavx512f -mavx512bw -mavx512dq -mavx512vl")
endif()
Expand Down Expand Up @@ -336,6 +336,14 @@ if(FIPS_DELOCATE)
set(TARGET "--target=${CMAKE_ASM_COMPILER_TARGET}")
endif()

set(DELOCATE_EXTRA_ARGS "")
# clang-6 (and older) do not appreciate the file number starting at a higher value, and incorrectly
# assume that all file numbers less than that value are defined upon later use.
if (CLANG AND (CMAKE_ASM_COMPILER_ID MATCHES "Clang" OR CMAKE_ASM_COMPILER MATCHES "clang") AND
(CMAKE_C_COMPILER_VERSION VERSION_LESS "7.0.0"))
set(DELOCATE_EXTRA_ARGS "-no-se-debug-directives")
endif()

go_executable(delocate boringssl.googlesource.com/boringssl/util/fipstools/delocate)
add_custom_command(
OUTPUT bcm-delocated.S
Expand All @@ -345,6 +353,7 @@ if(FIPS_DELOCATE)
-o bcm-delocated.S
-cc ${CMAKE_ASM_COMPILER}
-cc-flags "${TARGET} ${CMAKE_ASM_FLAGS}"
${DELOCATE_EXTRA_ARGS}
${PROJECT_SOURCE_DIR}/include/openssl/arm_arch.h
${PROJECT_SOURCE_DIR}/include/openssl/asm_base.h
${PROJECT_SOURCE_DIR}/include/openssl/target.h
Expand All @@ -362,8 +371,8 @@ if(FIPS_DELOCATE)
# clang-6 (and older) knows how to compile AVX512 assembly instructions,
# but only if it's given the right flags (e.g. -mavx512*).
# The flags are not required for any other compiler we are running in the CI.
if ((CMAKE_ASM_COMPILER_ID MATCHES "Clang" OR CMAKE_ASM_COMPILER MATCHES "clang") AND
(CMAKE_ASM_COMPILER_VERSION VERSION_LESS "7.0.0") AND (ARCH STREQUAL "x86_64"))
if (CLANG AND (CMAKE_ASM_COMPILER_ID MATCHES "Clang" OR CMAKE_ASM_COMPILER MATCHES "clang") AND
(CMAKE_C_COMPILER_VERSION VERSION_LESS "7.0.0") AND (ARCH STREQUAL "x86_64"))
set_source_files_properties(${CMAKE_CURRENT_BINARY_DIR}/bcm-delocated.S PROPERTIES COMPILE_FLAGS "-mavx512f -mavx512bw -mavx512dq -mavx512vl")
endif()

Expand Down
32 changes: 27 additions & 5 deletions util/fipstools/delocate/delocate.go
Original file line number Diff line number Diff line change
Expand Up @@ -1766,7 +1766,7 @@ func writeAarch64Function(w stringWriter, funcName string, writeContents func(st
w.WriteString(".size " + funcName + ", .-" + funcName + "\n")
}

func transform(w stringWriter, includes []string, inputs []inputFile) error {
func transform(w stringWriter, includes []string, inputs []inputFile, startEndDebugDirectives bool) error {
// symbols contains all defined symbols.
symbols := make(map[string]struct{})
// localEntrySymbols contains all symbols with a .localentry directive.
Expand All @@ -1776,6 +1776,10 @@ func transform(w stringWriter, includes []string, inputs []inputFile) error {
// maxObservedFileNumber contains the largest seen file number in a
// .file directive. Zero is not a valid number.
maxObservedFileNumber := 0
// fileDirectivesContainMD5 is true if the compiler is outputting MD5
// checksums in .file directives. If it does so, then this script needs
// to match that behaviour otherwise warnings result.
fileDirectivesContainMD5 := false

// OPENSSL_ia32cap_get will be synthesized by this script.
symbols["OPENSSL_ia32cap_get"] = struct{}{}
Expand Down Expand Up @@ -1843,6 +1847,12 @@ func transform(w stringWriter, includes []string, inputs []inputFile) error {
if fileNo > maxObservedFileNumber {
maxObservedFileNumber = fileNo
}

for _, token := range parts[2:] {
if token == "md5" {
fileDirectivesContainMD5 = true
}
}
}, ruleStatement, ruleLocationDirective)
}

Expand Down Expand Up @@ -1872,6 +1882,14 @@ func transform(w stringWriter, includes []string, inputs []inputFile) error {
}

w.WriteString(".text\n")
if startEndDebugDirectives {
var fileTrailing string
if fileDirectivesContainMD5 {
fileTrailing = " md5 0x00000000000000000000000000000000"
}
w.WriteString(fmt.Sprintf(".file %d \"inserted_by_delocate.c\"%s\n", maxObservedFileNumber+1, fileTrailing))
w.WriteString(fmt.Sprintf(".loc %d 1 0\n", maxObservedFileNumber+1))
}
if d.processor == aarch64 {
// Grab the address of BORINGSSL_bcm_test_[start,end] via a relocation
// from a redirector function. For this to work, need to add the markers
Expand All @@ -1888,9 +1906,12 @@ func transform(w stringWriter, includes []string, inputs []inputFile) error {
}

w.WriteString(".text\n")
if d.processor == aarch64 {
w.WriteString(fmt.Sprintf(".global BORINGSSL_bcm_text_end\n"))
w.WriteString(fmt.Sprintf(".type BORINGSSL_bcm_text_end, @function\n"))
if startEndDebugDirectives {
w.WriteString(fmt.Sprintf(".loc %d 2 0\n", maxObservedFileNumber+1))
if d.processor == aarch64 {
w.WriteString(fmt.Sprintf(".global BORINGSSL_bcm_text_end\n"))
w.WriteString(fmt.Sprintf(".type BORINGSSL_bcm_text_end, @function\n"))
}
}
w.WriteString("BORINGSSL_bcm_text_end:\n")

Expand Down Expand Up @@ -2176,6 +2197,7 @@ func main() {
outFile := flag.String("o", "", "Path to output assembly")
ccPath := flag.String("cc", "", "Path to the C compiler for preprocessing inputs")
ccFlags := flag.String("cc-flags", "", "Flags for the C compiler when preprocessing")
noStartEndDebugDirectives := flag.Bool("no-se-debug-directives", false, "Disables .file/.loc directives on boundary start and end symbols")

flag.Parse()

Expand Down Expand Up @@ -2251,7 +2273,7 @@ func main() {
}
defer out.Close()

if err := transform(out, includes, inputs); err != nil {
if err := transform(out, includes, inputs, !*noStartEndDebugDirectives); err != nil {
fmt.Fprintf(os.Stderr, "%s\n", err)
os.Exit(1)
}
Expand Down
42 changes: 22 additions & 20 deletions util/fipstools/delocate/delocate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,32 +28,34 @@ var (
)

type delocateTest struct {
name string
includes []string
inputs []string
out string
name string
includes []string
inputs []string
out string
startEndDebugDirectives bool
}

func (test *delocateTest) Path(file string) string {
return filepath.Join(*testDataDir, test.name, file)
}

var delocateTests = []delocateTest{
{"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"},
{"generic-FileDirectives", nil, []string{"in.s"}, "out.s", true},
{"generic-FileDirectives-no-start-end", nil, []string{"in.s"}, "out.s", false},
{"generic-Includes", []string{"/some/include/path/openssl/foo.h", "/some/include/path/openssl/bar.h"}, []string{"in.s"}, "out.s", true},
{"ppc64le-GlobalEntry", nil, []string{"in.s"}, "out.s", true},
{"ppc64le-LoadToR0", nil, []string{"in.s"}, "out.s", true},
{"ppc64le-Sample2", nil, []string{"in.s"}, "out.s", true},
{"ppc64le-Sample", nil, []string{"in.s"}, "out.s", true},
{"ppc64le-TOCWithOffset", nil, []string{"in.s"}, "out.s", true},
{"x86_64-Basic", nil, []string{"in.s"}, "out.s", true},
{"x86_64-BSS", nil, []string{"in.s"}, "out.s", true},
{"x86_64-GOTRewrite", nil, []string{"in.s"}, "out.s", true},
{"x86_64-LargeMemory", nil, []string{"in.s"}, "out.s", true},
{"x86_64-LabelRewrite", nil, []string{"in1.s", "in2.s"}, "out.s", true},
{"x86_64-Sections", nil, []string{"in.s"}, "out.s", true},
{"x86_64-ThreeArg", nil, []string{"in.s"}, "out.s", true},
{"aarch64-Basic", nil, []string{"in.s"}, "out.s", true},
}

func TestDelocate(t *testing.T) {
Expand All @@ -72,7 +74,7 @@ func TestDelocate(t *testing.T) {
}

var buf bytes.Buffer
if err := transform(&buf, test.includes, inputs); err != nil {
if err := transform(&buf, test.includes, inputs, test.startEndDebugDirectives); err != nil {
t.Fatalf("transform failed: %s", err)
}

Expand Down
3 changes: 3 additions & 0 deletions util/fipstools/delocate/testdata/aarch64-Basic/out.s
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
.text
.file 1 "inserted_by_delocate.c"
.loc 1 1 0
.global BORINGSSL_bcm_text_start
.type BORINGSSL_bcm_text_start, @function
BORINGSSL_bcm_text_start:
Expand Down Expand Up @@ -169,6 +171,7 @@ bss_symbol:
.word 0
.size bss_symbol, 4
.text
.loc 1 2 0
.global BORINGSSL_bcm_text_end
.type BORINGSSL_bcm_text_end, @function
BORINGSSL_bcm_text_end:
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
.text
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
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
3 changes: 3 additions & 0 deletions util/fipstools/delocate/testdata/generic-FileDirectives/out.s
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
.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"
Expand All @@ -7,6 +9,7 @@ BORINGSSL_bcm_text_start:
# 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
Expand Down
3 changes: 3 additions & 0 deletions util/fipstools/delocate/testdata/generic-Includes/out.s
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#include <openssl/foo.h>
#include <openssl/bar.h>
.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"
Expand All @@ -9,6 +11,7 @@ BORINGSSL_bcm_text_start:
# 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
Expand Down
3 changes: 3 additions & 0 deletions util/fipstools/delocate/testdata/ppc64le-GlobalEntry/out.s
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
.text
.file 1 "inserted_by_delocate.c"
.loc 1 1 0
BORINGSSL_bcm_text_start:
.text
.Lfoo_local_target:
Expand All @@ -19,6 +21,7 @@ foo:

bl
.text
.loc 1 2 0
BORINGSSL_bcm_text_end:
.LBORINGSSL_external_toc:
.quad .TOC.-.LBORINGSSL_external_toc
Expand Down
3 changes: 3 additions & 0 deletions util/fipstools/delocate/testdata/ppc64le-LoadToR0/out.s
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
.text
.file 1 "inserted_by_delocate.c"
.loc 1 1 0
BORINGSSL_bcm_text_start:
.text
.Lfoo_local_target:
Expand All @@ -23,6 +25,7 @@ foo:
ld 3, -8(1)
addi 1, 1, 288
.text
.loc 1 2 0
BORINGSSL_bcm_text_end:
.type bcm_loadtoc_bar, @function
bcm_loadtoc_bar:
Expand Down
3 changes: 3 additions & 0 deletions util/fipstools/delocate/testdata/ppc64le-Sample/out.s
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
.text
.file 1 "inserted_by_delocate.c"
.loc 1 1 0
BORINGSSL_bcm_text_start:
.file "foo.c"
.abiversion 2
Expand Down Expand Up @@ -415,6 +417,7 @@ exported_function:
.ident "GCC: (Ubuntu 4.9.2-10ubuntu13) 4.9.2"
.section .note.GNU-stack,"",@progbits
.text
.loc 1 2 0
BORINGSSL_bcm_text_end:
.section ".toc", "aw"
.Lredirector_toc_fprintf:
Expand Down
3 changes: 3 additions & 0 deletions util/fipstools/delocate/testdata/ppc64le-Sample2/out.s
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
.text
.file 1 "inserted_by_delocate.c"
.loc 1 1 0
BORINGSSL_bcm_text_start:
.file "foo.c"
.abiversion 2
Expand Down Expand Up @@ -534,6 +536,7 @@ bss:
.ident "GCC: (Ubuntu 4.9.2-10ubuntu13) 4.9.2"
.section .note.GNU-stack,"",@progbits
.text
.loc 1 2 0
BORINGSSL_bcm_text_end:
.section ".toc", "aw"
.Lredirector_toc___fprintf_chk:
Expand Down
3 changes: 3 additions & 0 deletions util/fipstools/delocate/testdata/ppc64le-TOCWithOffset/out.s
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
.text
.file 1 "inserted_by_delocate.c"
.loc 1 1 0
BORINGSSL_bcm_text_start:
.text
.Lfoo_local_target:
Expand Down Expand Up @@ -99,6 +101,7 @@ foo:
ld 3, -16(1)
addi 1, 1, 288
.text
.loc 1 2 0
BORINGSSL_bcm_text_end:
.type bcm_loadtoc__dot_Lfoo_local_target, @function
bcm_loadtoc__dot_Lfoo_local_target:
Expand Down
3 changes: 3 additions & 0 deletions util/fipstools/delocate/testdata/x86_64-BSS/out.s
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
.text
.file 1 "inserted_by_delocate.c"
.loc 1 1 0
BORINGSSL_bcm_text_start:
.text
movq %rax, %rax
Expand Down Expand Up @@ -41,6 +43,7 @@ z:

.quad 0
.text
.loc 1 2 0
BORINGSSL_bcm_text_end:
.type aes_128_ctr_generic_storage_bss_get, @function
aes_128_ctr_generic_storage_bss_get:
Expand Down
3 changes: 3 additions & 0 deletions util/fipstools/delocate/testdata/x86_64-Basic/out.s
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
.text
.file 2 "inserted_by_delocate.c"
.loc 2 1 0
BORINGSSL_bcm_text_start:
# Most instructions and lines should pass unaltered. This is made up of
# copy-and-pasted bits of compiler output and likely does not actually
Expand Down Expand Up @@ -57,6 +59,7 @@ foo:
.type foo, @function
.uleb128 .foo-1-.bar
.text
.loc 2 2 0
BORINGSSL_bcm_text_end:
.type OPENSSL_ia32cap_get, @function
.globl OPENSSL_ia32cap_get
Expand Down
Loading

0 comments on commit 3f3f830

Please sign in to comment.