Skip to content

[-Wunsafe-buffer-usage] Ignore constant safe indices in array subscri… #8196

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

Conversation

jkorous-apple
Copy link

…pts (llvm#80504)

[-Wunsafe-buffer-usage] Ignore safe array subscripts Don't emit warnings for array subscripts on constant size arrays where the index is constant and within bounds.

Example:
int arr[10];
arr[5] = 0; //safe, no warning

This patch recognizes only array indices that are integer literals - it doesn't understand more complex expressions (arithmetic on constants, etc.).

-Warray-bounds implemented in Sema::CheckArrayAccess() already solves a similar (opposite) problem, handles complex expressions and is battle-tested.

Adding -Wunsafe-buffer-usage diagnostics to Sema is a non-starter as we need to emit both the warnings and fixits and the performance impact of the fixit machine is unacceptable for Sema.

CheckArrayAccess() as is doesn't distinguish between "safe" and "unknown" array accesses. It also mixes the analysis that decides if an index is out of bounds with crafting the diagnostics.

A refactor of CheckArrayAccess() might serve both the original purpose and help us avoid false-positive with -Wunsafe-buffer-usage on constant size arrrays.

(cherry picked from commit 9a1e637)

@jkorous-apple
Copy link
Author

@swift-ci please test

…pts (llvm#80504)

[-Wunsafe-buffer-usage] Ignore safe array subscripts
Don't emit warnings for array subscripts on constant size arrays where the index is constant and within bounds.

Example:
int arr[10];
arr[5] = 0; //safe, no warning

This patch recognizes only array indices that are integer literals - it doesn't understand more complex expressions (arithmetic on constants, etc.).

-Warray-bounds implemented in Sema::CheckArrayAccess() already solves a similar
(opposite) problem, handles complex expressions and is battle-tested.

Adding -Wunsafe-buffer-usage diagnostics to Sema is a non-starter as we need to emit
both the warnings and fixits and the performance impact of the fixit machine is
unacceptable for Sema.

CheckArrayAccess() as is doesn't distinguish between "safe" and "unknown" array
accesses. It also mixes the analysis that decides if an index is out of bounds
with crafting the diagnostics.

A refactor of CheckArrayAccess() might serve both the original purpose
and help us avoid false-positive with -Wunsafe-buffer-usage on constant
size arrrays.

(cherry picked from commit 9a1e637)
@jkorous-apple jkorous-apple force-pushed the cxx-safe-buffers/integrate/ignore-constant-safe-subscripts branch from a1ccb22 to bb71067 Compare February 15, 2024 17:54
@shahmishal shahmishal merged commit 5fe208d into swiftlang:stable/20230725 Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants