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