Make ptls allocations at least 128 byte aligned

[gbaraldi]

Fixes #54560 (comment)

[d-netto]

Still not clear to me why alignment issues could be causing a bug which manifests on multithreaded Julia, but not single-threaded one -- #57309 (comment).

[d-netto]

Also, could you test this on the same architecture + OS the user specified on #54560 to check whether it fixes the issue there?

[d-netto]

Requesting this because I tested on a bunch of processor models which were not AMD Ryzen 9 (what the user has), but couldn't reproduce in any of them...

[gbaraldi]

The user mentioned this made it work on his machine :\

[LebedevRI]

Requesting this because I tested on a bunch of processor models which were not AMD Ryzen 9 (what the user has), but couldn't reproduce in any of them...

But was that tested on the same distro, and same CPU architecture at least?
(FYI this is the first time i'm reading that it was tested this extensively)

Testing this PR too...

[LebedevRI]

The user mentioned this made it work on his machine :\

For reference, the patch disscussed in Slack did:

diff --git a/src/threading.c b/src/threading.c
index d8b6b784c6..f3d289f310 100644
--- a/src/threading.c
+++ b/src/threading.c
@@ -346,7 +346,8 @@ jl_ptls_t jl_init_threadtls(int16_t tid)
 #endif
     if (jl_get_pgcstack() != NULL)
         abort();
-    jl_ptls_t ptls = (jl_ptls_t)calloc(1, sizeof(jl_tls_states_t));
+    jl_ptls_t ptls = (jl_ptls_t)aligned_alloc(alignof(jl_tls_states_t), sizeof(jl_tls_states_t));
+    memset(ptls, 0, sizeof(jl_tls_states_t));
 #ifndef _OS_WINDOWS_
     pthread_setspecific(jl_task_exit_key, (void*)ptls);
 #endif

[d-netto]

But was that tested on the same distro, and same CPU architecture at least?
(FYI this is the first time i'm reading that it was tested this extensively)

No, that's the point. We didn't catch this on CI and I didn't catch this on a few aarch64 and a few Intel x86_64, so that's why I'm requesting to check in this particular platform, which seems to be one of the few in which the bug manifests -- but that seems to be addressed by your comment already.

[LebedevRI]

But was that tested on the same distro, and same CPU architecture at least?
(FYI this is the first time i'm reading that it was tested this extensively)

No, that's the point. We didn't catch this on CI and I didn't catch this on a few aarch64 and a few Intel x86_64, so that's why I'm requesting to check in this particular platform, which seems to be one of the few in which the bug manifests -- but that seems to be addressed by your comment already.

(compile running...)
Well, i would guess that it doesn't need to be an actual same processor,
any Ryzen would do i would think, and honestly even an intel CPU would do.
If libc has different default alignments, then it would be based on CPU vector width or something.
I would think the distro mismatch may be a bigger issue.

[LebedevRI]

Ok, i CAN confirm that this PR fixes segfault i was seeing.
I do NOT know if this only hides the problem, or actually fixes it, or which change in master hid it previously.

@gbaraldi Thank you!

[d-netto]

Just to confirm: you cherry-picked this patch to 1.11 and tested it there, right? (Note this PR is against master, and the issue was not present there -- #54560 (comment)).

[LebedevRI]

Just to confirm: you cherry-picked this patch to 1.11 and tested it there, right? (Note this PR is against master, and the issue was not present there -- #54560 (comment)).

Yes, i checkout'ed v1.11.3, built it, checked that the segfault is still there,
then applied patch, built it, and tested to no segfault.
I just successfully re-did that experiment.

[d-netto]

Perfect, thanks.

[LebedevRI]

Just to repeat from Slack, it would be really good to know whether or not UBSan catches this issue.
(And if it does not, i would really love to have a reproducer, please)

[Keno]

We should audit other uses of our allocators and replace them with a version that static_asserts the alignment requirement.

[gbaraldi]

We should default to 16 byte alignment everywhere since that's what our GC promises to be consistent