From 62b16ac07eb7d4abde697659964ef707aa61e8f8 Mon Sep 17 00:00:00 2001 From: Volker Berlin Date: Sat, 7 Mar 2020 17:01:59 +0100 Subject: [PATCH] Fix switch blocks with string cases and add tests. --- .../jwebassembly/module/BranchManger.java | 51 ++++++++-- .../module/WasmBlockInstruction.java | 9 ++ .../runtime/ControlFlowOperators.java | 98 +++++++++++++++++++ 3 files changed, 150 insertions(+), 8 deletions(-) diff --git a/src/de/inetsoftware/jwebassembly/module/BranchManger.java b/src/de/inetsoftware/jwebassembly/module/BranchManger.java index fd42992..ff46050 100644 --- a/src/de/inetsoftware/jwebassembly/module/BranchManger.java +++ b/src/de/inetsoftware/jwebassembly/module/BranchManger.java @@ -603,7 +603,7 @@ class BranchManger { int end = parsedBlock.endPosition; if( start < end ) { BranchNode branch = blockNode; - while( branch.size() > 0 /*&& start < branch.endPos*/ ) { + while( branch.size() > 0 ) { BranchNode node = branch.get( 0 ); if( start > node.endPos ) { if( end > branch.endPos ) { @@ -616,13 +616,13 @@ class BranchManger { BranchNode child = parentNode.remove( 0 ); parentNode.add( middleNode ); middleNode.add( child ); + patchBrDeep( middleNode ); } else { // occur if the default is not the last case in the switch middleNode.add( blockNode ); blockNode = middleNode; lastPosition = end; } - cases[posCount].block++; } break; } @@ -679,17 +679,52 @@ class BranchManger { switchNode.add( blockNode ); parent.add( switchNode ); - // sort back in the natural order and create a br_table - Arrays.sort( cases, ( a, b ) -> Long.compare( a.key, b.key ) ); - int[] data = new int[cases.length]; - for( int i = 0; i < data.length; i++ ) { - data[i] = cases[i].block; - } if( brTableNode != null ) { + // sort back in the natural order and create a br_table + Arrays.sort( cases, ( a, b ) -> Long.compare( a.key, b.key ) ); + int[] data = new int[cases.length]; + for( int i = 0; i < data.length; i++ ) { + data[i] = cases[i].block; + } brTableNode.data = data; } } + /** + * Patch the existing BR instructions after a new BLOCK node was injected in the hierarchy. The break position must + * only increment if the old break position is outside of the new BLOCK. + * + * @param newNode + * the new node + */ + private void patchBrDeep( BranchNode newNode ) { + for( WasmInstruction instr : instructions ) { + int codePos = instr.getCodePosition(); + if( codePos < newNode.startPos ) { + continue; + } + if( codePos >= newNode.endPos ) { + break; + } + if( instr.getType() != Type.Block ) { + continue; + } + WasmBlockInstruction blockInstr = (WasmBlockInstruction)instr; + if( blockInstr.getOperation() != WasmBlockOperator.BR ) { + continue; + } + Integer data = (Integer)blockInstr.getData(); + int blockCount = 0; + while( newNode.size() > 0 && newNode.endPos > codePos ) { + newNode = newNode.get( 0 ); + blockCount++; + } + if( blockCount <= data ) { // old break position was outside of the new block + blockInstr.setData( data + 1 ); + } + } + } + /** * Helper structure for caculateSwitch */ diff --git a/src/de/inetsoftware/jwebassembly/module/WasmBlockInstruction.java b/src/de/inetsoftware/jwebassembly/module/WasmBlockInstruction.java index 53b6519..ca3a132 100644 --- a/src/de/inetsoftware/jwebassembly/module/WasmBlockInstruction.java +++ b/src/de/inetsoftware/jwebassembly/module/WasmBlockInstruction.java @@ -71,6 +71,15 @@ class WasmBlockInstruction extends WasmInstruction { return op; } + /** + * Get the current data vale of the instruction + * + * @return the value + */ + Object getData() { + return data; + } + /** * Set a new value for the data * diff --git a/test/de/inetsoftware/jwebassembly/runtime/ControlFlowOperators.java b/test/de/inetsoftware/jwebassembly/runtime/ControlFlowOperators.java index a7a2d63..0369545 100644 --- a/test/de/inetsoftware/jwebassembly/runtime/ControlFlowOperators.java +++ b/test/de/inetsoftware/jwebassembly/runtime/ControlFlowOperators.java @@ -71,6 +71,15 @@ public class ControlFlowOperators extends AbstractBaseTest { addParam( list, script, "ifAndOr6" ); addParam( list, script, "ifAndOr8" ); addParam( list, script, "ifWithoutElseAndLoop" ); + addParam( list, script, "stringSwitchNormalFoo" ); + addParam( list, script, "stringSwitchNormalBar" ); + addParam( list, script, "stringSwitchNormalDefault" ); + addParam( list, script, "stringSwitchReverseFoo" ); + addParam( list, script, "stringSwitchReverseBar" ); + addParam( list, script, "stringSwitchReverseDefault" ); + addParam( list, script, "stringSwitchContinue1" ); + addParam( list, script, "stringSwitchContinue2" ); + addParam( list, script, "stringSwitchContinueDefault" ); } rule.setTestParameters( list ); return list; @@ -465,5 +474,94 @@ public class ControlFlowOperators extends AbstractBaseTest { } return ifWithoutElseAndLoop; } + + @Export + static int stringSwitchNormalFoo() { + return stringSwitchNormal( "foo" ); + } + + @Export + static int stringSwitchNormalBar() { + return stringSwitchNormal( "bar" ); + } + + @Export + static int stringSwitchNormalDefault() { + return stringSwitchNormal( "default" ); + } + + private static int stringSwitchNormal( String tagName ) { + switch( tagName ) { + case "foo": + return 1; + case "bar": + return 2; + default: + return 3; + } + } + + @Export + static int stringSwitchReverseFoo() { + return stringSwitchReverse( "foo" ); + } + + @Export + static int stringSwitchReverseBar() { + return stringSwitchReverse( "bar" ); + } + + @Export + static int stringSwitchReverseDefault() { + return stringSwitchReverse( "default" ); + } + + private static int stringSwitchReverse( String tagName ) { + switch( tagName ) { + default: + return 3; + case "bar": + return 2; + case "foo": + return 1; + } + } + + @Export + static int stringSwitchContinue1() { + return stringSwitchContinue( "1" ); + } + + @Export + static int stringSwitchContinue2() { + return stringSwitchContinue( "2" ); + } + + @Export + static int stringSwitchContinueDefault() { + return stringSwitchContinue( "8" ); + } + + /** + * Strings have continue hash codes that a compiler could use a tableswitch. + */ + private static int stringSwitchContinue( String tagName ) { + switch( tagName ) { + case "1": + return 1; + case "2": + return 2; + case "3": + return 3; + case "4": + return 4; + case "5": + return 5; + case "6": + return 7; + default: + return 8; + } + } } }