[clang] [-Wunsafe-buffer-usage] Add FixableGadget for AddAssign in UnspecifiedUntypedContext (PR #71862)

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 16 19:12:53 PST 2023


================
@@ -1766,6 +1812,48 @@ fixUPCAddressofArraySubscriptWithSpan(const UnaryOperator *Node) {
       FixItHint::CreateReplacement(Node->getSourceRange(), SS.str())};
 }
 
+std::optional<FixItList>
+UUCAddAssignGadget::getFixits(const Strategy &S) const {
+  DeclUseList DREs = getClaimedVarUseSites();
+
+  if (DREs.size() != 1)
+    return std::nullopt; // In cases of `Ptr += n` where `Ptr` is not a DRE, we
+                         // give up
+  if (const VarDecl *VD = dyn_cast<VarDecl>(DREs.front()->getDecl())) {
+    if (S.lookup(VD) == Strategy::Kind::Span) {
+      FixItList Fixes;
+      std::stringstream SS;
+      const Stmt *AddAssignNode = getBaseStmt();
+      StringRef varName = VD->getName();
+      const ASTContext &Ctx = VD->getASTContext();
+
+      if (!isNonNegativeIntegerExpr(Offset, VD, Ctx))
+        return std::nullopt;
+
+      // To transform UUC(p += n) to UUC(p = p.subspan(..)):
+      SS << varName.data() << " = " << varName.data() << ".subspan";
----------------
haoNoQ wrote:

> _"Ironic. We could save others from `.data()` but not ourselves."_

On a serious note though, it looks like `StringRef.data()` is a plain `const char *` that isn't guaranteed to be null-terminated. And `std::stringstream` probably expects it to be null-terminated, because there's no other bounds information available to it. So I'm quite worried about an actual overrun in this case.

Worth noting that `StringRef.str()` is safer than `StringRef.data()` because it produces a `std::string`, but of course this requires an unnecessary deep copy.

I think the usual idiom in LLVM is
```
llvm::SmallString<128> Str;
llvm::raw_svector_ostream SS(Str);
SS << varName << ...;
```
because custom streams consume `llvm::StringRef` naturally. Though for such simple operations I think it's perfectly fine to just use `+`: 
```
std::string S = varName.str() + " = " + varName + ".subspan";
if (NotParenExpr)
  S += "(";
```

https://github.com/llvm/llvm-project/pull/71862


More information about the cfe-commits mailing list