Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jitlayers: replace sharedbytes intern pool with one that respects alignment #52182

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

vtjnash
Copy link
Sponsor Member

@vtjnash vtjnash commented Nov 15, 2023

The llvm optimizations may increase alignment beyond the initial MAX_ALIGN. This pool's alignment was previously only sizeof(struct { atomic<int> RefCount; size_t Length; char Data[]; }) however, potentially resulting in segfaults at runtime.

Fixes #52118. Should make CI much happier.

@vtjnash vtjnash added compiler:codegen Generation of LLVM IR and native code backport 1.10 Change should be backported to the 1.10 release labels Nov 15, 2023
@vtjnash vtjnash force-pushed the jn/52118 branch 2 times, most recently from 9d90c00 to 8cb8a12 Compare November 16, 2023 17:07
@vtjnash vtjnash added the status:merge me PR is reviewed. Merge when all tests are passing label Nov 16, 2023
@giordano
Copy link
Contributor

Asan is unhappy:

    JULIA tmp/test-asan/asan/usr/lib/julia/corecompiler.ji
julia-debug: /cache/build/builder-amdci5-1/julialang/julia-master/src/jitlayers.cpp:1469: llvm::Constant *(anonymous namespace)::JITPointersT::getSharedBytes(llvm::GlobalVariable &): Assertion `llvm::isAddrAligned(Preferred, Interned.data())' failed.
[16466] signal 6 (-6): Aborted
in expression starting at boot.jl:263
gsignal at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
abort at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
unknown function (ip: 0x7f16da65540e)
__assert_fail at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
getSharedBytes at /cache/build/builder-amdci5-1/julialang/julia-master/src/jitlayers.cpp:1469
operator() at /cache/build/builder-amdci5-1/julialang/julia-master/src/jitlayers.cpp:1420
withModuleDo<(lambda at /cache/build/builder-amdci5-1/julialang/julia-master/src/jitlayers.cpp:1417:30)> at /cache/build/builder-amdci5-1/julialang/julia-master/tmp/test-asan/asan/usr/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h:136
operator() at /cache/build/builder-amdci5-1/julialang/julia-master/src/jitlayers.cpp:1417
CallImpl<(anonymous namespace)::JITPointersT> at /cache/build/builder-amdci5-1/julialang/julia-master/tmp/test-asan/asan/usr/include/llvm/ADT/FunctionExtras.h:222
[...]

…gnment

The llvm optimizations may increase alignment beyond the initial
MAX_ALIGN. This pool's alignment was previously only
`sizeof(struct { atomic<int> RefCount; size_t Length; char Data[]; })`
however, potentially resulting in segfaults at runtime.

Fixes #52118
@DilumAluthge DilumAluthge merged commit a65bc9a into master Nov 17, 2023
7 checks passed
@DilumAluthge DilumAluthge deleted the jn/52118 branch November 17, 2023 02:55
vtjnash added a commit that referenced this pull request Nov 17, 2023
@giordano giordano removed the status:merge me PR is reviewed. Merge when all tests are passing label Nov 20, 2023
@KristofferC KristofferC mentioned this pull request Nov 27, 2023
39 tasks
@KristofferC
Copy link
Sponsor Member

Needs a manual backport to 1.10. @vtjnash

vtjnash added a commit that referenced this pull request Nov 28, 2023
…gnment (#52182)

The llvm optimizations may increase alignment beyond the initial
MAX_ALIGN. This pool's alignment was previously only `sizeof(struct {
atomic<int> RefCount; size_t Length; char Data[]; })` however,
potentially resulting in segfaults at runtime.

Fixes #52118. Should make CI much happier.

(cherry picked from commit a65bc9a)
vtjnash added a commit that referenced this pull request Nov 28, 2023
@vtjnash vtjnash removed the backport 1.10 Change should be backported to the 1.10 release label Nov 28, 2023
@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Nov 28, 2023

Done and pushed to backports-release-1.10

KristofferC added a commit that referenced this pull request Dec 2, 2023
Backported PRs:
- [x] #51213 <!-- Wait for other threads to finish compiling before
exiting -->
- [x] #51520 <!-- Make allocopt respect the GC verifier rules with non
usual address spaces -->
- [x] #51598 <!-- Use a simple error when reporting sysimg load
failures. -->
- [x] #51757 <!-- fix parallel peakflop usage -->
- [x] #51781 <!-- Don't make pkgimages global editable -->
- [x] #51848 <!-- allow finalizers to take any locks and yield during
exit -->
- [x] #51847 <!-- add missing wait during Timer and AsyncCondition close
-->
- [x] #50824 <!-- Add some aliasing warnings to docstrings for mutating
functions in Base -->
- [x] #51885 <!-- remove chmodding the pkgimages -->
- [x] #50207 <!-- [devdocs] Improve documentation about building
external forks of LLVM -->
- [x] #51967 <!-- further fix to the new promoting method for
AbstractDateTime subtraction -->
- [x] #51980 <!-- macroexpand: handle const/atomic struct fields
correctly -->
- [x] #51995 <!-- [Artifacts] Pass artifacts dictionary to
`ensure_artifact_installed` dispatch -->
- [x] #52098 <!-- Fix errors in `sort` docstring -->
- [x] #52136 <!-- Bump JuliaSyntax to 0.4.7 -->
- [x] #52140 <!-- Make c func `abspath` consistent on Windows. Fix
tracking path conversion. -->
- [x] #52009 <!-- fix completion that resulted in startpos of 0 for `\\
-->
- [x] #52192 <!-- cap the number of GC threads to number of cpu cores
-->
- [x] #52206 <!-- Make have_fma consistent between interpreter and
compiled -->
- [x] #52027 <!-- fix Unicode.julia_chartransform for Julia 1.10 -->
- [x] #52217 <!-- More helpful error message for empty `cpu_target` in
`Base.julia_cmd` -->
- [x] #51371 <!-- Memoize `cwstring` when used for env lookup /
modification on Windows -->
- [x] #52214 <!-- Turn Method Overwritten Error into a PrecompileError
-- turning off caching -->
- [x] #51895 <!-- Devdocs on fixing precompile hangs, take 2 -->
- [x] #51596 <!-- Reland "Don't mark nonlocal symbols as hidden"" -->
- [x] #51834 <!-- [REPLCompletions] allow symbol completions within
incomplete macrocall expression -->
- [x] #52010 <!-- Revert "Support sorting iterators (#46104)" -->
- [x] #51430 <!-- add support for async backtraces of Tasks on any
thread -->
- [x] #51471 <!-- Fix segfault if root task is NULL -->
- [x] #52194 <!-- Fix multiversioning issues caused by the parallel llvm
work -->
- [x] #51035 <!-- refactor GC scanning code to reflect jl_binding_t are
now first class -->
- [x] #52030 <!-- Bump Statistics -->
- [x] #52189 <!-- codegen: ensure i1 bool is widened to i8 before
storing -->
- [x] #52228 <!-- Widen diagonal var during `Type` unwrapping in
`instanceof_tfunc` -->
- [x] #52182 <!-- jitlayers: replace sharedbytes intern pool with one
that respects alignment -->

Contains multiple commits, manual intervention needed:
- [ ] #51092 <!-- inference: fix bad effects for recursion -->

Non-merged PRs with backport label:
- [ ] #52196 <!-- Fix creating custom log level macros -->
- [ ] #52170 <!-- fix invalidations related to `ismutable` -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:codegen Generation of LLVM IR and native code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

reinterpret arrays segfaults on CI
5 participants