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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits