[llvm-dev] Possible soundness issue with available_externally (split from "RFC: Add guard intrinsics")

[Mehdi Amini via llvm-dev]

> On Feb 26, 2016, at 9:03 AM, Sanjoy Das via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> 
> Couple of other problematic transforms:
> 
> # undef refinement
> 
> After thinking about this a bit, I think undef refinement happens a
> lot more often than I initially thought, and it happens implicitly.
> 
> Consider the following case:
> 
>  void @foo(int* ptr) available_externally {
>    int k = *ptr;
>    if (k == 1 && k == 2) print("X");
>  }
> 
>  void main() {
>    int* ptr = malloc();
>    *ptr = 200;
>    @foo(ptr)
>  }
> 
> =>
> 
>  void @foo(int* ptr) readnone available_externally {
>    //int k = *ptr;
>    //if (false) print("X");
>  }
> 
>  void main() {
>    int* ptr = malloc();
>    *ptr = 200;
>    @foo(ptr)
>  }
> 
> ==>
> 
>  void @foo(int* ptr) readnone available_externally {
>    //int k = *ptr;
>    //if (false) print("X");
>  }
> 
>  void main() {
>    int* ptr = malloc();
>    @foo(ptr) // since this is readnone
>    *ptr = 200;
>  }
> 
> 
> This is a problem if we replace @foo with an unoptimized version
> (`undef` can be both `== 1` and `== 2`): in such a case we can print
> `"X"` while in the original program that wasn't possible.


This is very interesting: until now in this thread we were considering (IIUC) only function replacement at link time / run time.
Now you are considering replacing at any point of the optimizer an ODR (or available externally) with another definition (its original or an intermediate "differently" optimized one).

This is exactly what is happening during an LTO build! ODR will be optimized in first compile phase as usual, and then the linker will merge the IR for all modules in a single one, keeping only one definition for each ODR, before re-running the optimizer.

Scary...

-- 
Mehdi




> 
> The problem here is that we're "implicitly" folding / constraining
> `undef` when folding `(k == 1 && k == 2)` to `false`.  In the case
> where `k` is `undef` (the non-undef case is trivial), the fold is
> justified by first constraining the two `undef` s to one value, and
> then folding the comparison.  And we do this kind of implicit `undef`
> refinement all over the place -- the rewrite
> 
>  i = 0;
>  N = ...;
>  do {
>  } while (i++ != N);
> 
> =>
> 
>  i = 0;
>  N = ...;
>  //do {
>  //} while (i++ != N);
>  i = N + 1;
> 
> makes the same refinement, that **if** `N` is `undef` then all of the
> uses of `N` through the loops iteration space are the same `N`.  OTOH
> If we *knew* `N` was `undef`, we could have correctly replaced the
> loop with `while (true)` (so that above loop is also problematic in
> the same way as the firat `k == 1 && k == 2` case.
> 
> 
> # dereferenceability
> 
> We cannot reorder `readnone nounwind available_externally` functions
> across `@free` since there could have been:
> 
> void @foo(int* ptr) available_externally {
> int unused = *ptr;
> }
> 
> where the load was optimized away in the current TU but is present in
> the -O0 TU.
> 
> This is similar to the dead-arg-elimination transform Andy Ayers and I
> mentioned earlier (passing in `undef` or `null` for `ptr` is also a
> problem for similar reasons).
> 
> 
> -- Sanjoy
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev