[all-commits] [llvm/llvm-project] 3542fe: [SCCP] Do not replace deref'able ptr with un-deref...

[Florian Hahn via All-commits]

  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 3542feeb2077f267bff1ab98fb4bf20099f44bb8
      https://github.com/llvm/llvm-project/commit/3542feeb2077f267bff1ab98fb4bf20099f44bb8
  Author: Florian Hahn <flo at fhahn.com>
  Date:   2020-09-03 (Thu, 03 Sep 2020)

  Changed paths:
    M llvm/lib/Transforms/Scalar/SCCP.cpp
    M llvm/test/Transforms/SCCP/apint-bigint2.ll
    M llvm/test/Transforms/SCCP/indirectbr.ll
    M llvm/test/Transforms/SCCP/replace-dereferenceable-ptr-with-undereferenceable.ll

  Log Message:
  -----------
  [SCCP] Do not replace deref'able ptr with un-deref'able one.

Currently IPSCCP (and others like CVP/GVN) blindly propagate pointer
equalities. In certain cases, that leads to dereferenceable pointers
being replaced, as in the example test case.

I think this is not allowed, as it introduces an access of an
un-dereferenceable pointer. Note that the pointer is inbounds, but one
past the last element, so it is valid, but not dereferenceable.

This patch is mostly to highlight the issue and start a discussion.
Currently it only checks for specifically looking
one-past-the-last-element pointers with array typed bases.

This causes the mis-compile outlined in
https://stackoverflow.com/questions/55754313/is-this-gcc-clang-past-one-pointer-comparison-behavior-conforming-or-non-standar

In the test case, if we replace %p with the GEP for the store, we
subsequently determine that the store and the load cannot alias, because
they are to different underlying objects.

Note that Alive2 seems to think that the replacement is valid:
https://alive2.llvm.org/ce/z/2rorhk

Reviewed By: efriedma

Differential Revision: https://reviews.llvm.org/D85332