Issue Details (XML | Word | Printable)

Key: HARMONY-6065
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Alexei Fedotov
Reporter: Andrew Cornwall
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
Harmony

[drlvm][testing] VMTT generates bad lookupswitch code 25% of the time

Created: 06/Jan/09 08:58 PM   Updated: 02/Mar/09 06:04 PM
Return to search
Component/s: DRLVM
Affects Version/s: 5.0M8
Fix Version/s: 5.0M9

Time Tracking:
Not Specified

File Attachments:
  Size
File Licensed for inclusion in ASF works 6065-switchpadding.diff 2009-01-15 05:29 PM Andrew Cornwall 1 kB

Resolution Date: 27/Feb/09 11:05 PM


 Description  « Hide
The VMTT tool appears to generate bad bytecode when the lookupswitch instruction starts at instruction (4n-1). According to the spec, this code:

0: nop
1: nop
2: iload_1
3: lookupswitch {
1: 44
4: 47
9: 51
16: 53
default: 57
}
should generate this output:
// 0: nop
           x00
           // 1: nop
           x00
           // 2: iload_1
           x1B
          /* lookupswitch size is:
           1: lookupswitch
           0: padding
           4: default address
           4: npairs
           4: case 1 match (1)
           4: case 1 value
           4: case 2 match (4)
           4: case 2 value
           4: case 3 match (9)
           4: case 3 value
           4: case 4 match (16)
           4: case 4 value
           Total: 41
          */
           // 3: lookupswitch {
           // 1: 44
           // 4: 47
           // 9: 51
           // 16: 53
           // default: 57
           // }
           xAB // lookupswitch
           // no padding
           x00 x00 x00 x36 // default: 3+54=57
           x00 x00 x00 x04 // npairs: 4
           x00 x00 x00 x01 // match: 1
           x00 x00 x00 x29 // offset 3+41=44
           x00 x00 x00 x04 // match: 4
           x00 x00 x00 x2C // offset 3+44=47
           x00 x00 x00 x09 // match: 9
           x00 x00 x00 x30 // offset 3+48=51
           x00 x00 x00 x10 // match: 16
           x00 x00 x00 x32 // offset 3+50=53

Instead, it generates this output:
// 0: nop
           x00
           // 1: nop
           x00
           // 2: iload_1
           x1B
          /* lookupswitch size is:
           1: lookupswitch
           4: padding - looks like VMTT is giving us 4 bytes?!?
           4: default address
           4: npairs
           4: case 1 match (1)
           4: case 1 value
           4: case 2 match (4)
           4: case 2 value
           4: case 3 match (9)
           4: case 3 value
           4: case 4 match (16)
           4: case 4 value
           Total: 41
          */
           // 3: lookupswitch {
           // 1: 44
           // 4: 47
           // 9: 51
           // 16: 53
           // default: 57
           // }
           xAB // lookupswitch
           x00 x00 x00 x00 // 4 bytes of padding?
           x00 x00 x00 x36 // default: 3+54=57
           x00 x00 x00 x04 // npairs: 4
           x00 x00 x00 x01 // match: 1
           x00 x00 x00 x29 // offset 3+41=44
           x00 x00 x00 x04 // match: 4
           x00 x00 x00 x2C // offset 3+44=47
           x00 x00 x00 x09 // match: 9
           x00 x00 x00 x30 // offset 3+48=51
           x00 x00 x00 x10 // match: 16
           x00 x00 x00 x32 // offset 3+50=53

In other words, it's emitting 4 bytes of padding instead of 0 bytes of padding after the xAB lookupswitch instruction. This is 4-aligned, but not what most VMs expect.

 All   Comments   Work Log   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Andrew Cornwall added a comment - 06/Jan/09 09:00 PM
The second lookupswitch size above should read Total: 45. The fact that it's 45 bytes long and not 41 bytes long is the defect.

Andrew Cornwall added a comment - 06/Jan/09 11:42 PM
It looks as if tableswitch has the same problem (VMTT should insert 0 bytes padding, but instead inserts 4 bytes padding.)

Andrew Cornwall added a comment - 15/Jan/09 05:29 PM
The attached file 6065-switchpadding.diff fixes the defect.

Alexei Fedotov added a comment - 27/Feb/09 11:05 PM
Committed at revision 748733. I have an excuse for committing this during freeze: VMTT is not a part of the release.

Alexei Fedotov added a comment - 27/Feb/09 11:06 PM
Andrew, please verify the fix.

Andrew Cornwall added a comment - 02/Mar/09 06:04 PM
That's a very pretty fix!

Issue verified. Thanks.