[PATCH] D60793: [Evaluator] Walk initial elements when handling load through bitcast

[Robert Lougher via Phabricator via llvm-commits]

rob.lougher created this revision.
rob.lougher added reviewers: tejohnson, evgeny777, mtrofin.
Herald added a project: LLVM.

On x86-64, if the following program is compiled at -O1 or above, the value 0 will be incorrectly printed.

  #include <stdio.h>
  
  union A {
    A(long long ll) : l(ll) {}
    void *p;
    long long l;
  } u(12345);
  
  long long l = u.l;
  
  int main() {
    printf("%lld\n", l);
  }

This is caused by the incorrect evaluation of the load from "u.l".

On input to the evaluator we have a store instruction, storing 12345 into "u", and a load reading the value to initialize "l". Both of these instructions are through a bitcast:

  define linkonce_odr dso_local void @_ZN1AC2Ex(%union.A* %this, i64 %ll) unnamed_addr comdat align 2 {
    %l = bitcast %union.A* %this to i64*
    store i64 %ll, i64* %l, align 8
    ret void
  ...
  define internal void @__cxx_global_var_init.1() section ".text.startup" {
    %1 = load i64, i64* bitcast (%union.A* @u to i64*), align 8
    store i64 %1, i64* @l, align 8
    ret void
  }

When evaluating a store through a bitcast, the evaluator tries to move the bitcast from the pointer onto the stored value. So in this case we will go from:

store i64 %ll, i64* bitcast (%union.A* %this to i64*), align 8

to:

store %union.A bitcast (i64 %ll to %union.A), %union.A* %this, align 8

However, as the cast is invalid, it tries to "introspect" the type to get a valid cast by obtaining a pointer to the initial element.  This ends up storing into the mutator memory an inttoptr of "12345", to an i8* pointer (a GEP).

This is so far OK. However, when evaluating the load, we first try to lookup the bitcast in the mutated memory (which doesn't exist). We then try to lookup the bitcast "from" pointer which also doesn't exist. But the "from" pointer is a global variable with a definitive initializer, so we end up folding the load to zero.

What is missing is equivalent logic to the store to introspect the type (in this case to obtain an i8* GEP, which does exist in the mutated memory, the inttoptr value obviously being castable to i64).

Note, when developing the patch I was unhappy with adding similar logic directly to the load case as it could get out of step again (e..g. given the comment in the code regarding arrays). Instead, I have abstracted the "introspection" into a helper function, with the specifics being handled by a passed-in lambda function. I have also taken the opportunity to replace the "getInitializer" helper function with a lambda. This is a non-functional change, and can be done separately if preferred.


Repository:
  rL LLVM

https://reviews.llvm.org/D60793

Files:
  lib/Transforms/Utils/Evaluator.cpp
  test/Transforms/GlobalOpt/evaluate-bitcast-2.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60793.195415.patch
Type: text/x-patch
Size: 8379 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190416/8e3136f4/attachment.bin>