[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

[Gabor Marton via Phabricator via cfe-commits]

martong added a subscriber: vabridgers.
martong added a comment.

Adding @vabridgers as a subscriber, he might be interested in this.



================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1675
+       //  belong to an array with one element of type T.
+       // Hence, the first element can be retrieved only. At least untill a
+       // paper P1839R0 be considered by the committee.
----------------
typo: `untill` -> `until`


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1758
+              // type.
+              QualType ElemT = Ctx.getCanonicalType(R->getElementType());
+              if (!canAccessStoredValue(ArrT, ElemT, I))
----------------
steakhal wrote:
> If you already compute the //canonical type// why do you recompute in the `canAccessStoredValue()`?
> You could simply assert that instead.
He removes the qualifiers there, but getting the canonical type is probably redundant here.


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1759-1760
+              QualType ElemT = Ctx.getCanonicalType(R->getElementType());
+              if (!canAccessStoredValue(ArrT, ElemT, I))
+                return UndefinedVal();
+
----------------
ASDenysPetrov wrote:
> steakhal wrote:
> > Even though I agree with you, I think it would be useful to hide this behavior behind an analyzer option.
> > There is quite a lot code out in the wild that violate the //strict-aliasing// rule and they probably pass the `-fno-strict-aliasing` compiler flag to accommodate this in codegen. AFAIK Linux is one of these projects for example.
> > So, I think there is a need to opt-out of this and/or bind the behavior to the presence of the mentioned compiler flag.
> > 
> > By not doing this, the user would get //garbage// value reports all over the place.
> > @NoQ @martong WDYT?
> > There is quite a lot code out in the wild that violate the strict-aliasing rule 
> Agree.
> > By not doing this, the user would get garbage value reports all over the place.
> Definitely.
> Using the flag is a good option. But the question whether to use existing `-fno-strict-aliasing` or introduce a new one?
I think we could simply reuse the existing `-fno-strict-aliasing`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110927/new/

https://reviews.llvm.org/D110927