Skip to content

Commit

Permalink
Do no use .cfi_negate_ra_state within the assembly on Arm64
Browse files Browse the repository at this point in the history 
Compiling openzfs on aarch64 with gcc-8 and gcc-9 is failing currently.
See issue #14965 for deeper context.

On platforms without pointer authentication, .cfi_negate_ra_state can be
defined to a no-op:
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/aarch64-tdep.c#l1413

I have tested this on Arm64 FreeBSD 13.2 and AlmaLinux-8.

Reviewed-by: Andrew Turner <[email protected]>
Signed-off-by: Tino Reichardt <[email protected]>
Closes #14965
Closes #15784
  • Loading branch information
@mcmilk
mcmilk authored Apr 15, 2024
1 parent c6da985 commit 90ba19e
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 5 deletions.
14 changes: 11 additions & 3 deletions module/icp/asm-aarch64/blake3/b3_aarch64_sse2.S
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@
*/

#if defined(__aarch64__)

/* make gcc <= 9 happy */
#if LD_VERSION >= 233010000

This comment has been minimized.

Sign in to view

Copy link
@mmatuska

mmatuska Apr 16, 2024

Contributor

@mcmilk shouldn't this be
if !defined(LD_VERSION) || LD_VERSION >= 233010000

This comment has been minimized.

Sign in to view

Copy link
@mcmilk

mcmilk Apr 17, 2024

Author Contributor

Do we have really some architecture where LD_VERSION isn't defined?
It's only an ugly hack around older compilers - I hope we can remove this later.

This comment has been minimized.

Sign in to view

Copy link
@mmatuska

mmatuska Apr 17, 2024

Contributor

Do we have really some architecture where LD_VERSION isn't defined? It's only an ugly hack around older compilers - I hope we can remove this later.

Yes it doesn't build when merged to FreeBSD base, as there is no LD_VERSION with clang.

This comment has been minimized.

Sign in to view

Copy link
@mcmilk

mcmilk Apr 17, 2024

Author Contributor

Okay, I will create a fast fix for that.
#define CFI_NEGATE_RA_STATE .cfi_negate_ra_state
#else
#define CFI_NEGATE_RA_STATE
#endif

.text
.section .note.gnu.property,"a",@note
.p2align 3
Expand All @@ -51,7 +59,7 @@
zfs_blake3_compress_in_place_sse2:
.cfi_startproc
hint #25
.cfi_negate_ra_state
CFI_NEGATE_RA_STATE
sub sp, sp, #96
stp x29, x30, [sp, #64]
add x29, sp, #64
Expand Down Expand Up @@ -555,7 +563,7 @@ compress_pre:
zfs_blake3_compress_xof_sse2:
.cfi_startproc
hint #25
.cfi_negate_ra_state
CFI_NEGATE_RA_STATE
sub sp, sp, #96
stp x29, x30, [sp, #64]
add x29, sp, #64
Expand Down Expand Up @@ -608,7 +616,7 @@ zfs_blake3_compress_xof_sse2:
zfs_blake3_hash_many_sse2:
.cfi_startproc
hint #25
.cfi_negate_ra_state
CFI_NEGATE_RA_STATE
stp d15, d14, [sp, #-160]!
stp d13, d12, [sp, #16]
stp d11, d10, [sp, #32]
Expand Down
12 changes: 10 additions & 2 deletions module/icp/asm-aarch64/blake3/b3_aarch64_sse41.S
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@
*/

#if defined(__aarch64__)

/* make gcc <= 9 happy */
#if LD_VERSION >= 233010000

This comment has been minimized.

Sign in to view

Copy link
@mmatuska

mmatuska Apr 16, 2024 •
edited
Loading

Contributor

@mcmilk same here?
if !defined(LD_VERSION) || LD_VERSION >= 233010000
#define CFI_NEGATE_RA_STATE .cfi_negate_ra_state
#else
#define CFI_NEGATE_RA_STATE
#endif

.text
.section .note.gnu.property,"a",@note
.p2align 3
Expand All @@ -51,7 +59,7 @@
zfs_blake3_compress_in_place_sse41:
.cfi_startproc
hint #25
.cfi_negate_ra_state
CFI_NEGATE_RA_STATE
sub sp, sp, #96
stp x29, x30, [sp, #64]
add x29, sp, #64
Expand Down Expand Up @@ -565,7 +573,7 @@ compress_pre:
zfs_blake3_compress_xof_sse41:
.cfi_startproc
hint #25
.cfi_negate_ra_state
CFI_NEGATE_RA_STATE
sub sp, sp, #96
stp x29, x30, [sp, #64]
add x29, sp, #64
Expand Down

0 comments on commit 90ba19e

Please sign in to comment.