Cadd_cmpLTMask doesn't handle 64-bit tmp register properly

[Tom Rodriguez]

Yes the assembly is very broken.  I filed 7048332 for this.  The standard reporting location is through http://bugreport.sun.com/bugreport though that goes through some triage before it makes it to us.  It's ok to report concrete things here, particularly if you've fully diagnosed the problem.  ;)  Thanks for the thorough report.  This may make it into 7 but it should be in the next update at worst.

tom

On May 25, 2011, at 5:49 AM, Tuure Laurinolli wrote:

> Hello,
> 
> I spent yesterday tracing a bug in our software, and the trail points to
> Hotspot.
> 
> Extremely short version:
> 
> cadd_cmpLTMask doesnt handle 64-bit register as tmp, and generates
> subtly broken code if it receives one.
> 
> Slightly longer version:
> 
> After extensive inlining and elision of the full-check, the interesting
> part of
> 
> @Override
> public int size() {
>    if (full) {
>        path = 0;
>        return capacity();
>    }
> 
>    final int result;
>    if (last >= first) {
>            result = last - first;
>    } else {
>            result = last - first + capacity();
>    }
>    return result;
> }
> 
> compiles down to something like
> 
> 11d     subl    R10, RDX        # cadd_cmpLTMask1
>           sbbl    R11, R11
>           andl    R11, RBX
>           addl    R10, R11
> 127     movl    [rsp + #40], R10        # spill
> 
> where initially R10 is last, RBX is first, R8 is whatever and RCX is
> result of capacity(). However, looking at the code that is actually run
> with GDB, the disassembly is rather different:
> 
> 0x2aaaac856d7d:    sub    %edx,%r10d
> 0x2aaaac856d80:    sbb    %ebx,%ebx
> 0x2aaaac856d82:    and    %ebx,%ebx
> 0x2aaaac856d84:    add    %ebx,%r11d
> 0x2aaaac856d87:    mov    %r10d,0x28(%rsp)
> 
> Both and and add operate on odd registers, and the code also produces
> wrong result into [RSP+40]. Looking at the binary code, it's as follows:
> 
> (gdb) x/15xb 0x2aaaac856d7d
> 0x2aaaac856d7d:    0x44    0x2b    0xd2    0x1b    0xdb    0x23   
> 0xdb    0x44
> 0x2aaaac856d85:    0x03    0xdb    0x44    0x89    0x54    0x24    0x28
> 
> Notably the SBB instruction is missing REX.RB prefix, which changes it
> from SBBL R11 R11 into SBBL EBX EBX and clobbers the actual EBX. Also,
> the prefix of the ADD is only REX.R, whereas it probably should be
> REX.RB, and in any case the destination register is wrong.
> 
> Looking at x86_64.ad, it's clear that the code that generates this
> section (cadd_cmpLTMask) is not prepared to handle 64-bit tmp register.
> This explain missing prefixes, and also wrong destination register of
> ADD, since the high bit of reg clobbers the low bit or r/m in emit_rm().
> 
> Now, where do I report this bug? Presumably there is a bug tracker
> somewhere.
> 
> -- 
> 
> Tuure Laurinolli
> Software Engineer
> Indagon Ltd.
>