Diagnostics for `-Wunsafe-buffer-usage` won't fire unless enabled at *end* of file

[mmdriley]

Like with other diagnostics it's possible to control -Wunsafe-buffer-usage with #pragma clang diagnostic within a source file. However, there's a weird nonlocal effect: for any instances of the warning to fire, it seems the warning must be enabled at the point of occurrence and at the end of the file.

This isn't true for other warnings like -Wsign-compare.

You can see an example here: https://godbolt.org/z/4eGc5hdzd

Initially there are no -Wunsafe-buffer-usage warnings. But if you uncomment the last line of the input (#pragma clang diagnostic warning "-Wunsafe-buffer-usage") then warnings that appear earlier in the file will show up.

[shafik]

CC @haoNoQ @AaronBallman

[haoNoQ]

This is actually a somewhat fundamental problem with so-called "analysis-based warnings", i.e. the warnings that aren't emitted by the compiler as a natural byproduct of compilation, but require a separate analysis "pass" through the code in order to connect bits and pieces of code the compiler doesn't normally need to connect. The compiler doesn't want to pay the cost of the analysis if the warning is disabled anyway, but it also can't guess where exactly is the warning going to be emitted before conducting that analysis. This plays very poorly with #pragma clang diagnostic that can turn the warning on and off at any point. For example you can break -Wuninitialized this way: https://godbolt.org/z/9jPa95az4 - it has to be enabled at the beginning of the function in order for analysis to be invoked.

In case of -Wunsafe-buffer-usage the analysis can't even go function-by-function. In some edge cases it has to connect multiple functions together, which results in even more weirdness of this kind. That said, there's definitely some room for improvement (and your example demonstrates very well why this is important). In particular, we can and arguably should go function-by-function unless the -fsafe-buffer-usage-suggestions flag is also passed (which also directly tells us that the analysis is desirable so we don't have to worry about paying for what we don't need).

Fundamentally in order to get rid of these problems we should probably teach the compiler how to tell whether the warning enabled at any point in a given section of code. That'll require some significant rework though.

Also worth noting that you may have a better experience with enabling the warning globally and then disabling it outside of your small code section via:

#pragma clang unsafe_buffer_usage begin
  // warning is turned OFF here
#pragma clang unsafe_buffer_usage end

This pragma is specialized to work with -Wunsafe-buffer-usage as it correctly disables the warning for raw pointer operations in the given range, even if the warning itself is emitted outside of that range. It works line-by-line as well, as it gets applied after the analysis has finished. So this is a much cleaner way to say "this right there, is possibly bounds-unsafe code".

[mmdriley]

Thank you! The begin / end markers will work great.

It seems like maybe the documentation for those pragmas from https://reviews.llvm.org/D136811 didn't make it over to Github? At least, I couldn't it in-tree or in any PRs.

[rnk]

I just rediscovered this issue, and I think it's worth reprioritizing. The Clang SourceManager should know if a given analysis-based warning was enabled anywhere during compilation, and we should run all the analyses that were requested. Once we have actual warning with source locations, we can check them against the diagnostic state in the source manager and suppress them later if requested.

Answering open questions from old comments for future readers: https://reviews.llvm.org/D136811 became #111624, and the docs are live at https://clang.llvm.org/docs/SafeBuffers.html

[rnk]

I think I found the glitch, if you hack this condition to "true", you get the desired behavior:

$  cat t.cpp 
extern int *p;
int *w1() { return p + 1; }
_Pragma("clang diagnostic ignored \"-Wunsafe-buffer-usage\"")
int *w2() { return p + 1; }

$ clang -c t.cpp  -Wunsafe-buffer-usage
t.cpp:2:20: warning: unsafe pointer arithmetic [-Wunsafe-buffer-usage]
    2 | int *w1() { return p + 1; }
      |                    ^
1 warning generated.

$ git diff
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 589869d01865..3475bbf87d26 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2627,7 +2627,7 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
   };
   // Emit per-function analysis-based warnings that require the whole-TU
   // reasoning. Check if any of them is enabled at all before scanning the AST:
-  if (!Diags.isIgnored(diag::warn_unsafe_buffer_operation, SourceLocation()) ||
+  if (true || !Diags.isIgnored(diag::warn_unsafe_buffer_operation, SourceLocation()) ||
       !Diags.isIgnored(diag::warn_unsafe_buffer_variable, SourceLocation()) ||
       !Diags.isIgnored(diag::warn_unsafe_buffer_usage_in_container,
                        SourceLocation()) ||