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

[Aleksey Shipilev]

On 07/16/2018 05:19 PM, Andrew Haley wrote:
> At present, the AArch64 back end does this when generating trampolines:
> 
>   if (far_branches() && !Compile::current()->in_scratch_emit_size()) {
>     address stub = emit_trampoline_stub(start_offset, entry.target());
> 
> This is only correct for C2 compilation. The class Compile is, despite
> its name, only present in C2, so we must check that we are
> C2-compiling before calling it.
> 
> http://cr.openjdk.java.net/~aph/8207345/

Looks good, mostly stylistic comments. Your call whether to change any of it:

*) Let's make NULL checks explicit:
 748     bool is_c2 = task && is_c2_compile(task->comp_level());

*) Also, seems useless to break the line here:
 749     bool in_scratch_emit_size
 750       = is_c2 && Compile::current()->in_scratch_emit_size();

*) Also, space after "!":
 751     if (! in_scratch_emit_size) {

*) Also, comment starts with "First, ...", and there is no "Second".

*) Maybe the whole thing does not need local variables, which would make the chained condition
clearer at expense of some code duplication.

In short, that's what I would do:

   // 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.
     if (ciEnv::current()->task() != NULL &&
         is_c2_compile(ciEnv::current()->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
       }
     }
   }


-Aleksey

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20180716/80e6fabd/signature.asc>