[PATCH] D138253: [-Wunsafe-buffer-usage] NFC: Implement fix-strategies and variable-use-claiming.

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 17 19:13:43 PST 2022


NoQ created this revision.
NoQ added reviewers: aaron.ballman, gribozavr2, xazax.hun, jkorous, t-rasmud, ziqingluo-90, malavikasamak.
Herald added subscribers: steakhal, martong, rnkovacs.
Herald added a project: All.
NoQ requested review of this revision.

This patch adds more abstractions that we'll need later for emitting `-Wunsafe-buffer-usage` fixits. It doesn't emit any actual fixits, so no change is observed behavior, but it introduces a way to emit fixits, and existing tests now verify that the compiler still emits no fixits, despite knowing how to do so.

The purpose of our code transformation analysis is to fix variable types in the code from raw pointer types to C++ standard collection/view types.

The analysis has to decide on its own which specific type is the most appropriate for every variable. This patch introduces the `Strategy` class that maps variables to their most appropriate types.

In D137348 <https://reviews.llvm.org/D137348> we've introduced the `Gadget` abstraction, which describes a rigid AST pattern that the analysis "fully understands" and may need to fix. Which specific fix is actually necessary for a given `Gadget`, and whether it's //necessary// at all, and whether it's //possible// in the first place, depends on the `Strategy`. So, this patch adds a virtual method which every gadget can implement in order to teach the analysis how to fix that gadget:

  Gadget->getFixits(Strategy)

However, even if the analysis knows how to fix every `Gadget`, doesn't necessarily mean it can fix the variable. Some uses of the variable may have never been covered by `Gadget`s, which corresponds to the situation that the analysis doesn't fully understand how the variable is used. This patch introduces a `Tracker` class that tracks all variable uses (i.e. `DeclRefExpr`s) in the function. Additionally, each `Gadget` now provides a new virtual method

  Gadget->getClaimedVarUseSites()

that the `Tracker` can call to see which `DeclRefExpr`s are "claimed" by the `Gadget`. In order to fix the variable with a certain `Strategy`, the `Tracker` needs to confirm that there are no unclaimed uses, and every `Gadget` has to provide a fix for that `Strategy`.

This "conservative" behavior guarantees that fixes emitted by our analysis are correct by construction. We can now be sure that the analysis won't attempt to emit a fix if it doesn't understand the code. Later, as we implement more `getFixits()` methods in individual `Gadget` classes, we'll start progressively emitting more and more fixits.


Repository:
  rC Clang

https://reviews.llvm.org/D138253

Files:
  clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D138253.476316.patch
Type: text/x-patch
Size: 14884 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221118/8102fda7/attachment-0001.bin>


More information about the cfe-commits mailing list