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

ASDenysPetrov abandoned this revision. ASDenysPetrov added a comment. Temporary suspended this revision in favor of making a new checker //StrictAliasingChecker//, which would define an access to values through unpermited types as Unefined Behavior according to certain statements of the

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

ASDenysPetrov added a comment. @steakhal Please, read the discussion started from here D104285#2943449 . It's directly relates to this patch and what we've been arguing about. I'm still hesitating about this patch. On one hand we have the fact that

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

ASDenysPetrov added a comment. @NoQ > If we ever prove that strict aliasing is violated on a given execution path > (while being enabled), the ideal thing to do is to terminate the analysis > immediately by generating a sink. We can then optionally develop a checker > that emits a warning in

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

NoQ added a comment. Taking advantage of strict aliasing is good as long as it produces strictly smaller analysis space (less paths, more constrained states). I.e., we can use it for eliminating possibilities, but not for discovering possibilities. If we ever prove that strict aliasing is

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

steakhal added inline comments. Comment at: clang/test/Analysis/initialization.cpp:295-299 +void glob_cast_opposite_sign1() { + auto *ptr = (unsigned int *)glob_arr2; + auto x1 = ptr[0]; // no-warning + auto x2 = ptr[1]; // expected-warning{{garbage or undefined}} +}

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

ASDenysPetrov added inline comments. Comment at: clang/test/Analysis/initialization.cpp:295-299 +void glob_cast_opposite_sign1() { + auto *ptr = (unsigned int *)glob_arr2; + auto x1 = ptr[0]; // no-warning + auto x2 = ptr[1]; // expected-warning{{garbage or undefined}} +}

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

steakhal added inline comments. Comment at: clang/test/Analysis/initialization.cpp:295-299 +void glob_cast_opposite_sign1() { + auto *ptr = (unsigned int *)glob_arr2; + auto x1 = ptr[0]; // no-warning + auto x2 = ptr[1]; // expected-warning{{garbage or undefined}} +}

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

ASDenysPetrov added a comment. @steakhal > I think I know. Great! Thank you! Comment at: clang/test/Analysis/initialization.cpp:295-299 +void glob_cast_opposite_sign1() { + auto *ptr = (unsigned int *)glob_arr2; + auto x1 = ptr[0]; // no-warning + auto x2 = ptr[1]; //

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

steakhal added a comment. In D110927#3118936 , @ASDenysPetrov wrote: >> You could have a parameter, and take its address to accomplish your >> reinterpret casts and type puns. > > Do you mean: ... > If so, IMO it doesn't matter. I see. Sorry about the

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

ASDenysPetrov added a comment. In D110927#3117728 , @steakhal wrote: > For testing this I would recommend using a separate test file. > That being said, you should avoid globals even in tests when you can. The > distance between its declaration and use

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

steakhal added a comment. For testing this I would recommend using a separate test file. That being said, you should avoid globals even in tests when you can. The distance between its declaration and use just makes it harder to comprehend and reason about. You could have a parameter, and take

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

ASDenysPetrov added a comment. Ping. Does anyone know how to check the status of`-fno-strict-aliasing` flag from CSA side? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110927/new/ https://reviews.llvm.org/D110927 ___ cfe-commits mailing

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

ASDenysPetrov updated this revision to Diff 383812. ASDenysPetrov added a comment. Updated according to comments. TODO: make the feature `-fno-strict-aliasing` dependent. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110927/new/ https://reviews.llvm.org/D110927 Files:

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

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

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

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]

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

steakhal added inline comments. 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*

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

ASDenysPetrov added a comment. In D110927#3037118 , @shafik wrote: > IIUC the object is `const int arr[42]` and the `(char *)arr` is an expression > of pointer type and adding `1` to this is valid. The case you refer to in > D104285

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

ASDenysPetrov updated this revision to Diff 376823. ASDenysPetrov added a comment. Fixed a comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110927/new/ https://reviews.llvm.org/D110927 Files: clang/lib/StaticAnalyzer/Core/RegionStore.cpp

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

shafik added a comment. In D110927#3036647 , @ASDenysPetrov wrote: > In D110927#3036436 , @steakhal > wrote: > >> I'm pretty sure that `int x4 = ((char*)arr)[1];` is supposed to be valid in >> your summary. >>

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

ASDenysPetrov added a comment. In D110927#3036436 , @steakhal wrote: > I'm pretty sure that `int x4 = ((char*)arr)[1];` is supposed to be valid in > your summary. > I think it's allowed by the standard to access any valid object via a `char*` > -

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

steakhal added a subscriber: shafik. steakhal added a comment. I'm pretty sure that `int x4 = ((char*)arr)[1];` is supposed to be valid in your summary. I think it's allowed by the standard to access any valid object via a `char*` - according to the strict aliasing rule. @shafik WDYT?

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

ASDenysPetrov created this revision. ASDenysPetrov added reviewers: aaron.ballman, martong, steakhal, NoQ, r.stahl. ASDenysPetrov added a project: clang. Herald added subscribers: manas, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.