Skip to content

Commit

Permalink
fix: don't clobber saved frame pointer in arm64 assembly functions
Browse files Browse the repository at this point in the history
The arm64 neon assembly functions in this repository overwrite the frame
pointer saved by their callers, leading to crashes from the Go runtime
execution tracer and profilers which use frame pointer unwinding. For
historical reasons, on arm64 Go functions save the caller's frame
pointer register (x29) one word below their stack frame. See
go.dev/s/regabi#arm64-architecture. The assembly functions here,
translated from C compiler output, save values at the top of their
frame, and overwrite the frame pointer saved by the caller. We can fix
this by decrementing the stack pointer past where that frame pointer is
saved before saving anything on the stack.

Fixed with this sed script on my macos laptop + manual cleanup to match
indentation:

```sed
/stp[\t ]*x29/i\
	// The Go ABI saves the frame pointer register one word below the \
	// caller's frame. Make room so we don't overwrite it. Needs to stay \
	// 16-byte aligned \
	SUB $16, RSP

/ldp[\t ]*x29/a\
	// Put the stack pointer back where it was \
	ADD $16, RSP

```

Ran the script from the root of this repository with

	find . -name '*_arm64.s' -exec sed -f fix.sed -i '' {} +

Then manually inspected the assembly for missing SUBs/ADDs at the
beginning of functions and prior to returns.

Fixes #150
  • Loading branch information
nsrip-dd committed Oct 23, 2024
1 parent 6843412 commit 7cfd90a
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 0 deletions.
6 changes: 6 additions & 0 deletions arrow/compute/internal/kernels/cast_numeric_neon_arm64.s
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ TEXT ·_cast_type_numeric_neon(SB), $0-40
MOVD len+32(FP), R4


// The Go ABI saves the frame pointer register one word below the
// caller's frame. Make room so we don't overwrite it. Needs to stay
// 16-byte aligned
SUB $16, RSP
WORD $0xa9bf7bfd // stp x29, x30, [sp, #-16]!
WORD $0x7100181f // cmp w0, #6
WORD $0x910003fd // mov x29, sp
Expand Down Expand Up @@ -4447,6 +4451,8 @@ LBB0_892:
BNE LBB0_892
LBB0_893:
WORD $0xa8c17bfd // ldp x29, x30, [sp], #16
// Put the stack pointer back where it was
ADD $16, RSP
RET
LBB0_894:
WORD $0x927b6909 // and x9, x8, #0xffffffe0
Expand Down
8 changes: 8 additions & 0 deletions arrow/math/int64_neon_arm64.s
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ TEXT ·_sum_int64_neon(SB), $0-24
MOVD len+8(FP), R1
MOVD res+16(FP), R2

// The Go ABI saves the frame pointer register one word below the
// caller's frame. Make room so we don't overwrite it. Needs to stay
// 16-byte aligned
SUB $16, RSP
WORD $0xa9bf7bfd // stp x29, x30, [sp, #-16]!
WORD $0x910003fd // mov x29, sp
CBZ R1, LBB0_3
Expand All @@ -23,6 +27,8 @@ LBB0_3:
WORD $0xaa1f03e9 // mov x9, xzr
WORD $0xf9000049 // str x9, [x2]
WORD $0xa8c17bfd // ldp x29, x30, [sp], #16
// Put the stack pointer back where it was
ADD $16, RSP
RET
LBB0_4:
WORD $0x927ef428 // and x8, x1, #0xfffffffffffffffc
Expand Down Expand Up @@ -54,5 +60,7 @@ LBB0_8:
LBB0_9:
WORD $0xf9000049 // str x9, [x2]
WORD $0xa8c17bfd // ldp x29, x30, [sp], #16
// Put the stack pointer back where it was
ADD $16, RSP
RET

8 changes: 8 additions & 0 deletions arrow/math/uint64_neon_arm64.s
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ TEXT ·_sum_uint64_neon(SB), $0-24
MOVD len+8(FP), R1
MOVD res+16(FP), R2

// The Go ABI saves the frame pointer register one word below the
// caller's frame. Make room so we don't overwrite it. Needs to stay
// 16-byte aligned
SUB $16, RSP
WORD $0xa9bf7bfd // stp x29, x30, [sp, #-16]!
WORD $0x910003fd // mov x29, sp
CBZ R1, LBB0_3
Expand All @@ -23,6 +27,8 @@ LBB0_3:
WORD $0xaa1f03e9 // mov x9, xzr
WORD $0xf9000049 // str x9, [x2]
WORD $0xa8c17bfd // ldp x29, x30, [sp], #16
// Put the stack pointer back where it was
ADD $16, RSP
RET
LBB0_4:
WORD $0x927ef428 // and x8, x1, #0xfffffffffffffffc
Expand Down Expand Up @@ -54,5 +60,7 @@ LBB0_8:
LBB0_9:
WORD $0xf9000049 // str x9, [x2]
WORD $0xa8c17bfd // ldp x29, x30, [sp], #16
// Put the stack pointer back where it was
ADD $16, RSP
RET

