From d06cba6376b2735c54bfa5d158649f308bedc844 Mon Sep 17 00:00:00 2001 From: kelvin Date: Mon, 3 Nov 2025 16:05:53 +0000 Subject: [PATCH 1/8] Update unit tests to reproduce bug --- test/asm/lib/BUILD | 5 ++++- test/asm/lib/add.s | 2 ++ test/asm/lib/asm.go | 8 ++++++++ test/asm/lib/asm_test.go | 4 ++++ test/asm/lib/golib/go_lib.go | 4 ++-- test/asm/lib/subtract.s | 9 +++++++++ 6 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 test/asm/lib/subtract.s diff --git a/test/asm/lib/BUILD b/test/asm/lib/BUILD index a084a748..e66eecf1 100644 --- a/test/asm/lib/BUILD +++ b/test/asm/lib/BUILD @@ -3,7 +3,10 @@ subinclude("//build_defs:go") go_library( name = "asm", srcs = ["asm.go"], - asm_srcs = ["add.s"], + asm_srcs = [ + "add.s", + "subtract.s", + ], deps = ["//test/asm/lib/golib"], ) diff --git a/test/asm/lib/add.s b/test/asm/lib/add.s index 94a24c33..927ac283 100644 --- a/test/asm/lib/add.s +++ b/test/asm/lib/add.s @@ -1,3 +1,5 @@ +GLOBL COLLIDINGSYMBOL<>(SB), 8, $32 + // func add(x, y int64) TEXT ·add(SB),$0-24 MOVQ x+0(FP), BX diff --git a/test/asm/lib/asm.go b/test/asm/lib/asm.go index f0520853..34104a68 100644 --- a/test/asm/lib/asm.go +++ b/test/asm/lib/asm.go @@ -10,3 +10,11 @@ func add(x, y int64) int64 func Add() int { return int(add(golib.LHS, golib.RHS)) } + +// subtract is the forward declaration of the assembly implementation. +func subtract(x, y int) int64 + +// Subtract subtracts two numbers using assembly. +func Subtract() int { + return int(subtract(golib.LHS, golib.RHS)) +} diff --git a/test/asm/lib/asm_test.go b/test/asm/lib/asm_test.go index 4809113e..1ec3b8c5 100644 --- a/test/asm/lib/asm_test.go +++ b/test/asm/lib/asm_test.go @@ -13,3 +13,7 @@ import ( func TestAssemblyAdd(t *testing.T) { assert.Equal(t, 15, asm.Add()) } + +func TestAssemblySubtract(t *testing.T) { + assert.Equal(t, 5, asm.Subtract()) +} diff --git a/test/asm/lib/golib/go_lib.go b/test/asm/lib/golib/go_lib.go index 8bcb60ce..147e50f5 100644 --- a/test/asm/lib/golib/go_lib.go +++ b/test/asm/lib/golib/go_lib.go @@ -1,4 +1,4 @@ package golib -const LHS = 5 -const RHS = 10 +const LHS = 10 +const RHS = 5 diff --git a/test/asm/lib/subtract.s b/test/asm/lib/subtract.s new file mode 100644 index 00000000..714039f4 --- /dev/null +++ b/test/asm/lib/subtract.s @@ -0,0 +1,9 @@ +GLOBL COLLIDINGSYMBOL<>(SB), 8, $32 + +// func subtract(x, y int64) +TEXT ·subtract(SB),$0-24 + MOVQ x+0(FP), BX + MOVQ y+8(FP), BP + SUBQ BP, BX + MOVQ BX, ret+16(FP) + RET From 96c51f9c6fde2a67bb25a3a616ce1767940a9027 Mon Sep 17 00:00:00 2001 From: kelvin Date: Mon, 3 Nov 2025 17:42:50 +0000 Subject: [PATCH 2/8] WIP: Compile each Assembly file individually --- build_defs/go.build_defs | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/build_defs/go.build_defs b/build_defs/go.build_defs index 741db108..f79dd2d2 100644 --- a/build_defs/go.build_defs +++ b/build_defs/go.build_defs @@ -386,9 +386,25 @@ def go_library(name:str, srcs:list, resources:list=[], asm_srcs:list=None, hdrs: 'asm': asm_srcs, 'hdrs': [hdrs_rule], }, - outs = [name + '.abi'], + output_dirs = ["_out"], building_description = 'Creating ABI...', - cmd = f'eval `"$TOOL" env` && mkdir include && touch include/go_asm.h && "$TOOL" tool asm -trimpath "$TMP_DIR" -I include -I $GOROOT/pkg/include -D GOOS_{CONFIG.OS} -D GOARCH_{CONFIG.ARCH} -gensymabis -p {package_path} -o "$OUT" $SRCS_ASM', + cmd = f"""eval `"$TOOL" env` && + mkdir _out && + mkdir include && + touch include/go_asm.h && + for ASM_FILE in $SRCS_ASM + do "$TOOL" tool asm \\ + -trimpath "$TMP_DIR" \\ + -I include \\ + -I $GOROOT/pkg/include \\ + -D GOOS_{CONFIG.OS} \\ + -D GOARCH_{CONFIG.ARCH} \\ + -gensymabis \\ + -p {package_path} \\ + -o "_out/$(basename -s .s $ASM_FILE).abi" \\ + $ASM_FILE + done + """, env= { "GOOS": CONFIG.OS, "GOARCH": CONFIG.ARCH, @@ -424,9 +440,22 @@ def go_library(name:str, srcs:list, resources:list=[], asm_srcs:list=None, hdrs: 'hdrs': [hdrs_rule], 'goasm': [lib_rule + '|h'], }, - outs = [name + '.o'], + output_dirs = ["_out"], building_description = 'Assembling...', - cmd = f'eval `"$TOOL" env` && mkdir include && mv $SRCS_GOASM include/go_asm.h && "$TOOL" tool asm -trimpath "$TMP_DIR" -I "$TMP_DIR"/include -I ${GOROOT}/pkg/include -D GOOS={CONFIG.OS} -D GOARCH_{CONFIG.ARCH} -p {package_path} -o "$OUT" $SRCS_ASM', + cmd = f"""eval `"$TOOL" env` && + mkdir include && + mv $SRCS_GOASM include/go_asm.h && + for ASM_FILE in $SRCS_ASM + do "$TOOL" tool asm \\ + -trimpath "$TMP_DIR" \\ + -I "$TMP_DIR"/include \\ + -I ${GOROOT}/pkg/include \\ + -D GOOS={CONFIG.OS} \\ + -D GOARCH_{CONFIG.ARCH} \\ + -p {package_path} \\ + -o "_out/$(basename -s .s $ASM_FILE).o" + $ASM_FILE + done""", env= { "GOOS": CONFIG.OS, "GOARCH": CONFIG.ARCH, From acd3b6648d01e89449ce8f7b685d6fd1b5900bbe Mon Sep 17 00:00:00 2001 From: kelvin Date: Tue, 4 Nov 2025 10:51:59 +0000 Subject: [PATCH 3/8] Revert changes to abi build rule --- build_defs/go.build_defs | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/build_defs/go.build_defs b/build_defs/go.build_defs index f79dd2d2..c744c5a5 100644 --- a/build_defs/go.build_defs +++ b/build_defs/go.build_defs @@ -386,25 +386,9 @@ def go_library(name:str, srcs:list, resources:list=[], asm_srcs:list=None, hdrs: 'asm': asm_srcs, 'hdrs': [hdrs_rule], }, - output_dirs = ["_out"], building_description = 'Creating ABI...', - cmd = f"""eval `"$TOOL" env` && - mkdir _out && - mkdir include && - touch include/go_asm.h && - for ASM_FILE in $SRCS_ASM - do "$TOOL" tool asm \\ - -trimpath "$TMP_DIR" \\ - -I include \\ - -I $GOROOT/pkg/include \\ - -D GOOS_{CONFIG.OS} \\ - -D GOARCH_{CONFIG.ARCH} \\ - -gensymabis \\ - -p {package_path} \\ - -o "_out/$(basename -s .s $ASM_FILE).abi" \\ - $ASM_FILE - done - """, + outs = [name + '.abi'], + cmd = f'eval `"$TOOL" env` && mkdir include && touch include/go_asm.h && "$TOOL" tool asm -trimpath "$TMP_DIR" -I include -I $GOROOT/pkg/include -D GOOS_{CONFIG.OS} -D GOARCH_{CONFIG.ARCH} -gensymabis -p {package_path} -o "$OUT" $SRCS_ASM', env= { "GOOS": CONFIG.OS, "GOARCH": CONFIG.ARCH, @@ -443,6 +427,7 @@ def go_library(name:str, srcs:list, resources:list=[], asm_srcs:list=None, hdrs: output_dirs = ["_out"], building_description = 'Assembling...', cmd = f"""eval `"$TOOL" env` && + mkdir _out && mkdir include && mv $SRCS_GOASM include/go_asm.h && for ASM_FILE in $SRCS_ASM @@ -453,7 +438,7 @@ def go_library(name:str, srcs:list, resources:list=[], asm_srcs:list=None, hdrs: -D GOOS={CONFIG.OS} \\ -D GOARCH_{CONFIG.ARCH} \\ -p {package_path} \\ - -o "_out/$(basename -s .s $ASM_FILE).o" + -o "_out/$(basename -s .s $ASM_FILE).o" \\ $ASM_FILE done""", env= { From ff8b762d5987e217d8f79f27d16ffb34b5be5416 Mon Sep 17 00:00:00 2001 From: kelvin Date: Tue, 4 Nov 2025 11:05:28 +0000 Subject: [PATCH 4/8] Revert .abi build rule change --- build_defs/go.build_defs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build_defs/go.build_defs b/build_defs/go.build_defs index c744c5a5..bf35fd8e 100644 --- a/build_defs/go.build_defs +++ b/build_defs/go.build_defs @@ -386,8 +386,8 @@ def go_library(name:str, srcs:list, resources:list=[], asm_srcs:list=None, hdrs: 'asm': asm_srcs, 'hdrs': [hdrs_rule], }, - building_description = 'Creating ABI...', outs = [name + '.abi'], + building_description = 'Creating ABI...', cmd = f'eval `"$TOOL" env` && mkdir include && touch include/go_asm.h && "$TOOL" tool asm -trimpath "$TMP_DIR" -I include -I $GOROOT/pkg/include -D GOOS_{CONFIG.OS} -D GOARCH_{CONFIG.ARCH} -gensymabis -p {package_path} -o "$OUT" $SRCS_ASM', env= { "GOOS": CONFIG.OS, From 25ed05c65b848ac949a1a3171d2fdba3c2a79994 Mon Sep 17 00:00:00 2001 From: kelvin Date: Tue, 4 Nov 2025 13:54:57 +0000 Subject: [PATCH 5/8] Improve unit tests --- test/asm/lib/asm_test.go | 4 ++-- test/asm/lib/golib/go_lib.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/asm/lib/asm_test.go b/test/asm/lib/asm_test.go index 1ec3b8c5..29e0df0b 100644 --- a/test/asm/lib/asm_test.go +++ b/test/asm/lib/asm_test.go @@ -11,9 +11,9 @@ import ( ) func TestAssemblyAdd(t *testing.T) { - assert.Equal(t, 15, asm.Add()) + assert.Equal(t, 14, asm.Add()) } func TestAssemblySubtract(t *testing.T) { - assert.Equal(t, 5, asm.Subtract()) + assert.Equal(t, 6, asm.Subtract()) } diff --git a/test/asm/lib/golib/go_lib.go b/test/asm/lib/golib/go_lib.go index 147e50f5..560e6a18 100644 --- a/test/asm/lib/golib/go_lib.go +++ b/test/asm/lib/golib/go_lib.go @@ -1,4 +1,4 @@ package golib const LHS = 10 -const RHS = 5 +const RHS = 4 From 66074916b203bbdbcb5009c474de719d30a2dfb3 Mon Sep 17 00:00:00 2001 From: Chris Novakovic Date: Wed, 5 Nov 2025 15:27:21 +0000 Subject: [PATCH 6/8] Please-ify --- build_defs/go.build_defs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/build_defs/go.build_defs b/build_defs/go.build_defs index bf35fd8e..aa61309b 100644 --- a/build_defs/go.build_defs +++ b/build_defs/go.build_defs @@ -426,21 +426,21 @@ def go_library(name:str, srcs:list, resources:list=[], asm_srcs:list=None, hdrs: }, output_dirs = ["_out"], building_description = 'Assembling...', - cmd = f"""eval `"$TOOL" env` && - mkdir _out && - mkdir include && - mv $SRCS_GOASM include/go_asm.h && - for ASM_FILE in $SRCS_ASM - do "$TOOL" tool asm \\ - -trimpath "$TMP_DIR" \\ - -I "$TMP_DIR"/include \\ - -I ${GOROOT}/pkg/include \\ - -D GOOS={CONFIG.OS} \\ - -D GOARCH_{CONFIG.ARCH} \\ - -p {package_path} \\ - -o "_out/$(basename -s .s $ASM_FILE).o" \\ - $ASM_FILE - done""", + cmd = " && ".join([ + 'eval `"$TOOL" env`', + 'mkdir _out', + 'mkdir include', + 'mv $SRCS_GOASM include/go_asm.h', + 'for ASM_FILE in $SRCS_ASM; do "$TOOL" tool asm ' + " ".join([ + '-trimpath "$TMP_DIR"', + '-I "$TMP_DIR"/include', + '-I ${GOROOT}/pkg/include', + f'-D GOOS={CONFIG.OS}', + f'-D GOARCH_{CONFIG.ARCH}', + f'-p {package_path}', + '-o "_out/$(basename -s .s $ASM_FILE).o"', + ]) + ' $ASM_FILE; done', + ]), env= { "GOOS": CONFIG.OS, "GOARCH": CONFIG.ARCH, From d45262d71f5af3025d382d8bc4c2b58be2b7381a Mon Sep 17 00:00:00 2001 From: Chris Novakovic Date: Wed, 5 Nov 2025 15:39:46 +0000 Subject: [PATCH 7/8] Avoid using `output_dirs`; invoke `basename(1)` POSIXly --- build_defs/go.build_defs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/build_defs/go.build_defs b/build_defs/go.build_defs index aa61309b..d1fde33e 100644 --- a/build_defs/go.build_defs +++ b/build_defs/go.build_defs @@ -424,11 +424,10 @@ def go_library(name:str, srcs:list, resources:list=[], asm_srcs:list=None, hdrs: 'hdrs': [hdrs_rule], 'goasm': [lib_rule + '|h'], }, - output_dirs = ["_out"], + outs = [name + "#" + (splitext(basename(a)))[0] + ".o" for a in asm_srcs], building_description = 'Assembling...', cmd = " && ".join([ 'eval `"$TOOL" env`', - 'mkdir _out', 'mkdir include', 'mv $SRCS_GOASM include/go_asm.h', 'for ASM_FILE in $SRCS_ASM; do "$TOOL" tool asm ' + " ".join([ @@ -438,7 +437,7 @@ def go_library(name:str, srcs:list, resources:list=[], asm_srcs:list=None, hdrs: f'-D GOOS={CONFIG.OS}', f'-D GOARCH_{CONFIG.ARCH}', f'-p {package_path}', - '-o "_out/$(basename -s .s $ASM_FILE).o"', + f'-o "{name}#$(basename $ASM_FILE .s).o"', ]) + ' $ASM_FILE; done', ]), env= { From 5ffa7b755fc01735157acbfa0051fa07f02f5194 Mon Sep 17 00:00:00 2001 From: Chris Novakovic Date: Wed, 5 Nov 2025 16:01:53 +0000 Subject: [PATCH 8/8] Handle `.s` and `.S` files --- build_defs/go.build_defs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build_defs/go.build_defs b/build_defs/go.build_defs index d1fde33e..8de215fd 100644 --- a/build_defs/go.build_defs +++ b/build_defs/go.build_defs @@ -437,7 +437,7 @@ def go_library(name:str, srcs:list, resources:list=[], asm_srcs:list=None, hdrs: f'-D GOOS={CONFIG.OS}', f'-D GOARCH_{CONFIG.ARCH}', f'-p {package_path}', - f'-o "{name}#$(basename $ASM_FILE .s).o"', + f'''-o "{name}#$(basename $ASM_FILE | sed -e 's/\.[sS]$//').o"''', ]) + ' $ASM_FILE; done', ]), env= {