-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Backports for julia 1.11.7 #59336
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
Backports for julia 1.11.7 #59336
Conversation
…54840) Like #54671, but for `speccache_eq`. Saw another segfault with this in the stack trace, hence this fix. I also looked for other uses of `jl_smallintset_lookup` and there's one in `idset.c`. That doesn't appear to be racy but I'm not familiar with the code, so maybe you can take a look at it in case we need to push a fix for that one too @gbaraldi or @vtjnash? (cherry picked from commit dd1ed17)
This was in DAECompiler.jl code found by @serenity4. He also mentioned that writing up how one might go and fix a bug like this so i'll give a quick writeup (this was a very simple bug so it might not be too interesting) The original crash which looked something like > %19 = alloca [10 x i64], align 8 %155 = insertelement <4 x ptr> poison, ptr %19, i32 0 Unexpected instruction > [898844] signal 6 (-6): Aborted in expression starting at /home/gbaraldi/DAECompiler.jl/test/reflection.jl:28 pthread_kill at /lib/x86_64-linux-gnu/libc.so.6 (unknown line) gsignal at /lib/x86_64-linux-gnu/libc.so.6 (unknown line) abort at /lib/x86_64-linux-gnu/libc.so.6 (unknown line) RecursivelyVisit<llvm::IntrinsicInst, LateLowerGCFrame::PlaceRootsAndUpdateCalls(llvm::ArrayRef<int>, int, State&, std::map<llvm::Value*, std::pair<int, int> >)::<lambda(llvm::AllocaInst*&)>::<lambda(llvm::Use&)> > at /home/gbaraldi/julia4/src/llvm-late-gc-lowering.cpp:803 operator() at /home/gbaraldi/julia4/src/llvm-late-gc-lowering.cpp:2560 [inlined] PlaceRootsAndUpdateCalls at /home/gbaraldi/julia4/src/llvm-late-gc-lowering.cpp:2576 runOnFunction at /home/gbaraldi/julia4/src/llvm-late-gc-lowering.cpp:2638 run at /home/gbaraldi/julia4/src/llvm-late-gc-lowering.cpp:2675 run at /home/gbaraldi/julia4/usr/include/llvm/IR/PassManagerInternal.h:91 which means it was crashing inside of late-gc-lowering, so the first thing I did was ran julia and the same test with LLVM_ASSERTIONS=1 and FORCE_ASSERTIONS=1 to see if LLVM complained about a malformed module, and both were fine. Next step was trying to get the failing code out for inspection. Easiest way is to do `export JULIA_LLVM_ARGS="--print-before=LateLowerGCFrame --print-module-scope"` and pipe the output to a file. The file is huge, but since it's a crash in LLVM we know that the last thing is what we want, and that gave me the IR I wanted. To verify that this is failing I did `make -C src install-analysis-deps` to install the LLVM machinery (opt...). That gets put in the `tools` directory of a julia build. Then I checked if this crashed outside of julia by doing `./opt -load-pass-plugin=../lib/libjulia-codegen.dylib --passes=LateLowerGCFrame -S test.ll -o tmp3.ll `. This is run from inside the tools dir so your paths might vary (the -S is so LLVM doesn't generate bitcode) and my code did crash, however it was over 500 lines of IR which makes it harder to debug and to write a test. Next step then is to minimize the crash by doing [`llvm-reduce`](https://llvm.org/docs/CommandGuide/llvm-reduce.html) over it (it's basically creduce but optimized for LLVM IR) which gave me a 2 line reproducer (in this case apparently just having the insertelement was enough for the pass to fail). One thing to be wary is that llvm-reduce will usually make very weird code, so it might be useful to modify the code slightly so it doesn't look odd (it will have unreachable basic-blocks and such). After the cleanup fixing the bug here wasn't interesting but this doesn't apply generally. And also always transform your reduced IR into a test to put in llvmpasses. (cherry picked from commit 906d348)
When this API was added, this function inlined, which is important, because the API relies on the allocation of the `Ref` being elided. At some point (I went back to 1.8) this regressed. For example, it is currently responsible for substantially all non-Expr allocations in JuliaParser. Before (parsing all of Base with JuliaParser): ``` │ Memory estimate: 76.93 MiB, allocs estimate: 719922. ``` After: ``` │ Memory estimate: 53.31 MiB, allocs estimate: 156. ``` Also add a test to make sure this doesn't regress again. (cherry picked from commit d6294ba)
Co-authored-by: Jameson Nash <[email protected]> (cherry picked from commit 3ed13ea)
Currently, `similar(::CodeUnits)` works as expected by going through the generic `AbstractArray` method. However, the fallback method hit by `similar(::Type{<:CodeUnits}, dims)` does not work, as it assumes the existence of a constructor that accepts an `UndefInitializer`. This can be made to work by defining a corresponding `similar` method that returns an `Array`. One could make a case that this is a bugfix since it was arguably a bug that this method didn't work given that `CodeUnits` is an `AbstractArray` subtype and the other `similar` methods work. If anybody buys that argument, it could be nice to backport this; it came up in some internal code that uses Arrow.jl and JSON3.jl together. (cherry picked from commit 8e524c7)
would fix #57170, fix #54623 @nanosoldier `runbenchmarks("array", vs=":master")` (cherry picked from commit e853a4f)
(cherry picked from commit c0cc1e1)
(cherry picked from commit 2349f43)
(cherry picked from commit 107e1ac)
(cherry picked from commit fde79f5)
(cherry picked from commit 1f6eff1)
I have a Distributed.jl bugfix to backport, but I first need to do the backport in the Distributed repo, and then I'll bump Distributed here. |
…release-julia-1.11` (but keep the commit the same) (#59390) This PR will be followed up with a stdlib bump with the Distributed backports.
…o e9b9023 (#59391) Stdlib: Distributed URL: https://github.com/JuliaLang/Distributed.jl Stdlib branch: release-julia-1.11 Julia branch: backports-release-1.11 Old commit: 6c7cdb5 New commit: e9b9023 Julia version: 1.11.5 Distributed version: 1.11.0(Does not match) Bump invoked by: @DilumAluthge Powered by: [BumpStdlibs.jl](https://github.com/JuliaLang/BumpStdlibs.jl) Diff: JuliaLang/Distributed.jl@6c7cdb5...e9b9023 ``` $ git log --oneline 6c7cdb5..e9b9023 e9b9023 Backports for Julia 1.11.x (#141) ``` Co-authored-by: DilumAluthge <[email protected]>
Okay, I've pushed two commits to this branch:
@KristofferC Should I edit the PR description manually and add these two PRs to the list? Or would that break the backporter script? |
We should get the openblas patch in: #59346 |
CI is currently failing on this PR, because of segfaults in multiple test sets. Here is a MWE that reproduces the segfault: test_list = [
"ambiguous",
]
Base.runtests(test_list) With that MWE, bisect blames 3b04664 (which backports #58837):
Here are the details of the bisect (click to expand):I ran the bisect on Linux x86_64. My starting good commit was: 9615af0 My starting bad commit was: 562699a Here is my #!/usr/bin/env bash
set -euf -o pipefail
which -a ccache
make cleanall
make distclean
rm -f Make.user
echo "USECCACHE=1" >> Make.user
make -j16
./julia ../test.jl Here are the contents of the test_list = [
"ambiguous",
]
Base.runtests(test_list) The
|
I've confirmed that reverting 3b04664 locally fixes my MWE. |
Okay, we are very close to green CI on this PR now. The last failure is |
Alright, I'll wait for CI to finish (just to double-check that it is all green now), and then I'll kick off PkgEval. |
Much of CI is green now, including the previously-failing |
@nanosoldier |
CI is all green now. |
Hm, I don't see the nanosoldier notification showing that it is running. |
@nanosoldier |
The package evaluation job you requested has completed - possible new issues were detected. Report summary❗ Packages that crashed3 packages crashed only on the current version.
19 packages crashed on the previous version too. ✖ Packages that failed33 packages failed only on the current version.
3109 packages failed on the previous version too. ✔ Packages that passed tests40 packages passed tests only on the current version.
6417 packages passed tests on the previous version too. ➖ Packages that were skipped altogether1 packages were skipped only on the current version.
1310 packages were skipped on the previous version too. |
@nanosoldier |
The package evaluation job you requested has completed - possible new issues were detected. Report summary✖ Packages that failed2 packages failed only on the current version.
3 packages failed on the previous version too. ✔ Packages that passed tests2 packages passed tests only on the current version.
27 packages passed tests on the previous version too. |
@KristofferC The JuliaInterpreter failure looks real?
|
Nah, it just that test didn't run at all on the baseline due to: |
Ah cool. From the log, that looks like the only JuliaInterpreter test that failed, so presumably I can ignore JuliaInterpreter? |
Okay, so if we ignore JuliaInterpreter, then the only remaining PkgEval failure is ONSAS.jl, which failed with one error:
|
I'll try it again, just to make sure it still reproduces. |
@nanosoldier |
Okay, so locally I can reproduce the ONSAS.jl failure (an
However, the failure is non-deterministic. When I run my reproducer1 multiple times in a row, sometimes it passes and sometimes it fails.
Therefore, I think that this is a non-deterministic failure that already exists on I've opened mvanzulli/ONSAS.jl#537. Footnotes
|
To summarize the current status of PkgEval on this PR: There are currently two packages failing:
Therefore, my conclusion is that PkgEval looks good on this PR. |
The package evaluation job you requested has completed - no new issues were detected. Report summary✔ Packages that passed tests1 packages passed tests on the previous version too. |
Ugh this defaulted to squash-merge, which isn't correct. That's my bad. We want to regular merge backports PRs, so that people can bisect on the release branch. I'll fix this. |
Merge branch `backports-release-1.11` into `release-1.11` #59336
I've fixed this. It's now a regular merge commit (not a squash merge). This will allow people to bisect on the |
Backported PRs:
using Foo: Foo, ...
in package code (was: "using considered harmful") #42080Ptr
values static-show w/ type-information #58584similar
method forType{<:CodeUnits}
#57826Need manual backport:
@time_imports
@trace_compile
@trace_dispatch
#58011exp_impl
topow
#58062one
for theSizedArray
test helper #58209Core
methods from newly-inferred list #58510IteratorSize
method forGenerator
#58110hygienic-scope
s in inner macro expansions #58965Contains multiple commits, manual intervention needed:
struct
to hard #56755--project=@script
when outside script directory #56351jl_type_hash
is concrete evaluated) #58401Non-merged PRs with backport label:
--trace-compile
#58535transcode
: prevent Windows sysimage invalidation #58038@nospecialize
forstring_index_err
#57604displaysize
to theIOContext
used by the REPL #55499isfile_casesensitive
fixes on Windows #55220@inbounds
andBase.@propagate_inbounds
#50157