6 changes: 6 additions & 0 deletions arrow/memory/memory_neon_arm64.s
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ TEXT ·_memset_neon(SB), $0-24
MOVD len+8(FP), R1
MOVD c+16(FP), R2

// The Go ABI saves the frame pointer register one word below the
// caller's frame. Make room so we don't overwrite it. Needs to stay
// 16-byte aligned
SUB $16, RSP
WORD $0xa9bf7bfd // stp x29, x30, [sp, #-16]!
WORD $0x8b010008 // add x8, x0, x1
WORD $0xeb00011f // cmp x8, x0
Expand Down Expand Up @@ -40,4 +44,6 @@ LBB0_6:
BNE LBB0_6
LBB0_7:
WORD $0xa8c17bfd // ldp x29, x30, [sp], #16
// Put the stack pointer back where it was
ADD $16, RSP
RET
32 changes: 32 additions & 0 deletions internal/utils/min_max_neon_arm64.s
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ TEXT ·_int32_max_min_neon(SB), $0-32
MOVD minout+16(FP), R2
MOVD maxout+24(FP), R3

// The Go ABI saves the frame pointer register one word below the
// caller's frame. Make room so we don't overwrite it. Needs to stay
// 16-byte aligned
SUB $16, RSP
WORD $0xa9bf7bfd // stp x29, x30, [sp, #-16]!
WORD $0x7100043f // cmp w1, #1
WORD $0x910003fd // mov x29, sp
Expand All @@ -32,6 +36,8 @@ LBB0_3:
WORD $0xb900006b // str w11, [x3]
WORD $0xb900004a // str w10, [x2]
WORD $0xa8c17bfd // ldp x29, x30, [sp], #16
// Put the stack pointer back where it was
ADD $16, RSP
RET
LBB0_4:
WORD $0x927e7509 // and x9, x8, #0xfffffffc
Expand Down Expand Up @@ -76,6 +82,8 @@ LBB0_9:
WORD $0xb900006b // str w11, [x3]
WORD $0xb900004a // str w10, [x2]
WORD $0xa8c17bfd // ldp x29, x30, [sp], #16
// Put the stack pointer back where it was
ADD $16, RSP
RET

// func _uint32_max_min_neon(values unsafe.Pointer, length int, minout, maxout unsafe.Pointer)
Expand All @@ -86,6 +94,10 @@ TEXT ·_uint32_max_min_neon(SB), $0-32
MOVD minout+16(FP), R2
MOVD maxout+24(FP), R3

// The Go ABI saves the frame pointer register one word below the
// caller's frame. Make room so we don't overwrite it. Needs to stay
// 16-byte aligned
SUB $16, RSP
WORD $0xa9bf7bfd // stp x29, x30, [sp, #-16]!
WORD $0x7100043f // cmp w1, #1
WORD $0x910003fd // mov x29, sp
Expand All @@ -105,6 +117,8 @@ LBB1_3:
WORD $0xb900006a // str w10, [x3]
WORD $0xb900004b // str w11, [x2]
WORD $0xa8c17bfd // ldp x29, x30, [sp], #16
// Put the stack pointer back where it was
ADD $16, RSP
RET
LBB1_4:
WORD $0x927e7509 // and x9, x8, #0xfffffffc
Expand Down Expand Up @@ -149,6 +163,8 @@ LBB1_9:
WORD $0xb900006a // str w10, [x3]
WORD $0xb900004b // str w11, [x2]
WORD $0xa8c17bfd // ldp x29, x30, [sp], #16
// Put the stack pointer back where it was
ADD $16, RSP
RET

// func _int64_max_min_neon(values unsafe.Pointer, length int, minout, maxout unsafe.Pointer)
Expand All @@ -159,6 +175,10 @@ TEXT ·_int64_max_min_neon(SB), $0-32
MOVD minout+16(FP), R2
MOVD maxout+24(FP), R3

// The Go ABI saves the frame pointer register one word below the
// caller's frame. Make room so we don't overwrite it. Needs to stay
// 16-byte aligned
SUB $16, RSP
WORD $0xa9bf7bfd // stp x29, x30, [sp, #-16]!
WORD $0x7100043f // cmp w1, #1
WORD $0x910003fd // mov x29, sp
Expand All @@ -178,6 +198,8 @@ LBB2_3:
WORD $0xf900006b // str x11, [x3]
WORD $0xf900004a // str x10, [x2]
WORD $0xa8c17bfd // ldp x29, x30, [sp], #16
// Put the stack pointer back where it was
ADD $16, RSP
RET
LBB2_4:
WORD $0x927e7509 // and x9, x8, #0xfffffffc
Expand Down Expand Up @@ -234,6 +256,8 @@ LBB2_9:
WORD $0xf900006b // str x11, [x3]
WORD $0xf900004a // str x10, [x2]
WORD $0xa8c17bfd // ldp x29, x30, [sp], #16
// Put the stack pointer back where it was
ADD $16, RSP
RET


