Code generation patches for fuzzer (feedback requested)

[André Bargull]

(6) SlotStackMisaligned.patch

CodeGeneratorLexicalContext#pop() must only decrease "nextFreeSlotSize" 
for a given node when #nextFreeSlot() has been called for that node.
- pop() without nextFreeSlot() happens for anonymous function 
expressions which are copied in finally blocks, see early return in 
CodeGenerator#enterBlock(). Named function expressions don't show this 
bug because they are assigned new names for each copy. Yes, that means 
named function expressions (and function declarations) which are copied 
due to finally-inlining are generated multiple times in the class file...

test case:
function f(a) { try {} catch(x1 if (function(){})()) {} finally { try {} 
catch(y if (function(){})()) {} catch(x3 if (x3)) {} } }

- André

On 10/15/2013 1:16 PM, André Bargull wrote:
> (5) EnforceObjectTypeForNull.patch
>
> This one looks like a bug in asm's frame computation, adding a 
> `checkcast` instruction makes sure things work as expected...
>
> test cases:
> function f(){ try { var x = 1, x = null; } catch(x2) {} }
> function f(){ try { var x = 1; x = null; } catch(x2) {} return x }
>
> - André
>
> On 10/15/2013 10:45 AM, André Bargull wrote:
>> I've needed to apply the following patches to continue fuzzing. If 
>> those changes look reasonable, can someone make sure they get into 
>> the upstream repo?
>>
>> - André
>>
>>
>> (1) IncompatibleAssignmentCoercion.patch:
>>
>> `Type.widest(Type, Type)` can promote boolean -> number, or number -> 
>> string. This is not wanted for (?:) - ConditionalExpressions or 
>> ReturnStatements. Added a new uncoercedWidest(Type, Type) method to 
>> Attr which makes sure only valid promotions are applied.
>>
>> test cases:
>> /* no verify error */ function f1(c,x){ c (x ? [1] : 1); }
>> function f2(c,x){ c = (x ? "123" : 1); return c }
>> typeof f2(null,true) === "string"
>> typeof f2(null,false) === "number"
>> function f3(v){if (v) return 123; else return true}
>> f3(true) === 123
>> f3(false) === true
>>
>>
>> (2) JDK-8026249-WidenDISCARD.patch:
>>
>> The left-hand side of a BinaryNode does not have a Type if it is a 
>> Discard node, that means `lhs.getType()` may throw an assertion 
>> error. Moving the Type.widest() calls makes sure `lhs.getType()` is 
>> only called when it is safe to do.
>>
>> test case:
>> /* no assertion */ function f() { if(x3, y) x; }
>>
>>
>> (3) JDK-8026249-EnsureTypeDiscard.patch:
>>
>> The type for a CommaRight BinaryNode is based on the right-hand 
>> side's type, make sure the RHS always has a proper type attached. 
>> [I've also fixed this for CommaLeft, but that one isn't actually 
>> used, is it?]
>>
>> test case:
>> /* no assertion */ function f(x) { return y, x }
>>
>>
>> (4) JDK-8026249-ControlFlowEscape.patch:
>>
>> Control flow may escape from SwitchNode or LabelNode through `break` 
>> or `continue` statements. Simply moving the "controlFlowEscapes" 
>> logic from LoopNode to BreakableStatement solves the SwitchNode 
>> issue. And for LabelNode just clear the "terminal" on its body node.
>>
>> test cases:
>> /* no verify error / NPE */ function f() { L: { try { return 1; } 
>> finally { break L; }} }
>> /* no verify error / NPE */ function f() { L: { if (v) break L; 
>> return} }
>> /* no verify error / NPE */ function f() { L: {{ break L; } return; } }
>> /* no verify error / NPE */ function f() { L: { if (x2) { break L; } 
>> throw x; } }
>> /* no verify error / NPE */ function f() { switch(v) { default: if(v) 
>> break; return; } }
>> /* no verify error / NPE */ function f() { switch(x) { default: 
>> if(true) break; return; } }
>> /* no verify error / NPE */ function f() { switch(x) { default: L: 
>> break; return; } }
>

-------------- next part --------------
diff --git a/src/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java b/src/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java
--- a/src/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java
+++ b/src/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java
@@ -70,6 +70,9 @@
     /** size of next free slot vector */
     private int nextFreeSlotsSize;
 
+    /** Stack tracking the currently active blocks */
+    private Deque<Block> blocks = new ArrayDeque<>();
+
     @Override
     public <T extends LexicalContextNode> T push(final T node) {
         if (isDynamicScopeBoundary(node)) {
@@ -84,7 +87,9 @@
         if (isDynamicScopeBoundary(popped)) {
             --dynamicScopeCount;
         }
-        if (node instanceof Block) {
+        if (node instanceof Block && blocks.peek() == node) {
+            assert nextFreeSlotsSize > 0;
+            blocks.pop();
             --nextFreeSlotsSize;
         }
         return popped;
@@ -200,6 +205,7 @@
             nextFreeSlots = newNextFreeSlots;
         }
         nextFreeSlots[nextFreeSlotsSize++] = assignSlots(block, nextFreeSlot);
+        blocks.push(block);
     }
 
     private static int assignSlots(final Block block, final int firstSlot) {