[PATCH] D64258: [InferFuncAttributes] extend 'dereferenceable' attribute based on loads

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D64258#1579214 <https://reviews.llvm.org/D64258#1579214>, @jfb wrote:

> >> Also, would it make sense to separate readable from writable? We currently have this bug where LLVM will promote all const static globals to rodata, and sometimes generate atomic cmpxchg to them (e.g. because we're trying to load a 128-bit value). Similarly, we might want to honor R / W memory protection in general. Right now dereferenceable just means "you can load from this", because we can't speculate most stores.
> > 
> > I do not understand the problem but I have the feeling this is an orthogonal issue.
>
> `mprotect` can make memory readable but not writable, or writable but not readable... or neither. What does `dereferenceable` mean when faced with this fact? Further, what happens to `dereferenceable` when `mprotect` is called (any opaque function could call it)? I don't think this is an orthogonal problem at all.


So, I guess what the above means is "dereferenceable" is too coarse grained. We have "global dereferenceability" that cannot be changed, and we have "local dereferenceability" that can be changed, e.g., through calls to `free`, `realloc`, or `mprotect`. From accesses we can only deduce "local dereferenceability". Now, that is why we need D61652 <https://reviews.llvm.org/D61652>, or more precisely, D63243 <https://reviews.llvm.org/D63243>. After those changes landed, the above reasoning and deduction should be fine, before, they  are as broken as Clang is when it emits `dereferenceable` for arguments passed by reference. (The logic above, with the same problems and more, is also used in ArgumentPromotion right now...).


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

https://reviews.llvm.org/D64258