Expand All @@ -245,6 +269,10 @@ TEXT ·_uint64_max_min_neon(SB), $0-32
MOVD minout+16(FP), R2
MOVD maxout+24(FP), R3

// The Go ABI saves the frame pointer register one word below the
// caller's frame. Make room so we don't overwrite it. Needs to stay
// 16-byte aligned
SUB $16, RSP
WORD $0xa9bf7bfd // stp x29, x30, [sp, #-16]!
WORD $0x7100043f // cmp w1, #1
WORD $0x910003fd // mov x29, sp
Expand All @@ -264,6 +292,8 @@ LBB3_3:
WORD $0xf900006a // str x10, [x3]
WORD $0xf900004b // str x11, [x2]
WORD $0xa8c17bfd // ldp x29, x30, [sp], #16
// Put the stack pointer back where it was
ADD $16, RSP
RET
LBB3_4:
WORD $0x927e7509 // and x9, x8, #0xfffffffc
Expand Down Expand Up @@ -320,5 +350,7 @@ LBB3_9:
WORD $0xf900006a // str x10, [x3]
WORD $0xf900004b // str x11, [x2]
WORD $0xa8c17bfd // ldp x29, x30, [sp], #16
// Put the stack pointer back where it was
ADD $16, RSP
RET

6 changes: 6 additions & 0 deletions parquet/internal/bmi/bitmap_neon_arm64.s
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ TEXT ·_levels_to_bitmap_neon(SB), $0-32
MOVD numLevels+8(FP), R1
MOVD rhs+16(FP), R2

// The Go ABI saves the frame pointer register one word below the
// caller's frame. Make room so we don't overwrite it. Needs to stay
// 16-byte aligned
SUB $16, RSP
WORD $0xa9bf7bfd // stp x29, x30, [sp, #-16]!
WORD $0x7100043f // cmp w1, #1
WORD $0x910003fd // mov x29, sp
Expand Down Expand Up @@ -79,6 +83,8 @@ LBB1_7:
BNE LBB1_7
LBB1_8:
WORD $0xa8c17bfd // ldp x29, x30, [sp], #16
// Put the stack pointer back where it was
ADD $16, RSP
MOVD R8, res+24(FP)
RET

6 changes: 6 additions & 0 deletions parquet/internal/utils/bit_packing_neon_arm64.s
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,10 @@ TEXT ·_unpack32_neon(SB), $0-40
// LEAQ LCDATA1<>(SB), BP

// %bb.0:
// The Go ABI saves the frame pointer register one word below the
// caller's frame. Make room so we don't overwrite it. Needs to stay
// 16-byte aligned
SUB $16, RSP
WORD $0xa9ba7bfd // stp x29, x30, [sp, #-96]!
WORD $0xd10643e9 // sub x9, sp, #400
WORD $0xa9016ffc // stp x28, x27, [sp, #16]
Expand Down Expand Up @@ -6922,5 +6926,7 @@ LBB0_156:
WORD $0xa94267fa // ldp x26, x25, [sp, #32]
WORD $0xa9416ffc // ldp x28, x27, [sp, #16]
WORD $0xa8c67bfd // ldp x29, x30, [sp], #96
// Put the stack pointer back where it was
ADD $16, RSP
MOVD R0, num+32(FP)
RET
6 changes: 6 additions & 0 deletions parquet/internal/utils/unpack_bool_neon_arm64.s
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ TEXT ·_bytes_to_bools_neon(SB), $0-32
MOVD out+16(FP), R2
MOVD outlen+24(FP), R3

// The Go ABI saves the frame pointer register one word below the
// caller's frame. Make room so we don't overwrite it. Needs to stay
// 16-byte aligned
SUB $16, RSP
WORD $0xa9bf7bfd // stp x29, x30, [sp, #-16]!
WORD $0x7100043f // cmp w1, #1
WORD $0x910003fd // mov x29, sp
Expand Down Expand Up @@ -78,4 +82,6 @@ LBB0_3:
JMP LBB0_2
LBB0_12:
WORD $0xa8c17bfd // ldp x29, x30, [sp], #16
// Put the stack pointer back where it was
ADD $16, RSP
RET

0 comments on commit 7cfd90a

Please sign in to comment.