[polly] r281052 - FlattenAlgo: Ensure we _really_ obtain a param space

[Michael Kruse via llvm-commits]

2016-09-09 22:22 GMT+02:00 Tobias Grosser via llvm-commits
<llvm-commits at lists.llvm.org>:
> On Fri, Sep 9, 2016, at 06:50 PM, Tobias Grosser via llvm-commits wrote:
>> On Fri, Sep 9, 2016, at 06:37 PM, Michael Kruse via llvm-commits wrote:
>> > Thank you for finding a fix.
>> >
>> > This is somewhat strange. Isn't isl_union_map_get_space defined to
>> > return a parameter space?
>>
>> Right.
>>
>> > What is different on those machines where it
>> > happened?
>>
>> They are owned by me. Seriously, I have no idea. This is very strange. I
>> spent 30 minutes to look into this but was not able to find anything
>> useful. :(
>
> OK. I spent a couple of hours to look into this. No idea what is going
> on still. I was first suspecting the new C++ interface to incorrectly
> free/deallocate memory. However, I could not find any issue and valgrind
> also does not report anything.
>
> Then I just went to printf debugging. I attach you both my patch and the
> typescript.
>
> The interesting piece is this:
>
> checkFlatten - SPtr: 18302032
> checkFlatten - isParams :  is_params: space 18298960
>  a
>  b
> space->tuple_id[0] 8021840
> isl_id_none 8021840
>  c
>  d
> 1
> copy
> Take ownership: 0
> Move That2
> copy
> flattenSchedule - SchedulePtr: 18302032
> flattenSchedule - isParams:  is_params: space 18298960
>  a
>  b
> space->tuple_id[0] 8021840
> isl_id_none 140647671602904
>
> The schedule remains the same when moving into flattenSchedule and the
> space as well, even the id in space->tuple_id[0]. However, it seems the
> address of the global isl_id_none changes. I tried to run this in gdb
> and run 'watch  isl_id_none', but do not observe any change. I would
> expect this address to stay constant at all times. I feel as if I missed
> something very basic. Any idea what is going on?

Thank you for your debugging efforts. This is as puzzling for me as for you.

If the code is not compiled using -fpic, "&isl_id_none" should turn
into a constant.

0115E1A6  push        offset isl_id_none (01441810h)
0115E1AB  push        1426678h
0115E1B0  call        _printf (0E6E9C5h)
0115E1B5  add         esp,8
(MSVC 32-bit disassembly, debug build)

With -fpic, it becomes relative to the instruction pointer (%rip)

   0x00000000004e73b1 <+129>:   lea    0x2bd2e8(%rip),%rbp        #
0x7a46a0 <isl_id_none>
   0x00000000004e73b8 <+136>:   lea    0x86d14(%rip),%rsi        # 0x56e0d3
   0x00000000004e73bf <+143>:   xor    %eax,%eax
   0x00000000004e73c1 <+145>:   mov    $0x1,%edi
   0x00000000004e73c6 <+150>:   mov    %rbp,%rdx
   0x00000000004e73c9 <+153>:   callq  0x404730 <__printf_chk at plt>
(gdb 64-bit disassembly, release build)


Your new address 0x7FEB16811ED8 (140647671602904) would indicate a
location on the stack. With this code, I have no idea how this could
print different values in the middle of the program's execution
(except the loader decided to relocate the .text section while
running?!?)

If you are interested into looking further into it, you could try to
compile without -fpic. Or debug with gdb until before it changes, step
forward while observing &isl_id_none, maybe even tracking it through
the registers. Does gdb even evaluate it to the same value as printf
prints? Btw, 'watch  isl_id_none' would observe isl_id_none's
contents, not its address.

Another idea: try to write to that location with their supposed value.
If the address is not the real isl_id_none location, the program
should run havok at that point.
isl_id_none.ref = -1;
isl_id_none.ctx = NULL;
isl_id_none.name = "#none";
isl_id_none.user = .NULL;
(or just add an assertion comparing their values to the expected one)


Michael