-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Update Clang toolchain from 18.0.0 to 18.1.8 #12077
Conversation
@maflcko you mentioned in #11626 that before doing this all existing projects should be un-pinned from their Also, do you know of a way to more easily enumerate the projects which break as a result of this upate? I probably can't feasibly build everything locally. If CI takes care of this though I can also just wait for that. |
Also, for reference, I've confirmed that by layering #12075 on top of this Rust projects no longer have any warnings in coverage builds and coverage looks like it might work. |
Also, following up from your comment here you mentioned that pinned projects might break since they're using clang 15. I think though that the decoding of coverage data supports older versions, just not newer versions, so my assumption would be that LLVM 18 tooling would be able to decode clang 15-generated coverage information. I don't have data to back up this assumption, however. |
/gcbrun trial_build.py all |
Yes, because the coverage container uses the current llvm to parse the coverage profile (regardless of what the project uses), but if the profile was generated with llvm-15 (pinned projects) it will fail. It should be possible to observe this in the trial build. |
In theory, yes, earlier coverage profiles can be read. However, the raw profile version is a separate versioning, and a breaking change every time, as far as I understood it. |
@@ -50,7 +50,7 @@ LLVM_DEP_PACKAGES="build-essential make ninja-build git python3 python3-distutil | |||
apt-get update && apt-get install -y $LLVM_DEP_PACKAGES --no-install-recommends | |||
|
|||
# For manual bumping. | |||
OUR_LLVM_REVISION=llvmorg-18-init-4631-gd50b56d1 | |||
OUR_LLVM_REVISION=llvmorg-18.1.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unrelated nit: Could add a comment to check if CMAKE_VERSION
is up-to-date? Otherwise, a future bump could cause issues, because cmake may not properly detect the newer llvm version. Most recently this happened for 32-bit builds.
Generally, I'd say the two should be bumped at the same time to avoid build issues in projects down the line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved as part of #12083
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I'll admit I wasn't sure what CMAKE_VERSION
was referring to here so assistance is appreciated!
Thanks for the links! I'll start reviewing those hopefully soon. Also I was having a tough time understanding what you were referring to before about LLVM 15 breaking. I thought that because the current coverage container was using LLVM 18 that updating it to LLVM 18.1 wouldn't be an issue because it would already be broken with LLVM 15 coverage information. Upon checking though it looks like the version in the LLVM source for profiling was "8" both with LLVM 15 and LLVM 18.0.0. The first change happened with LLVM 18.1 so that also makes more sense to me. I'll try to dig in further to the failure logs and see what the impact of this is. Also I'll note I'm happy to rebase around other changes, so please don't block on me for anything. |
The failures should all be related to the raw coverage profile version in some way or another. I don't see another way other than to atomically and globally bump the coverage version for all projects and all languages. But that requires the projects to be un-pinned, and a rust-nightly bump to be combined into this pull. My recommendation would be to change #12075 to nightly-2024-02-12 for now, then wait for it to land and then bump |
Ok I've gone through many of the failures and I'm sort of quite new to updating the toolchain here so I wanted to ask a few questions. I've tried to bucket all the various failure logs into a few categories:
My main question is how to handle most of these. Two action items for this PR are to update Rust in this PR and update Swift as well. Everything else though I'm less certain about. For example resolving new Clang errors will require source changes. I tested a few of the There's still other failures I don't fully understand which I'm not sure if y'all would recognize or not |
Oh and one final category of failures I forgot to mention are those that failed to build but also failed to build according to their latest status, so I ignored a few builds like that. |
They are expected, I think, and can be ignored for now, because the infra check does not spin up arm64 machines. You can use
You will have to rebase or merge with master before the trial build. Otherwise the changes (2c03690) aren't picked up. |
If more than one project is affected by a build warning, you can soften it. For example: diff --git a/infra/base-images/base-clang/Dockerfile b/infra/base-images/base-clang/Dockerfile
index f61b85443..c82ed1008 100644
--- a/infra/base-images/base-clang/Dockerfile
+++ b/infra/base-images/base-clang/Dockerfile
@@ -58,9 +58,9 @@ ENV CCC "clang++"
# The implicit-function-declaration and implicit-int errors are downgraded to a
# warning, to allow compiling legacy code.
# See https://releases.llvm.org/16.0.0/tools/clang/docs/ReleaseNotes.html#potentially-breaking-changes
-# Same for deprecated-declarations, int-conversion,
+# Same for vla-cxx-extension, deprecated-declarations, int-conversion,
# incompatible-function-pointer-types, enum-constexpr-conversion
-ENV CFLAGS "-O1 -fno-omit-frame-pointer -gline-tables-only -Wno-error=enum-constexpr-conversion -Wno-error=incompatible-function-pointer-types -Wno-error=int-conversion -Wno-error=deprecated-declarations -Wno-error=implicit-function-declaration -Wno-error=implicit-int -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION"
+ENV CFLAGS "-O1 -fno-omit-frame-pointer -gline-tables-only -Wno-error=enum-constexpr-conversion -Wno-error=incompatible-function-pointer-types -Wno-error=int-conversion -Wno-error=deprecated-declarations -Wno-error=vla-cxx-extension -Wno-error=implicit-function-declaration -Wno-error=implicit-int -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION"
ENV CXXFLAGS_EXTRA "-stdlib=libc++"
ENV CXXFLAGS "$CFLAGS $CXXFLAGS_EXTRA" |
Looks like Swift 5.8.1 is currently used which uses LLVM 13.0.0. The latest release of Swift is 5.10.1 which comes with LLVM 15.0.0. It looks like Swift 6 is in development but I wasn't able to find a binary to download and see if it's at the right version. Given that I don't think there's an easy fix for Swift for now. |
c90949c
to
eb72f1d
Compare
-Wno-error=format-truncation \ | ||
-Wno-error=thread-safety-reference-return \ | ||
-Wno-error=nan-infinity-disabled \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-Wno-error=format-truncation \ | |
-Wno-error=thread-safety-reference-return \ | |
-Wno-error=nan-infinity-disabled \ |
nit: I'd say, if there is only a single project needing the suppressions, it should go into the project file. Especially, those warnings may point to real errors (as opposed to just warn about compile extensions, deprecations, or implicit behavior).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you forgot to add the three -Wno-error=
to the three project's build.sh
in the latest push?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry I misinterpreted this as leaving the flags out entirely. I'll try to work through some C++-specific failures and add per-image flags.
/gcbrun trial_build.py all |
From the trial build I've categorized the failures into: Miscellaneous failuresVarious failures in C++ projectsProjects that fail to compile with the latest Rust compilerTransient failures - OOM, out of disk, network issues, etcSwift failures - swift older LLVM causes coverage mismatchJava related failures having to do with maven and network issuesFailures due to `@sha256` pinning, all failed due to coverage version mismatchGiven this categorization the open questions I would have are:
|
I am working on unpinning them, but it will take some time. See https://github.com/google/oss-fuzz/pulls?q=is%3Aopen+is%3Apr+author%3Amaflcko+%22Use+latest+builder%22 for the current progress. |
But yeah, I'd say to remove the pin of all projects here. This will fix a few projects, like #12128 (comment). A few will remain broken, but those can be handled later/separately. Also, make sure to rebase again to pick up d63f82f. |
3d874e4
to
d335e91
Compare
Sounds good, I've rebased, removed the one-off warning flag allowances, and removed |
Only envoy/samba needed new flags, looks like the other projects have updated in the meantime and no longer need a fix |
Should fix the presubmit CI in https://github.com/google/oss-fuzz/actions/runs/9696807444/job/26759529232?pr=12077 (#12077) Co-authored-by: MarcoFalke <[email protected]>
@@ -54,7 +54,7 @@ apt-get update && apt-get install -y $LLVM_DEP_PACKAGES --no-install-recommends | |||
# languages, projects, ...) is needed. | |||
# Check CMAKE_VERSION infra/base-images/base-clang/Dockerfile was released | |||
# recently enough to fully support this clang version. | |||
OUR_LLVM_REVISION=llvmorg-18-init-4631-gd50b56d1 | |||
OUR_LLVM_REVISION=llvmorg-18.1.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: there's only a couple commits difference (llvm/llvm-project@llvmorg-18.1.7...llvmorg-18.1.8), but this could be 18.1.8
(which should also be the last 18.x tag).
Do we need another trial build here? |
Required for the compiler bump in #12077 See https://oss-fuzz-gcb-logs.storage.googleapis.com/log-7213ec9e-cac7-4eec-bd21-472547b52220.txt ``` Step #21 - "compile-libfuzzer-coverage-x86_64": CXXLD TestBinding Step #21 - "compile-libfuzzer-coverage-x86_64": CXXLD TestEventLogging Step #21 - "compile-libfuzzer-coverage-x86_64": CXXLD TestInetLayer Step #21 - "compile-libfuzzer-coverage-x86_64": clang++: �[0;1;31merror: �[0m�[1munable to execute command: Killed�[0m Step #21 - "compile-libfuzzer-coverage-x86_64": clang++: �[0;1;31merror: �[0m�[1mlinker command failed due to signal (use -v to see invocation)�[0m Step #21 - "compile-libfuzzer-coverage-x86_64": make[4]: *** [Makefile:4371: TestWeaveCert] Error 1 Step #21 - "compile-libfuzzer-coverage-x86_64": make[4]: *** Waiting for unfinished jobs.... Step #21 - "compile-libfuzzer-coverage-x86_64": clang++: �[0;1;31merror: �[0m�[1munable to execute command: Killed�[0m Step #21 - "compile-libfuzzer-coverage-x86_64": clang++: �[0;1;31merror: �[0m�[1mlinker command failed due to signal (use -v to see invocation)�[0m Step #21 - "compile-libfuzzer-coverage-x86_64": make[4]: *** [Makefile:4188: TestTAKE] Error 1 Step #21 - "compile-libfuzzer-coverage-x86_64": clang++: �[0;1;31merror: �[0m�[1munable to execute command: Killed�[0m Step #21 - "compile-libfuzzer-coverage-x86_64": clang++: �[0;1;31merror: �[0m�[1mlinker command failed due to signal (use -v to see invocation)�[0m Step #21 - "compile-libfuzzer-coverage-x86_64": make[4]: *** [Makefile:4078: TestInetEndPoint] Error 1 Step #21 - "compile-libfuzzer-coverage-x86_64": clang++: �[0;1;31merror: �[0m�[1munable to execute command: Killed�[0m Step #21 - "compile-libfuzzer-coverage-x86_64": clang++: �[0;1;31merror: �[0m�[1mlinker command failed due to signal (use -v to see invocation)�[0m Step #21 - "compile-libfuzzer-coverage-x86_64": make[4]: *** [Makefile:4098: TestKeyExport] Error 1 Step #21 - "compile-libfuzzer-coverage-x86_64": clang++: �[0;1;31merror: �[0m�[1munable to execute command: Killed�[0m Step #21 - "compile-libfuzzer-coverage-x86_64": clang++: �[0;1;31merror: �[0m�[1mlinker command failed due to signal (use -v to see invocation)�[0m Step #21 - "compile-libfuzzer-coverage-x86_64": make[4]: *** [Makefile:4106: TestMsgEnc] Error 1 Step #21 - "compile-libfuzzer-coverage-x86_64": clang++: �[0;1;31merror: �[0m�[1munable to execute command: Killed�[0m Step #21 - "compile-libfuzzer-coverage-x86_64": clang++: �[0;1;31merror: �[0m�[1mlinker command failed due to signal (use -v to see invocation)�[0m Step #21 - "compile-libfuzzer-coverage-x86_64": make[4]: *** [Makefile:4048: TestECMath] Error 1 Step #21 - "compile-libfuzzer-coverage-x86_64": clang++: �[0;1;31merror: �[0m�[1munable to execute command: Killed�[0m Step #21 - "compile-libfuzzer-coverage-x86_64": clang++: �[0;1;31merror: �[0m�[1mlinker command failed due to signal (use -v to see invocation)�[0m Step #21 - "compile-libfuzzer-coverage-x86_64": make[4]: *** [Makefile:4044: TestECDSA] Error 1 Step #21 - "compile-libfuzzer-coverage-x86_64": make[3]: *** [Makefile:6509: all-recursive] Error 1 Step #21 - "compile-libfuzzer-coverage-x86_64": make[2]: *** [Makefile:642: all-recursive] Error 1 Step #21 - "compile-libfuzzer-coverage-x86_64": make[1]: *** [Makefile:739: all-recursive] Error 1 Step #21 - "compile-libfuzzer-coverage-x86_64": make: *** [Makefile:665: all] Error 2 Step #21 - "compile-libfuzzer-coverage-x86_64": ******************************************************************************** Step #21 - "compile-libfuzzer-coverage-x86_64": Failed to build. Step #21 - "compile-libfuzzer-coverage-x86_64": To reproduce, run: Step #21 - "compile-libfuzzer-coverage-x86_64": python infra/helper.py build_image openweave Step #21 - "compile-libfuzzer-coverage-x86_64": python infra/helper.py build_fuzzers --sanitizer coverage --engine libfuzzer --architecture x86_64 openweave Step #21 - "compile-libfuzzer-coverage-x86_64": ******************************************************************************** Finished Step #21 - "compile-libfuzzer-coverage-x86_64" ERROR ERROR: build step 21 "gcr.io/cloud-builders/docker" failed: step exited with non-zero status: 1 --------- Co-authored-by: MarcoFalke <[email protected]> Co-authored-by: jonathanmetzman <[email protected]>
|
This is done in the interest of assisting google#12075 and google#11626. Currently the Rust toolchain cannot be updated because the latest nightly uses LLVM 18.1.8 and coverage information breaks. This breakage is because LLVM 18.1.8 records coverage information with version "9" but LLVM 18.0.0 recorded coverage information with version "8". This means that the recordings created by Rust binaries use version "9" which are unreadable by the processing that OSS-Fuzz does with the 18.0.0-based toolchain using version "8". This commit updates the Clang toolchain to the latest 18.x.x release to get the two in sync so the same coverage recording version is used.
Pull in LLVM 18.1.7 with Rust to match the previous Clang update
Fixes wolfssl, muduo
These builds will all be broken with the Clang update anyway so go ahead and remove the pins to keep everything on the latest.
Still fails at runtime with: ERROR: AddressSanitizer: use-after-poison on address 0x506000000050 at pc 0x55df3d07ee4d bp 0x7fffb5205a60 sp 0x7fffb5205a58 but it's at least further than compiling
06640af
to
6defef2
Compare
Did you fix
|
Also, I am not sure what to do about the remaining build failures, such as
Fixing the bug would be ideal, but I am not sure how relevant the project is, given that no reply has been received upstream and the project itself has been stale for more than a year? Given that this pull request here will unlock coverage runs for quite a few rust projects, I'd say pinning |
For librawspeed I'm now getting For xnu/others/etc, one thing I'd like to do is to clarify what the expectations are here. I don't personally have the time to keep going through all the projects and update their own build scripts and such. If that's expected of updates like this I think that's reasonable but I'll probably close this PR and shelve it for someone else to tackle later. I'm happy to rebase but I know very little about C/C++ fuzzing/updates/infrastructure here so I don't know how to approach these failures and what's expected of me vs what's expected of projects themselves. |
I'm OK pinning it. |
1 similar comment
I'm OK pinning it. |
Required for the compiler bump in #12077 --------- Co-authored-by: MarcoFalke <[email protected]>
My conclusion is that I don't have the time/energy to keep up with all the changes needed for this, so I'm going to close this to make room for if anyone else is interested in driving the update. |
Thanks for the work so far. I can pick it up later, if no one beats me to it. |
Required for the compiler bump in #12077 See https://oss-fuzz-gcb-logs.storage.googleapis.com/log-97a29aa7-6149-4c95-8428-5de204bd2214.txt ``` Step #21 - "compile-afl-address-x86_64": + cd /src/solidity/build Step #21 - "compile-afl-address-x86_64": + CXXFLAGS='-O1 -fno-omit-frame-pointer -gline-tables-only -Wno-error=enum-constexpr-conversion -Wno-error=incompatible-function-pointer-types -Wno-error=int-conversion -Wno-error=deprecated-declarations -Wno-error=implicit-function-declaration -Wno-error=implicit-int -Wno-error=vla-cxx-extension -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize=address -fsanitize-address-use-after-scope -stdlib=libc++ -I/usr/local/include/c++/v1' Step #21 - "compile-afl-address-x86_64": + cmake -DCMAKE_TOOLCHAIN_FILE=cmake/toolchains/ossfuzz.cmake -DCMAKE_BUILD_TYPE=Release /src/solidity Step #21 - "compile-afl-address-x86_64": -- The C compiler identification is Clang 18.1.8 Step #21 - "compile-afl-address-x86_64": -- The CXX compiler identification is Clang 18.1.8 Step #21 - "compile-afl-address-x86_64": -- Detecting C compiler ABI info Step #21 - "compile-afl-address-x86_64": -- Detecting C compiler ABI info - done Step #21 - "compile-afl-address-x86_64": -- Check for working C compiler: /src/aflplusplus/afl-clang-fast - skipped Step #21 - "compile-afl-address-x86_64": -- Detecting C compile features Step #21 - "compile-afl-address-x86_64": -- Detecting C compile features - done Step #21 - "compile-afl-address-x86_64": -- Detecting CXX compiler ABI info Step #21 - "compile-afl-address-x86_64": -- Detecting CXX compiler ABI info - done Step #21 - "compile-afl-address-x86_64": -- Check for working CXX compiler: /src/aflplusplus/afl-clang-fast++ - skipped Step #21 - "compile-afl-address-x86_64": -- Detecting CXX compile features Step #21 - "compile-afl-address-x86_64": -- Detecting CXX compile features - done Step #21 - "compile-afl-address-x86_64": �[31mCMake Error at /usr/lib/cmake/Boost-1.73.0/BoostConfig.cmake:141 (find_package): Step #21 - "compile-afl-address-x86_64": Found package configuration file: Step #21 - "compile-afl-address-x86_64": Step #21 - "compile-afl-address-x86_64": /usr/lib/cmake/boost_unit_test_framework-1.73.0/boost_unit_test_framework-config.cmake Step #21 - "compile-afl-address-x86_64": Step #21 - "compile-afl-address-x86_64": but it set boost_unit_test_framework_FOUND to FALSE so package Step #21 - "compile-afl-address-x86_64": "boost_unit_test_framework" is considered to be NOT FOUND. Reason given by Step #21 - "compile-afl-address-x86_64": package: Step #21 - "compile-afl-address-x86_64": Step #21 - "compile-afl-address-x86_64": No suitable build variant has been found. Step #21 - "compile-afl-address-x86_64": Step #21 - "compile-afl-address-x86_64": Call Stack (most recent call first): Step #21 - "compile-afl-address-x86_64": /usr/lib/cmake/Boost-1.73.0/BoostConfig.cmake:258 (boost_find_component) Step #21 - "compile-afl-address-x86_64": /usr/local/share/cmake-3.29/Modules/FindBoost.cmake:594 (find_package) Step #21 - "compile-afl-address-x86_64": cmake/EthDependencies.cmake:40 (find_package) Step #21 - "compile-afl-address-x86_64": CMakeLists.txt:51 (include) Step #21 - "compile-afl-address-x86_64": Step #21 - "compile-afl-address-x86_64": �[0m Step #21 - "compile-afl-address-x86_64": -- Configuring incomplete, errors occurred! Step #21 - "compile-afl-address-x86_64": ******************************************************************************** Step #21 - "compile-afl-address-x86_64": Failed to build. Step #21 - "compile-afl-address-x86_64": To reproduce, run: Step #21 - "compile-afl-address-x86_64": python infra/helper.py build_image solidity Step #21 - "compile-afl-address-x86_64": python infra/helper.py build_fuzzers --sanitizer address --engine afl --architecture x86_64 solidity Step #21 - "compile-afl-address-x86_64": ******************************************************************************** Finished Step #21 - "compile-afl-address-x86_64" ERROR ERROR: build step 21 "gcr.io/cloud-builders/docker" failed: step exited with non-zero status: 1 Co-authored-by: MarcoFalke <[email protected]>
Required for the compiler bump in #12077 Full log: https://oss-fuzz-gcb-logs.storage.googleapis.com/log-49527212-a452-4fff-aaa5-293bf0924395.txt --------- Co-authored-by: MarcoFalke <[email protected]>
These builds will all be broken with the Clang update anyway so go ahead and remove the pins to keep everything on the latest. XNU remains pinned, given the comment here: google#12077 (comment).
These builds will all be broken with the Clang update anyway so go ahead and remove the pins to keep everything on the latest. XNU remains pinned, given the comment here: google#12077 (comment).
These builds will all be broken with the Clang update anyway so go ahead and remove the pins to keep everything on the latest. XNU remains pinned, given the comment here: google#12077 (comment).
These builds will all be broken with the Clang update anyway so go ahead and remove the pins to keep everything on the latest. XNU remains pinned, given the comment here: google#12077 (comment).
Follow-up on #12077 by @alexcrichton cc @maflcko Main difference is to update infra/base-images/base-runner/profraw_update.py so that oss-fuzz converts profraw version 8 to 9 (and llvm-cov seems more tolerant in older version reading cf llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp This way, it should be more transparent for projects, that can be updated individually or not --------- Co-authored-by: Alex Crichton <[email protected]>
This is done in the interest of assisting #12075 and #11626. Currently the Rust toolchain cannot be updated because the latest nightly uses LLVM 18.1.7 and coverage information breaks. This breakage is because LLVM 18.1.7 records coverage information with version "9" but LLVM 18.0.0 recorded coverage information with version "8". This means that the recordings created by Rust binaries use version "9" which are unreadable by the processing that OSS-Fuzz does with the 18.0.0-based toolchain using version "8".
This commit updates the Clang toolchain to the latest 18.x.x release to get the two in sync so the same coverage recording version is used.