RFR: 8207345: AArch64: Trampoline generation code reads from uninitialized memory

[Andrew Haley]

On 07/16/2018 05:53 PM, Aleksey Shipilev wrote:
> On 07/16/2018 06:42 PM, Aleksey Shipilev wrote:
>> On 07/16/2018 06:40 PM, Andrew Haley wrote:
>>> On 07/16/2018 05:38 PM, Aleksey Shipilev wrote:
>>>> Your choice, really. I used to frown about conditionals like this all around Hotspot, but then I
>>>> started to see the appeal: you can clearly see (A && B && C) here, even if A, B, C are not trivial.
>>>> But structurally, you immediately grasp that if A fails, we don't enter the block; if B fails, we
>>>> don't enter the block, if C fails, we don't enter the block. It takes some time to read through
>>>> chain of local variables to reach the same verdict.
>>>
>>> I see.  I tried it, and I think your suggestions do help, and the
>>> hybrid result is best of all:
>>>
>>>   // We need a trampoline if branches are far.
>>>   if (far_branches()) {
>>>     // We don't want to emit a trampoline if C2 is generating dummy
>>>     // code during its branch shortening phase.
>>>     CompileTask* task = ciEnv::current()->task();
>>>     if (task != NULL && is_c2_compile(task->comp_level()) &&
>>>         Compile::current->in_scratch_emit_size()) {
>>>       address stub = emit_trampoline_stub(offset(), entry.target());
>>>       if (stub == NULL) {
>>>         return NULL; // CodeCache is full
>>>       }
>>>     }
>>
>> Looks good.
> 
> ...except that it should be !Compile::current->in_scratch_emit_size()? (I made this mistake when
> rewriting, sorry).

LOL!  It's simpler because it's actually wrong!  :-)

Using the form above, it actually should be

  // We need a trampoline if branches are far.
  if (far_branches()) {
    // We don't want to emit a trampoline if C2 is generating dummy
    // code during its branch shortening phase.
    CompileTask* task = ciEnv::current()->task();
    if (task != NULL
        && ! (is_c2_compile(task->comp_level())
              && Compile::current()->in_scratch_emit_size())) {
      address stub = emit_trampoline_stub(offset(), entry.target());
      if (stub == NULL) {
        return NULL; // CodeCache is full
      }
    }
  }

i.e. we want a trampoline if we're using C1 or (we're using C2 and not in scratch emit).

Back to Plan A, I think.  :-)

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671