Make ptls allocations at least 128 byte aligned#57310
Conversation
|
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). |
|
Also, could you test this on the same architecture + OS the user specified on #54560 to check whether it fixes the issue there? |
|
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... |
|
The user mentioned this made it work on his machine :\ |
But was that tested on the same distro, and same CPU architecture at least? Testing this PR too... |
For reference, the patch disscussed in Slack did: |
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...) |
|
Ok, i CAN confirm that this PR fixes segfault i was seeing. @gbaraldi Thank you! |
|
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 |
|
Perfect, thanks. |
|
Just to repeat from Slack, it would be really good to know whether or not UBSan catches this issue. |
Keno
left a comment
There was a problem hiding this comment.
We should audit other uses of our allocators and replace them with a version that static_asserts the alignment requirement.
|
We should default to 16 byte alignment everywhere since that's what our GC promises to be consistent |
e100582 to
1761f63
Compare
Fixes #54560 (comment) (cherry picked from commit 79e98e3)
Backported PRs: - [x] #57142 <!-- Add reference to time_ns in time --> - [x] #57241 <!-- Handle `waitpid` race condition when `SIGCHLD` is set to `SIG_IGN` --> - [x] #57249 <!-- restore non-freebsd-unix fix for profiling --> - [x] #57211 <!-- Ensure read/readavailable for BufferStream are threadsafe --> - [x] #57262 <!-- edit NEWS for v1.12 --> - [x] #57226 <!-- cfunction: reimplement, as originally planned, for reliable performance --> - [x] #57253 <!-- bpart: Fully switch to partitioned semantics --> - [x] #57273 <!-- fix "Right arrow autocompletes at line end" implementation --> - [x] #57280 <!-- dep: Update JuliaSyntax --> - [x] #57229 <!-- staticdata: Close data race after backedge insertion --> - [x] #57298 <!-- Updating binding version to fix MMTk CI --> - [x] #57248 <!-- improve concurrency safety for `Compiler.finish!` --> - [x] #57312 <!-- Profile.print: de-focus sleeping frames as gray --> - [x] #57289 <!-- Make `OncePerX` subtype `Function` --> - [x] #57310 <!-- Make ptls allocations at least 128 byte aligned --> - [x] #57311 <!-- Add a warning for auto-import of types --> - [x] #57338 <!-- fix typo in Float32 random number generation --> - [x] #57293 <!-- Fix getfield_tfunc when order or boundscheck is Vararg --> - [x] #57349 <!-- docs: fix-up world-age handling for META access --> - [x] #57344 <!-- Add missing type asserts when taking the queue out of the task struct --> - [x] #57348 <!-- 🤖 [master] Bump the SparseArrays stdlib from 212981b to 72c7cac --> - [x] #55040 <!-- Allow macrocall as function sig --> - [x] #57299 <!-- Add missing latestworld after parameterized type alias -->
Fixes #54560 (comment) (cherry picked from commit 79e98e3)
Fixes #54560 (comment) (cherry picked from commit 79e98e3)
Fixes #54560 (comment)