steakhal added a comment.
Oh, so we would canonicalize towards a signed extent type. I see. I think I'm
okay with that.
However, I think this needs a little bit of polishing and testing when the
region does not point to the beginning of an array or object, but rather inside
an object, or like this:
int nums[] = {1,2,3};
char *p = (char*)&nums[1];
clang_analyzer_dumpExtent(p);
clang_analyzer_dumpElementCount(p);
++p;
clang_analyzer_dumpExtent(p);
clang_analyzer_dumpElementCount(p);
++p;
int *q = (int*)p;
clang_analyzer_dumpExtent(q);
clang_analyzer_dumpElementCount(q);
================
Comment at: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp:47-48
+ const MemRegion *MR) {
+ if (!MR)
+ return UnknownVal();
+ MR = MR->StripCasts();
----------------
I'd prefer hoisting this check to the callsite.
Usually, we assume `MR` is non-null. This is already the case for a sibling
API, `getDynamicExtent()`.
Let's keep these "overloads" behave the same.
================
Comment at: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp:56-57
+ QualType Ty = TVR->getValueType();
+ if (const auto *ConstArrayTy =
+ dyn_cast_or_null<ConstantArrayType>(Ty->getAsArrayTypeUnsafe()))
+ return SVB.makeIntVal(ConstArrayTy->getSize(), false);
----------------
Funnily enough, one must use the `ASTContext::getAsConstantArrayType()` to
achieve this.
I'm not sure why, but I was bitten by this.
It says something like this at the ASTContext:
```
/// Type Query functions. If the type is an instance of the specified class,
/// return the Type pointer for the underlying maximally pretty type. This
/// is a member of ASTContext because this may need to do some amount of
/// canonicalization, e.g. to move type qualifiers into the element type.
const ArrayType *getAsArrayType(QualType T) const;
const ConstantArrayType *getAsConstantArrayType(QualType T) const {
return dyn_cast_or_null<ConstantArrayType>(getAsArrayType(T));
}
```
================
Comment at: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp:58
+ dyn_cast_or_null<ConstantArrayType>(Ty->getAsArrayTypeUnsafe()))
+ return SVB.makeIntVal(ConstArrayTy->getSize(), false);
+
----------------
For bool literal arguments we usually use "named parameter passing", aka.
`/*paramname=*/false` constructs.
================
Comment at: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp:66-75
+ assert(!ElementSize.isZeroConstant() && "Non-zero element size expected");
+ if (Size.isUndef())
+ return UnknownVal();
+
+ SValBuilder &SVB = State->getStateManager().getSValBuilder();
+
+ auto ElementCount =
----------------
If `ElementSize` would be some symbol, constrained to null, we would pass the
assertion, but still end up modeling a division by zero, resulting in
`Undefined`, which then turned into `Unknown` - I see.
Looking at this, the assertion seems misleading as it won't protect the
division.
That said, the early return on undef could be also dropped.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158707/new/
https://reviews.llvm.org/D158707
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
