[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type
Denys Petrov via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 28 11:45:03 PDT 2021
ASDenysPetrov added a comment.
@steakhal I'll address all of your remarks. Thanks a lot!
================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1630-1641
+/// Returns true if the stored value can be accessed through the pointer to
+/// another type:
+/// const int arr[42] = {};
+/// auto* pchar = (char*)arr;
+/// auto* punsigned = (unsigned int*)arr;
+/// auto* pshort = (short*)arr;
+/// auto x1 = pchar[0]; // valid
----------------
steakhal wrote:
> This is basically the //strict-aliasing// rule, we could probably mention this.
> Although, I don't mind the current name.
> You probably know about it, but Shafik made a fancy [[ https://gist.github.com/shafik/848ae25ee209f698763cffee272a58f8 | GitHub Gist ]] about it.
> The only thing I missed was checking //type similarity// according to [[https://eel.is/c++draft/basic.lval#11 | basic.lval ]], but I'm not exactly sure if we should check that here.
> You probably know about it, but Shafik made a fancy GitHub Gist about it.
Yes, this is the one of those things which inspired me to take care of aliasing as a part of RegionStoreManager improvements.
================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1759-1760
+ QualType ElemT = Ctx.getCanonicalType(R->getElementType());
+ if (!canAccessStoredValue(ArrT, ElemT, I))
+ return UndefinedVal();
+
----------------
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?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110927/new/
https://reviews.llvm.org/D110927
More information about the cfe-commits
mailing list