xazax.hun added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:22
+ ProgramStateRef State) const;
+ mutable std::unique_ptr<BuiltinBug> BT_Placement;
+};
----------------
I think now it is safe to have the bugtype by value and use member
initialization.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:35
+ // long *lp = ::new (buf) long;
+ if (const auto *TVRegion = dyn_cast<TypedValueRegion>(MRegion))
+ // FIXME Handle offset other than 0. E.g.:
----------------
Add braces here.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:40
+ if (const auto *ERegion = dyn_cast<ElementRegion>(TVRegion)) {
+ const auto *SRegion = cast<SubRegion>(ERegion->getSuperRegion());
+ DefinedOrUnknownSVal Extent = SRegion->getExtent(svalBuilder);
----------------
Hmm, I think this logic might need some more testing. Could you add some tests
with multi dimensional arrays?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:44
+ // This is modelled by the MallocChecker.
+ const llvm::APSInt *ExtentInt = svalBuilder.getKnownValue(State, Extent);
+ if (!ExtentInt)
----------------
This query will only check if you know the exact value of the target of
placement you.
What you actually care about if the size is at least as big as the placed
object.
So instead of getting a known value I think it might be a better idea to
evaluate a less than or equal operator with `evalBinOp`.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:64
+ CheckerContext &C, const CXXNewExpr *NE, ProgramStateRef State) const {
+ if (!State)
+ return SVal();
----------------
When do you see a null state?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:72
+ } else {
+ ElementCount = svalBuilder.makeIntVal(1, true);
+ }
----------------
Probably you could just return the size without building a more complex
symbolic expression in this case? I.e. just put the right value in the
makeIntVal.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:79
+
+ if (ElementCount.getAs<NonLoc>()) {
+ // size in Bytes = ElementCount*TypeSize
----------------
You could:
```
Optional<NonLoc> NL = ElementCount.getAs<NonLoc>();
```
And later you could replace the `castAs` with a deref of the optional.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:81
+ // size in Bytes = ElementCount*TypeSize
+ SVal SizeInBytes = svalBuilder.evalBinOpNN(
+ State, BO_Mul, ElementCount.castAs<NonLoc>(),
----------------
Prefer to use `evalBinOp` over `evalBinOpNN`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71612/new/
https://reviews.llvm.org/D71612
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
