[Bug target/110773] New: [Aarch64] crash (SIGBUS) due to atomic instructions on under-aligned memory

[Bug target/110773] New: [Aarch64] crash (SIGBUS) due to atomic instructions on under-aligned memory

[scw-gcc at google dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110773

            Bug ID: 110773
           Summary: [Aarch64] crash (SIGBUS) due to atomic instructions on
                    under-aligned memory
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: scw-gcc at google dot com
  Target Milestone: ---

This reproduces in versions as far back as godbolt has ARM64 gcc (5.4).

The following code snippet has two copies of 4-byte-aligned, 8-byte-sized
objects `fp1` and `fp2`. Their placements in `storage` guarantee that they are
12 bytes apart, and thus one would be 8-byte aligned and one would not (it'd
still be 4-byte aligned, though).

=====
struct FloatPair {
    float f1;
    float f2;
};

struct Storage {
    FloatPair fp1;
    float padding;
    FloatPair fp2;
} storage;

float f() {
    FloatPair fp1, fp2;
    __atomic_load(&storage.fp1, &fp1, __ATOMIC_SEQ_CST);
    __atomic_load(&storage.fp2, &fp2, __ATOMIC_SEQ_CST);
    return fp1.f1 + fp1.f2 + fp2.f1 + fp2.f2;
}
=====

Godbolt with GCC and Clang: https://godbolt.org/z/P9rbTePnG

GCC uses two `ldar` instructions for the loads while Clang makes calls to
libatomic. The GCC codegen crashes on AArch64 machines (tested on Cavium
ThunderX2 as well as Neoverse-N1).

AArch64 allows unaligned memory access, except for atomic operations:
https://developer.arm.com/documentation/ddi0596/2021-03/Shared-Pseudocode/AArch64-Functions?lang=en#AArch64.CheckAlignment.4

For memory reads, it uses the size as the alignment (unless it's operating on a
pair of 64-bit registers):
https://developer.arm.com/documentation/ddi0596/2021-03/Shared-Pseudocode/AArch64-Functions?lang=en#impl-aarch64.Mem.read.3

In this case, `FloatPair` has size 8 and fit in one 64-bit register, so the
64-bit `ldar` can only be used on 8-byte-aligned reads. One of the two call
will thus violate the alignment requirement.

Potentially related, GCC also uses single atomic RMW instructions when
available regardless of alignment. To trigger this, one has to construct
unaligned pointers so I'm not sure if it's a problem.

=====
struct Storage {
    char c;
    int i;
} __attribute__((packed))
storage;

int inc() {
    return __atomic_add_fetch(&storage.i, 1, __ATOMIC_SEQ_CST);
}
=====

Needs -march=armv8.1-a (LSE) to reproduce: https://godbolt.org/z/qKM1fGbrj

In my testing, even using the clang codegen it still crashes inside libatomic,
though.

[Bug middle-end/110773] [Aarch64] crash (SIGBUS) due to atomic instructions on under-aligned memory

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110773

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|target                      |middle-end

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
this happens on x86_64 too:
        movq    storage+12(%rip), %rax

This is almost definitely not atomic too ..

[Bug middle-end/110773] [Aarch64] crash (SIGBUS) due to atomic instructions on under-aligned memory

[wilco at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110773

Wilco <wilco at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wilco at gcc dot gnu.org

--- Comment #2 from Wilco <wilco at gcc dot gnu.org> ---
This is really a user error, not a compiler issue. Just write it like:

struct Storage {
    std::atomic<FloatPair> fp1;
    float padding;
    std::atomic<FloatPair> fp2;
} storage;

This ensures the correct alignment required for atomic accesses of fp1/fp2.

[Bug middle-end/110773] [Aarch64] crash (SIGBUS) due to atomic instructions on under-aligned memory

[sainan+gcc.bugzilla at calamity dot gg]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110773

Sainan <sainan+gcc.bugzilla at calamity dot gg> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sainan+gcc.bugzilla@calamit
                   |                            |y.gg

--- Comment #3 from Sainan <sainan+gcc.bugzilla at calamity dot gg> ---
I seem to be having a related issue, although in my case the struct looks like
this:

template <typename T>
struct Data
{
    T* data;
    std::atomic_uint count;
    bool flag;
};

And it's crashing on `--count;`

Surely this is not a user issue in this case because the pointer should always
be 8 bytes, so count should be evenly aligned on a 8-byte boundary. (Unless the
atomic operation needs 16-byte alignment?)

Same code also runs fine when compiled via MSVC and run on Windows, although
it's unclear if this might simply be my Linux test machine running an older ARM
CPU compared to my Windows on ARM test machine.

[Bug middle-end/110773] [Aarch64] crash (SIGBUS) due to atomic instructions on under-aligned memory

[wilco at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110773

--- Comment #4 from Wilco <wilco at gcc dot gnu.org> ---
(In reply to Sainan from comment #3)
> I seem to be having a related issue, although in my case the struct looks
> like this:
> 
> template <typename T>
> struct Data
> {
>     T* data;
>     std::atomic_uint count;
>     bool flag;
> };
> 
> And it's crashing on `--count;`
> 
> Surely this is not a user issue in this case because the pointer should
> always be 8 bytes, so count should be evenly aligned on a 8-byte boundary.
> (Unless the atomic operation needs 16-byte alignment?)

The atomic will also set correct struct alignment.

> Same code also runs fine when compiled via MSVC and run on Windows, although
> it's unclear if this might simply be my Linux test machine running an older
> ARM CPU compared to my Windows on ARM test machine.

You would get a crash if you build for LSE so you get a LDADDAL instruction and
then run it on a CPU that doesn't. So try -mcpu=native and it should work.

[Bug middle-end/110773] [Aarch64] crash (SIGBUS) due to atomic instructions on under-aligned memory

[sainan+gcc.bugzilla at calamity dot gg]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110773

--- Comment #5 from Sainan <sainan+gcc.bugzilla at calamity dot gg> ---
(In reply to Wilco from comment #4)
> The atomic will also set correct struct alignment.

My thinking was that maybe this is not the case (= standard library issue)
since both GCC and Clang seem to be causing this issue, but manually adding
alignas(16) also didn't help.

> You would get a crash if you build for LSE so you get a LDADDAL instruction
> and then run it on a CPU that doesn't. So try -mcpu=native and it should
> work.

-mcpu=native didn't fix the SIGBUS, only removed __aarch64_ldadd4_acq_rel from
the stack trace.

FWIW, the CPU on this system where I get the SIGBUS is Cortex-A76, which should
support LSE and atomics, but it seems everytime it encounters atomics, it just
throws a SIGBUS. It works fine on Snapdragon 8cx Gen 3.

[Bug middle-end/110773] [Aarch64] crash (SIGBUS) due to atomic instructions on under-aligned memory

[wilco at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110773

--- Comment #6 from Wilco <wilco at gcc dot gnu.org> ---
(In reply to Sainan from comment #5)
> (In reply to Wilco from comment #4)
> > The atomic will also set correct struct alignment.
> 
> My thinking was that maybe this is not the case (= standard library issue)
> since both GCC and Clang seem to be causing this issue, but manually adding
> alignas(16) also didn't help.
> 
> > You would get a crash if you build for LSE so you get a LDADDAL instruction
> > and then run it on a CPU that doesn't. So try -mcpu=native and it should
> > work.
> 
> -mcpu=native didn't fix the SIGBUS, only removed __aarch64_ldadd4_acq_rel
> from the stack trace.
> 
> FWIW, the CPU on this system where I get the SIGBUS is Cortex-A76, which
> should support LSE and atomics, but it seems everytime it encounters
> atomics, it just throws a SIGBUS. It works fine on Snapdragon 8cx Gen 3.

That does not make any sense. The only thing I think might happen is that your
structure is not correctly aligned (for example by using a custom memory
allocator). Can you check the address of count when it fails? (should be in the
crash logs, or you can see it in gdb or just printf it).

[Bug middle-end/110773] [Aarch64] crash (SIGBUS) due to atomic instructions on under-aligned memory

[sainan+gcc.bugzilla at calamity dot gg]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110773

--- Comment #7 from Sainan <sainan+gcc.bugzilla at calamity dot gg> ---
(In reply to Wilco from comment #6)
> That does not make any sense. The only thing I think might happen is that
> your structure is not correctly aligned (for example by using a custom
> memory allocator). Can you check the address of count when it fails? (should
> be in the crash logs, or you can see it in gdb or just printf it).

I feel silly for not thinking of printing the address, but now that I did, I
see the final hexit is '9' and so it just so happens this CPU can't deal with
that...

[Bug middle-end/110773] [Aarch64] crash (SIGBUS) due to atomic instructions on under-aligned memory

[wilco at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110773

--- Comment #8 from Wilco <wilco at gcc dot gnu.org> ---
(In reply to Sainan from comment #7)
> (In reply to Wilco from comment #6)
> > That does not make any sense. The only thing I think might happen is that
> > your structure is not correctly aligned (for example by using a custom
> > memory allocator). Can you check the address of count when it fails? (should
> > be in the crash logs, or you can see it in gdb or just printf it).
> 
> I feel silly for not thinking of printing the address, but now that I did, I
> see the final hexit is '9' and so it just so happens this CPU can't deal
> with that...

So it's unaligned then, and that's not supported. And you're lucky your
specific alignment happens to work on v8.4 cores - it would fail for other
offsets.

[Bug middle-end/110773] [Aarch64] crash (SIGBUS) due to atomic instructions on under-aligned memory

[sainan+gcc.bugzilla at calamity dot gg]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110773

--- Comment #9 from Sainan <sainan+gcc.bugzilla at calamity dot gg> ---
(In reply to Wilco from comment #8)
> So it's unaligned then, and that's not supported. And you're lucky your
> specific alignment happens to work on v8.4 cores - it would fail for other
> offsets.

I'd say unlucky rather because I was going crazy about not being able to
reproduce this on the Windows ARM machine.