[analyzer] Treat system globals as mutable if they are not const
Previously, system globals were treated as immutable regions, unless it was the `errno` which is known to be frequently modified. D124244 wants to add a check for stores to immutable regions. It would basically turn all stores to system globals into an error even though we have no reason to believe that those mutable sys globals should be treated as if they were immutable. And this leads to false-positives if we apply D124244. In this patch, I'm proposing to treat mutable sys globals actually mutable, hence allocate them into the `GlobalSystemSpaceRegion`, UNLESS they were declared as `const` (and a primitive arithmetic type), in which case, we should use `GlobalImmutableSpaceRegion`. In any other cases, I'm using the `GlobalInternalSpaceRegion`, which is no different than the previous behavior. --- In the tests I added, only the last `expected-warning` was different, compared to the baseline. Which is this: ```lang=C++ void test_my_mutable_system_global_constraint() { assert(my_mutable_system_global > 2); clang_analyzer_eval(my_mutable_system_global > 2); // expected-warning {{TRUE}} invalidate_globals(); clang_analyzer_eval(my_mutable_system_global > 2); // expected-warning {{UNKNOWN}} It was previously TRUE. } void test_my_mutable_system_global_assign(int x) { my_mutable_system_global = x; clang_analyzer_eval(my_mutable_system_global == x); // expected-warning {{TRUE}} invalidate_globals(); clang_analyzer_eval(my_mutable_system_global == x); // expected-warning {{UNKNOWN}} It was previously TRUE. } ``` --- Unfortunately, the taint checker will be also affected. The `stdin` global variable is a pointer, which is assumed to be a taint source, and the rest of the taint propagation rules will propagate from it. However, since mutable variables are no longer treated immutable, they also get invalidated, when an opaque function call happens, such as the first `scanf(stdin, ...)`. This would effectively remove taint from the pointer, consequently disable all the rest of the taint propagations down the line from the `stdin` variable. All that said, I decided to look through `DerivedSymbol`s as well, to acquire the memregion in that case as well. This should preserve the previously existing taint reports. Reviewed By: martong Differential Revision: https://reviews.llvm.org/D127306
Loading
Please sign in to comment