Repository navigation
build(hipfile): fix HIPFILE_WARN_UNSAFE_BUFFER_OPS and default it to OFF - #12939
Merged
Merged
Conversation
The option added -Wunsafe-buffer-usage and -fsafe-buffer-usage-suggestions to a warning-flags variable that nothing used, so the flags never reached the compiler. The flags are now added in hipfile_set_compiler_flags() for C++ sources compiled with Clang. HIP sources are excluded since kernels use raw pointers by necessity, and the flags are skipped with IWYU, like the other warning flags. With the flags actually applied, the build produces a few hundred warnings, which would break the -Werror CI builds. The option now defaults to OFF until those are fixed.
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
Isolated CMake generation confirmed correct flag handling, and no blocking issues remain.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes hipFile’s optional unsafe-buffer warnings so the flags reach eligible compiler invocations.
Changes:
- Applies warning flags to Clang C++ sources, excluding HIP and IWYU builds.
- Defaults the option to
OFFand documents the behavior.
| File | Description |
|---|---|
| projects/hipfile/cmake/HipFileCompilerOptions.cmake | Applies conditional compiler flags. |
| projects/hipfile/cmake/HipFileClangSafeBuffers.cmake | Disables warnings by default and retains compiler validation. |
| projects/hipfile/CHANGELOG.md | Documents the fix and default change. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
spolifroni-amd
approved these changes
Oct 6, 2026
spolifroni-amd
left a comment
spolifroni-amd
left a comment
Contributor
There was a problem hiding this comment.
A minior change to that first one, but otherwise fine.
Co-authored-by: spolifroni-amd <Sandra.Polifroni@amd.com>
spolifroni-amd
approved these changes
Oct 6, 2026
Co-authored-by: spolifroni-amd <Sandra.Polifroni@amd.com>
gaoikawa
approved these changes
Oct 6, 2026
riley-dixon
approved these changes
Oct 6, 2026
spolifroni-amd
approved these changes
Oct 7, 2026
Contributor
TheRock Submodule Bump ActivityNewest first
|
anujshuk-amd
pushed a commit
that referenced
this pull request
Oct 12, 2026
…OFF (#12939) ## Motivation Fixing incidental findings while updating hipFile CMake ## Technical Details The option added -Wunsafe-buffer-usage and -fsafe-buffer-usage-suggestions to a warning-flags variable that nothing used, so the flags never reached the compiler. The flags are now added in hipfile_set_compiler_flags() for C++ sources compiled with Clang. HIP sources are excluded since kernels use raw pointers by necessity, and the flags are skipped with IWYU, like the other warning flags. With the flags actually applied, the build produces a few hundred warnings, which would break the -Werror CI builds. The option now defaults to OFF until those are fixed. ## Issue Tracking JIRA ID: AIHIPFILE-296 ## Test Plan CI passes ## Test Result Success ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. --------- Co-authored-by: spolifroni-amd <Sandra.Polifroni@amd.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Fixing incidental findings while updating hipFile CMake
Technical Details
The option added -Wunsafe-buffer-usage and
-fsafe-buffer-usage-suggestions to a warning-flags variable that nothing used, so the flags never reached the compiler.
The flags are now added in hipfile_set_compiler_flags() for C++ sources compiled with Clang. HIP sources are excluded since kernels use raw pointers by necessity, and the flags are skipped with IWYU, like the other warning flags.
With the flags actually applied, the build produces a few hundred warnings, which would break the -Werror CI builds. The option now defaults to OFF until those are fixed.
Issue Tracking
JIRA ID: AIHIPFILE-296
Test Plan
CI passes
Test Result
Success
Submission Checklist