# HG changeset patch # User attila # Date 1430843729 -7200 # Node ID 357f9a3f939491f3daeb5e73e4ff0af4b21d178f # Parent 35e1a33f3d12eb69291b8723cfd074bae399253c 8079269: Optimistic rewrite in object literal causes ArrayIndexOutOfBoundsException Reviewed-by: hannesw, lagergren diff -r 35e1a33f3d12 -r 357f9a3f9394 nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java --- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java Tue May 05 14:30:00 2015 +0200 +++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java Tue May 05 18:35:29 2015 +0200 @@ -1275,7 +1275,7 @@ return true; } - private boolean useOptimisticTypes() { + boolean useOptimisticTypes() { return !lc.inSplitNode() && compiler.useOptimisticTypes(); } diff -r 35e1a33f3d12 -r 357f9a3f9394 nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/FieldObjectCreator.java --- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/FieldObjectCreator.java Tue May 05 14:30:00 2015 +0200 +++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/FieldObjectCreator.java Tue May 05 18:35:29 2015 +0200 @@ -127,6 +127,8 @@ method.invoke(constructorNoLookup(className, PropertyMap.class)); } + helpOptimisticRecognizeDuplicateIdentity(method); + // Set values. final Iterator> iter = tuples.iterator(); @@ -136,6 +138,7 @@ //if we didn't load, we need an array property if (tuple.symbol != null && tuple.value != null) { final int index = getArrayIndex(tuple.key); + method.dup(); if (!isValidArrayIndex(index)) { putField(method, tuple.key, tuple.symbol.getFieldIndex(), tuple); } else { @@ -164,8 +167,6 @@ * @param tuple Tuple to store. */ private void putField(final MethodEmitter method, final String key, final int fieldIndex, final MapTuple tuple) { - method.dup(); - final Type fieldType = codegen.useDualFields() && tuple.isPrimitive() ? PRIMITIVE_FIELD_TYPE : Type.OBJECT; final String fieldClass = getClassName(); final String fieldName = getFieldName(fieldIndex, fieldType); @@ -187,7 +188,6 @@ * @param tuple Tuple to store. */ private void putSlot(final MethodEmitter method, final long index, final MapTuple tuple) { - method.dup(); if (JSType.isRepresentableAsInt(index)) { method.load((int)index); } else { diff -r 35e1a33f3d12 -r 357f9a3f9394 nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/ObjectCreator.java --- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/ObjectCreator.java Tue May 05 14:30:00 2015 +0200 +++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/ObjectCreator.java Tue May 05 18:35:29 2015 +0200 @@ -146,4 +146,28 @@ return loadTuple(method, tuple, true); } + /** + * If using optimistic typing, let the code generator realize that the newly created object on the stack + * when DUP-ed will be the same value. Basically: {NEW, DUP, INVOKESPECIAL init, DUP} will leave a stack + * load specification {unknown, unknown} on stack (that is "there's two values on the stack, but neither + * comes from a known local load"). If there's an optimistic operation in the literal initializer, + * OptimisticOperation.storeStack will allocate two temporary locals for it and store them as + * {ASTORE 4, ASTORE 3}. If we instead do {NEW, DUP, INVOKESPECIAL init, ASTORE 3, ALOAD 3, DUP} we end up + * with stack load specification {ALOAD 3, ALOAD 3} (as DUP can track that the value it duplicated came + * from a local load), so if/when a continuation needs to be recreated from it, it'll be + * able to emit ALOAD 3, ALOAD 3 to recreate the stack. If we didn't do this, deoptimization within an + * object literal initialization could in rare cases cause an incompatible change in the shape of the + * local variable table for the temporaries, e.g. in the following snippet where a variable is reassigned + * to a wider type in an object initializer: + * var m = 1; var obj = {p0: m, p1: m = "foo", p2: m} + * @param method the current method emitter. + */ + void helpOptimisticRecognizeDuplicateIdentity(final MethodEmitter method) { + if (codegen.useOptimisticTypes()) { + final Type objectType = method.peekType(); + final int tempSlot = method.defineTemporaryLocalVariable(objectType.getSlots()); + method.storeHidden(objectType, tempSlot); + method.load(objectType, tempSlot); + } + } } diff -r 35e1a33f3d12 -r 357f9a3f9394 nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/SpillObjectCreator.java --- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/SpillObjectCreator.java Tue May 05 14:30:00 2015 +0200 +++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/SpillObjectCreator.java Tue May 05 18:35:29 2015 +0200 @@ -146,6 +146,8 @@ // instantiate the script object with spill objects method.invoke(constructorNoLookup(objectClass, PropertyMap.class, long[].class, Object[].class)); + helpOptimisticRecognizeDuplicateIdentity(method); + // Set prefix array data if any if (arrayData.length() > 0) { method.dup(); diff -r 35e1a33f3d12 -r 357f9a3f9394 nashorn/test/script/basic/JDK-8079269.js --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/nashorn/test/script/basic/JDK-8079269.js Tue May 05 18:35:29 2015 +0200 @@ -0,0 +1,312 @@ +/* + * Copyright (c) 2015 Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * JDK-8079269: Optimistic rewrite in object literal causes ArrayIndexOutOfBoundsException + * + * @test + * @run + */ + +// m must be in scope so it's accessed with optimistic getters on scope +var m = 1; + +(function() { + return { + p0: m, + p1: m = "foo", + p2: m + } +})(); + +var n = 1; + +// Test the spill object creator too +(function() { + return { + p0: n, + p1: n = "foo", + p2: n, + p3: n, + p4: n, + p5: n, + p6: n, + p7: n, + p8: n, + p9: n, + p10: n, + p11: n, + p12: n, + p13: n, + p14: n, + p15: n, + p16: n, + p17: n, + p18: n, + p19: n, + p20: n, + p21: n, + p22: n, + p23: n, + p24: n, + p25: n, + p26: n, + p27: n, + p28: n, + p29: n, + p30: n, + p31: n, + p32: n, + p33: n, + p34: n, + p35: n, + p36: n, + p37: n, + p38: n, + p39: n, + p40: n, + p41: n, + p42: n, + p43: n, + p44: n, + p45: n, + p46: n, + p47: n, + p48: n, + p49: n, + p50: n, + p51: n, + p52: n, + p53: n, + p54: n, + p55: n, + p56: n, + p57: n, + p58: n, + p59: n, + p60: n, + p61: n, + p62: n, + p63: n, + p64: n, + p65: n, + p66: n, + p67: n, + p68: n, + p69: n, + p70: n, + p71: n, + p72: n, + p73: n, + p74: n, + p75: n, + p76: n, + p77: n, + p78: n, + p79: n, + p80: n, + p81: n, + p82: n, + p83: n, + p84: n, + p85: n, + p86: n, + p87: n, + p88: n, + p89: n, + p90: n, + p91: n, + p92: n, + p93: n, + p94: n, + p95: n, + p96: n, + p97: n, + p98: n, + p99: n, + p100: n, + p101: n, + p102: n, + p103: n, + p104: n, + p105: n, + p106: n, + p107: n, + p108: n, + p109: n, + p110: n, + p111: n, + p112: n, + p113: n, + p114: n, + p115: n, + p116: n, + p117: n, + p118: n, + p119: n, + p120: n, + p121: n, + p122: n, + p123: n, + p124: n, + p125: n, + p126: n, + p127: n, + p128: n, + p129: n, + p130: n, + p131: n, + p132: n, + p133: n, + p134: n, + p135: n, + p136: n, + p137: n, + p138: n, + p139: n, + p140: n, + p141: n, + p142: n, + p143: n, + p144: n, + p145: n, + p146: n, + p147: n, + p148: n, + p149: n, + p150: n, + p151: n, + p152: n, + p153: n, + p154: n, + p155: n, + p156: n, + p157: n, + p158: n, + p159: n, + p160: n, + p161: n, + p162: n, + p163: n, + p164: n, + p165: n, + p166: n, + p167: n, + p168: n, + p169: n, + p170: n, + p171: n, + p172: n, + p173: n, + p174: n, + p175: n, + p176: n, + p177: n, + p178: n, + p179: n, + p180: n, + p181: n, + p182: n, + p183: n, + p184: n, + p185: n, + p186: n, + p187: n, + p188: n, + p189: n, + p190: n, + p191: n, + p192: n, + p193: n, + p194: n, + p195: n, + p196: n, + p197: n, + p198: n, + p199: n, + p200: n, + p201: n, + p202: n, + p203: n, + p204: n, + p205: n, + p206: n, + p207: n, + p208: n, + p209: n, + p210: n, + p211: n, + p212: n, + p213: n, + p214: n, + p215: n, + p216: n, + p217: n, + p218: n, + p219: n, + p220: n, + p221: n, + p222: n, + p223: n, + p224: n, + p225: n, + p226: n, + p227: n, + p228: n, + p229: n, + p230: n, + p231: n, + p232: n, + p233: n, + p234: n, + p235: n, + p236: n, + p237: n, + p238: n, + p239: n, + p240: n, + p241: n, + p242: n, + p243: n, + p244: n, + p245: n, + p246: n, + p247: n, + p248: n, + p249: n, + p250: n, + p251: n, + p252: n, + p253: n, + p254: n, + p255: n, + p256: n, + p257: n, + p258: n, + p259: n + } +})(); + +// No output; as long as it completes without +// ArrayIndexOutOfBoundsException in the OSR continuation handler, it's +// a success.