Skip to content

bypass null-deref warnings#7876

Open
nirandaperera wants to merge 4 commits intoNVIDIA:mainfrom
nirandaperera:enable-null-dereference
Open

bypass null-deref warnings#7876
nirandaperera wants to merge 4 commits intoNVIDIA:mainfrom
nirandaperera:enable-null-dereference

Conversation

@nirandaperera
Copy link
Contributor

Description

Fixes #7875

Enables -Wnull-dereference (with -Werror) in the libcudacxx test suite and suppresses the resulting GCC false-positive warnings in the __basic_any type-erasure implementation.

Why -Wnull-dereference?

-Wnull-dereference asks GCC to perform inter-procedural null-pointer flow analysis and warn (with -Werror, hard-fail) on any code path that can reach a dereference through a pointer that could be null. This catches a class of bugs that -Wall and -Wextra miss.

Why the suppressions?

GCC's analysis cannot see through __basic_any's type-erasure invariants. When __query_interface is inlined, GCC observes its return nullptr branch and concludes the result might be null at every call site, even those that are guarded by the design (e.g., __iunknown is always registered, virtual dispatch is only reached through a live object). This leads to false-positive errors in three locations:

File Location Root cause
virtcall.h __virtcall__vptr->__fn_(...) __query_interface inlined; GCC sees potential nullptr
basic_any_value.h reset() / type()__query_interface(__iunknown())->... Same: __iunknown lookup inlined, nullable path visible
rtti.h __try_vptr_castrtti->__query_interface(...) __query_interface(__iunknown()) inlined into slow-path cast

Each suppression is scoped as tightly as possible — a _CCCL_DIAG_PUSH / _CCCL_DIAG_SUPPRESS_GCC("-Wnull-dereference") / _CCCL_DIAG_POP bracket around only the offending statement(s). _CCCL_DIAG_SUPPRESS_GCC expands to a no-op on all non-GCC compilers.

Changes

  • libcudacxx/test/CMakeLists.txt — adds -Wnull-dereference to headertest_warning_levels_device (NVIDIA/NVCC, Clang-as-device, and generic paths) and headertest_warning_levels_host (non-MSVC). MSVC paths are unchanged.
  • libcudacxx/include/cuda/__utility/__basic_any/virtcall.h — suppresses false positive in __virtcall at the virtual dispatch call site.
  • libcudacxx/include/cuda/__utility/__basic_any/basic_any_value.h — suppresses false positives in reset() and type() around the __query_interface(__iunknown()) dereference chains.
  • libcudacxx/include/cuda/__utility/__basic_any/rtti.h — suppresses false positive in __try_vptr_cast slow-path (down-cast / cross-cast) where rtti is known non-null by the __iunknown registration invariant.

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Signed-off-by: niranda perera <niranda.perera@gmail.com>
@nirandaperera nirandaperera requested review from a team as code owners March 4, 2026 01:07
@nirandaperera nirandaperera requested a review from miscco March 4, 2026 01:08
@github-project-automation github-project-automation bot moved this to Todo in CCCL Mar 4, 2026
@cccl-authenticator-app cccl-authenticator-app bot moved this from Todo to In Review in CCCL Mar 4, 2026
@pciolkosz
Copy link
Contributor

Thanks for looking into it, it seems someone with cmake ownership needs to approve as well

@github-actions

This comment has been minimized.

Comment on lines +351 to +353
// GCC cannot prove __query_interface returns non-null for __iunknown (the invariant guarantees it)
_CCCL_DIAG_PUSH
_CCCL_DIAG_SUPPRESS_GCC("-Wnull-dereference")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical: some compilers have issues with warning suppression inside of class definitions. We should rather move the suppression outside to a single one at the top and one push at the bottom.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miscco what's the better practice here? Should I move the PUSH to the very top of the cpp file covering all methods? Or just cover these 2 methods?

Because, in the future, if some other warning needs to be suppressed, we should be able to see the existing warning suppressions.

@github-project-automation github-project-automation bot moved this from In Review to In Progress in CCCL Mar 4, 2026
@nirandaperera nirandaperera requested a review from miscco March 4, 2026 19:00
@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

🥳 CI Workflow Results

🟩 Finished in 1h 31m: Pass: 100%/99 | Total: 1d 10h | Max: 1h 14m | Hits: 94%/255613

See results here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

[BUG]: Warnings with -Wnull-dereference cmake flag

3 participants