[PATCH] D140179: [WIP][-Wunsafe-buffer-usage] Add unsafe buffer checking opt-out pragmas

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 15 18:57:53 PST 2022


NoQ added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:943
+def warn_pragma_unsafe_buffer_usage_misorder :
+ExtWarn<"pragma unsafe_buffer_usage used in misorder">,
+   InGroup<IgnoredPragmas>;
----------------
IIUC `ExtWarn` means it's a warning of the form "warning: XXX is a language extension". It's not just a warning that has something to do with language extensions, it's a warning that tries to warn the user about the very fact that it's an extension. So we should just use `Warning` or `Error`.

Also about wording: we already have a somewhat similar `#pragma assume_nonnull`, can we use similar text? And probably prefer `Error` because that's what the other pragma uses too:
```
def err_pp_assume_nonnull_syntax : Error<"expected 'begin' or 'end'">;
def err_pp_double_begin_of_assume_nonnull : Error<
  "already inside '#pragma clang assume_nonnull'">;
def err_pp_unmatched_end_of_assume_nonnull : Error<
  "not currently inside '#pragma clang assume_nonnull'">;
def err_pp_include_in_assume_nonnull : Error<
  "cannot %select{#include files|import headers}0 "
  "inside '#pragma clang assume_nonnull'">;
def err_pp_eof_in_assume_nonnull : Error<
  "'#pragma clang assume_nonnull' was not ended within this file">;
```


================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2379
+    for (auto UnsafeUse : UnsafeUses)
+      if (!DE.isSafeBufferOptOut(SM, UnsafeUse->getBeginLoc()))
+        UnsafeUsesToReport.push_back(UnsafeUse);
----------------
I believe this check should be performed *much earlier*. It's not about how we display unsafe usages to the user; we can exclude variables from analysis entirely when all their unsafe uses are guarded by the pragma. I suspect that we can drop these gadgets as early as in `findGadgets()` phase (assuming that D140062 causes us to never rely on unsafe gadgets for fixits).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140179/new/

https://reviews.llvm.org/D140179



More information about the cfe-commits mailing list