[PATCH] D127306: [analyzer] Treat system globals as mutable if they are not const

Thu Jun 9 07:35:24 PDT 2022

steakhal added a comment.

In D127306#3570083 <https://reviews.llvm.org/D127306#3570083>, @martong wrote:

> In D127306#3566984 <https://reviews.llvm.org/D127306#3566984>, @steakhal wrote:
>
>> In D127306#3566761 <https://reviews.llvm.org/D127306#3566761>, @martong wrote:
>>
>>>> In theory, the engine should propagate taint in default eval call. If that would be the case, we would still have this report.
>>>
>>> How complex would it be to add the taint propagation to the engine/evalCall? Do you see that feasible as a parent patch of this?
>>
>> I see your point, but I don't think we should delay D124244 <https://reviews.llvm.org/D124244> because of that. By landing this, we could also land that. And the change you propose might take significantly longer to make and verify.
>> I could make some prototypes, but I'm not making promises.
>
> Ok.
>
> One concern. Can we decide for a global **const system** variable if that it is a //system// variable? Consider `my_const_system_global`, it will be in `GlobalImmutableSpaceRegion`, however, it is also a //system// variable. Would it make sense to have a GlobalImmutable**System**SpaceRegion?

Well, there is more to this story. Constness and storage addressspace should be two different propery. Memspaces supposed to represent the latter. The mutability should have been designed as a GDM trait instead. It just happens to be that on some platforms there is a segment which is mapped as  read only. As far as memspaces goes, it shouldn't be important.

Consider some addspace attributed pointers. We should be able to conclude aliasing properties from just the type. In general (modulo weird cases), different addspace pointers should not refer to the same lvalue. Hence, the corresponding memspaces should also differ.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127306/new/

https://reviews.llvm.org/D127306