Repository navigation
fix(rocjitsu): suppress GCC 14 -Wfree-nonheap-object false positive on ppc64le - #14412
Conversation
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
kuhar
left a comment
kuhar
left a comment
There was a problem hiding this comment.
It would help to correct the GCC reference and document the trigger beside the suppression. GCC 104568 concerns a C++20 compiler crash involving zero-sized new, rather than this vector warning, so it does not explain when this workaround can be removed.
Please include the GCC 14.3.1 ppc64le diagnostic and inlining chain in the PR description, then link the matching compiler report from a short rationale comment. That would identify the affected operation and give future maintainers a way to reassess the workaround. The description should also reflect the final scope: as written, x86 GCC 14+ warning handling changes too.
| ) | ||
| target_compile_options( | ||
| ${target} | ||
| PRIVATE -Wno-error=free-nonheap-object |
There was a problem hiding this comment.
I think we should scope this demotion to execution_fuzz_core.cpp on the affected POWER configuration. The guard currently changes every rj_configure_target(... WARNINGS) target on all GCC 14+ architectures. In a native x86_64 GCC 16 fixture using this helper, freeing a stack variable fails compilation before this change but succeeds afterward, with the warning still emitted. That broadens the warning policy beyond the reported ppc64le false positive.
The source-specific COMPILE_OPTIONS workaround for lds_test.cpp in tests/CMakeLists.txt:685-697, combined with the existing ppc64 processor guard, provides a nearby pattern. Putting this beside the execution-fuzzer target would preserve the error policy elsewhere without needing to extend the demotion to the separate object-library helper.
There was a problem hiding this comment.
Updated the commit message. And updated the change so it only affects POWER.
There was a problem hiding this comment.
I think we should still limit this to execution_fuzz_core.cpp in fuzz/execution/CMakeLists.txt, using the source-level COMPILE_OPTIONS pattern in tests/CMakeLists.txt:685-697 and retaining the POWER/GNU guard. The new processor check fixes the x86 concern, but on POWER this still demotes the warning for every rj_configure_target(... WARNINGS) target, including unrelated tests and tools. Object-library targets use a separate helper and retain the error policy. Scoping the exception to the reported source keeps that policy intact elsewhere and puts the rationale beside the affected target.
There was a problem hiding this comment.
Scoped the suppression to execution_fuzz_core.cpp only
8a2e61f to
6286c8a
Compare
kuhar
left a comment
kuhar
left a comment
There was a problem hiding this comment.
Left an inline reply and a new comment
| # new_allocator.h:172 on a pointer the compiler loses track of | ||
| # after reserve() + push_back(). Only observed on ppc64le; x86 | ||
| # inlining decisions do not trigger it. Revisit when the project | ||
| # moves to GCC 16+, where the libstdc++ fix has landed. |
There was a problem hiding this comment.
It would help to replace the definite GCC 16 libstdc++ fix claim with a verified removal criterion. GCC 115180 redirects to 115016, whose discussion reports that its testcase stopped reproducing on trunk; it does not establish that this POWER build is fixed by a GCC 16 libstdc++ change.
Please name execution_fuzz_core.cpp in the rationale and say to recheck it on ppc64le at -O3 with the demotion removed. That gives maintainers a concrete check instead of relying on new_allocator.h:172, which varies by libstdc++ version. Narrow the version guard once that check establishes which compiler versions no longer need the workaround.
There was a problem hiding this comment.
Fixed the rationale comment
…n ppc64le
GCC 14 on ppc64le emits a false-positive -Wfree-nonheap-object inside
std::vector<uint8_t>::push_back / _M_realloc_append when inlining
random_input() in execution_fuzz_core.cpp at -O3 (GCC PR 115016).
The inlining chain is:
random_input (execution_fuzz_core.cpp:500)
-> vector::push_back (stl_vector.h:1314)
-> vector::emplace_back (vector.tcc:123)
-> vector::_M_realloc_append (vector.tcc:688)
-> __new_allocator::deallocate (new_allocator.h:172)
=> -Wfree-nonheap-object
Scope the suppression to execution_fuzz_core.cpp only, guarded to
GCC >= 14 on ppc64 targets, following the per-source COMPILE_OPTIONS
pattern used for lds_test.cpp in tests/CMakeLists.txt. The warning
is demoted from error but still emitted, so real instances remain
visible in build logs.
Recheck: remove this block and build execution_fuzz_core.cpp on
ppc64le at -O3 with the next GCC upgrade.
Co-Authored-By: Claude <noreply@anthropic.com>
6286c8a to
9c47d6f
Compare
rocjitsu post-commit fuzz passedThe 32-worker multiplexer campaigns for commit
|
TheRock Submodule Bump ActivityNewest first
|
Motivation
Part of work for ROCm/TheRock#5518
rocjitsu fails to build on ppc64le with GCC 14 due to a false-positive
-Wfree-nonheap-objectwarning inexecution_fuzz_core.cpp, promoted to an error by-Werror. This is a known GCC regression (GCC PR 115180) triggered bystd::vector<uint8_t>::_M_realloc_appendinlining at-O3. A similar false positive was previously worked around in rocjitsu in #13075.Technical Details
GCC 14.3.1 on ppc64le loses track of the vector's base pointer when inlining
reserve()followed bypush_back()inrandom_input(), and incorrectly warns thatoperator deleteis called on a non-heap pointer:Test Plan
Built rocjitsu on a POWER10 ppc64le machine (GCC 14.3.1) via
cmake --buildin the TheRock build tree. Verified all 476 targets compile and link.Test Result
476/476 targets succeeded, exit 0. The remaining compiler notes about SIMD parameter passing (
std::experimental::parallelism_v2) are harmless informational messages, not warnings or errors.