Conversation
|
Oh, cool, with the changes in #59946 temporarily incorporated here, for the platforms where compilation is successful (macos and freebsd) tests are already passing, that's encouraging. |
|
The compile error on linux seems to be a Werror for uninitialized variable in LLVM. It could easily be a false positive since it's in the part of the code where LLVM does it's trick with memory layout of objects... |
|
I think was fixed by llvm/llvm-project#177168, so we'll have to backport that PR to our fork (we want to build a newer patch version with other patches anyway) |
|
Windows build fails with I'll need help for that one. |
|
32-bit Windows somehow builds successfully, but then there's an interesting test failure Lines 664 to 669 in 3360c8c is that an ABI problem with Int128 on 32-bit systems? |
|
@xal-0 |
|
After Sam's fix to the 64-bit Windows build, its tests are successful 🥳 Ok, I guess now it's time to start a new rebuild of LLVM in Yggdrasil |
https://buildkite.com/julialang/julia-master/builds/55880#019d1a76-6b59-47af-badc-653ea1406ce2/L1440 |
|
We're still getting despite backporting llvm/llvm-project#177168, which puzzles me quite a bit. But also, I can't reproduce the warning/error with GCC 15. Edit: I can't reproduce the error with |
|
I'm temporarily (?) disabling the maybe uninitialized warning for the sake of moving forward because I don't know what to do with it (I can't even reproduce it with gcc from 9 to 15). All 64-bit platforms are looking good, both 32-bit platforms are sad about ccalls involving Int128 (incorrect result on windows, segfault in Linux). Analyzegc needs adapting to new clang API |
LLVM 21 replaced the `Stmt*` parameter in `SValBuilder::conjureSymbolVal` with `ConstCFGElementRef` (llvm/llvm-project#128251). Update all four call sites in GCChecker.cpp to use `C.getCFGElementRef()` instead of passing `Expr*` pointers directly. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
|
My pattern-matching-powered fix to the analyzegc build failure seems to have worked in the sense that now there are actual errors reported by the analysis: I'll let someone else have fun fixing them 🙃 Edit: my friendly assistant proposed the following patch diff --git a/src/Makefile b/src/Makefile
index 2f12ff8d80..11c5df6634 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -642,6 +642,8 @@ SA_EXCEPTIONS-jloptions.c := -Xanalyzer -analyzer-config -Xana
# clang doesn't understand that e->vars has the same value in save_env (NULL) and restore_env (assumed non-NULL)
SA_EXCEPTIONS-subtype.c := -Xanalyzer -analyzer-config -Xanalyzer silence-checkers="core.uninitialized.Assign;core.UndefinedBinaryOperatorResult"
SA_EXCEPTIONS-codegen.c := -Xanalyzer -analyzer-config -Xanalyzer silence-checkers="core"
+SA_EXCEPTIONS-gc-stock.c := -Xanalyzer -analyzer-config -Xanalyzer silence-checkers="core.FixedAddressDereference"
+SA_EXCEPTIONS-aotcompile.cpp := -Xanalyzer -analyzer-config -Xanalyzer silence-checkers="core.FixedAddressDereference"
# these need to be annotated (and possibly fixed)
SKIP_GC_CHECK := codegen.cpp rtutils.c
diff --git a/src/aotcompile.cpp b/src/aotcompile.cpp
index 1fbfd459fc..96220989fa 100644
--- a/src/aotcompile.cpp
+++ b/src/aotcompile.cpp
@@ -341,7 +341,10 @@ public:
jl_value_t *rval = jl_idset_get(list, keyset, val);
if (rval == NULL) {
ssize_t idx;
+ JL_GC_PROMISE_ROOTED(list);
list = jl_idset_put_key(list, val, &idx);
+ JL_GC_PROMISE_ROOTED(list);
+ JL_GC_PROMISE_ROOTED(keyset);
keyset = jl_idset_put_idx(list, keyset, idx);
}
}but I can't really judge it, so I'm not even trying to push these changes. |
LLVM 21 (llvm/llvm-project#138092) aligns i128 to 16 bytes when passing on the x86-32 stack. However, GCC does not support __int128 on 32-bit x86 at all, so C libraries use struct { int64_t, int64_t } which only has 4-byte stack alignment. This mismatch caused segfaults on Linux and incorrect results on Windows for ccalls involving Int128. Fix by using [4 x i32] instead of i128 as the byval type for large integer arguments, which naturally has 4-byte alignment matching the C ABI. Also fix abi_win32.cpp to pass large primitive types like Int128 by reference (byval) instead of directly, avoiding the same alignment issue. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
All tests green, yay! Now only analyzegc is unhappy. |
Add JL_GC_PROMISE_ROOTED annotations in egal_set::insert() for val, list, and keyset. These class members are rooted by the caller via JL_GC_PUSH3, but the GC checker cannot track rooting through class member indirection. The post-put_key annotation on list is needed because the returned pointer is a new value assigned to the already- pushed member address. Suppress optin.core.FixedAddressDereference false positives in llvm-alloc-opt.cpp (LLVM ilist_node_base.h internals) and gc-stock.c (gc_read_stack bounds-checked address) — this checker is new in LLVM 21. Co-authored-by: Claude <noreply@anthropic.com>
|
@nanosoldier |
|
@maleadt What's up with nanosoldier? |
|
Down until further notice. |
|
@nanosoldier |
|
The package evaluation job you requested has completed - possible new issues were detected. Report summary❗ Packages that crashed5 packages crashed only on the current version.
458 packages crashed on the previous version too. ✖ Packages that failed83 packages failed only on the current version.
3904 packages failed on the previous version too. ✔ Packages that passed tests7 packages passed tests only on the current version.
5797 packages passed tests on the previous version too. ➖ Packages that were skipped altogether1409 packages were skipped on the previous version too. |
|
Of the 83 failing packages, I think
are just packages depending on LLVM.jl which doesn't support LLVM 21 yet? |
|
@nanosoldier |
|
Your job failed. Consult the server logs for more details. |
I won't be able to work on this besides for rebasing the PR and other small edits, I'm opening this PR to keep the ball rolling, but everyone is more than welcome to take it over.
This needs #59946 to start with.