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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to