NoQ added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp:43
+
+ if (!isa<GlobalImmutableSpaceRegion>(R->getMemorySpace()))
+ return;
----------------
zukatsinadze wrote:
> NoQ wrote:
> > This check catches a lot more regions than the ones produced by the other
> > checker. In particular, it'll warn on all global constants. Did you intend
> > this to happen? You don't seem to have tests for that.
> Could you give an example? I tried with the following snippet and it didn't
> give a warning
>
> ```
> int a = 1, b = 1;
> void foo(int *p) {
> a = 2;
> p[b++] = 3;
> int *c = &a;
> *c = 5;
> }
> ```
I was thinking about constants, i.e. `const int a` etc.
================
Comment at:
clang/lib/StaticAnalyzer/Checkers/cert/ModelConstQualifiedReturnChecker.cpp:30-43
+class ModelConstQualifiedReturnChecker : public Checker<eval::Call> {
+private:
+ void evalConstQualifiedReturnCall(const CallEvent &Call,
+ CheckerContext &C) const;
+
+ // SEI CERT ENV30-C
+ const CallDescriptionSet ConstQualifiedReturnFunctions = {
----------------
NoQ wrote:
> steakhal wrote:
> > I can see a small issue by `evaCall`-ing these functions.
> > The problem is that we might not model all aspects of these functions, thus
> > the modeling can cause harm to the analysis. E.g. by not doing invalidation
> > where we actually would need invalidation to kick in, or anything else
> > really.
> > Consequently, another problem is that no other checker can evaluate these,
> > since we evaluate them here.
> > Thus, fixing such improper modeling could end up in further changes down
> > the line.
> > Unfortunately, I don't have any better ATM, so let's go with this
> > `evalCall` approach.
> Right, this definitely shouldn't be `evalCall`. Post-call seems to be
> sufficient for this checker.
Yeah so the reason why this was made an `evalCall` is because we needed to
replace the memory space for the return value. I think the right solution is to
re-implement SymRegion memory spaces as traits, so that they could be updated
during analysis at any time (under the natural condition that they only ever
get narrower). So like dynamic type, just dynamic memory space.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124244/new/
https://reviews.llvm.org/D124244
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
