Changeset 243280 in webkit


Ignore:
Timestamp:
Mar 21, 2019 12:51:12 AM (5 years ago)
Author:
mark.lam@apple.com
Message:

Cap length of an array with spread to MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH.
https://bugs.webkit.org/show_bug.cgi?id=196055
<rdar://problem/49067448>

Reviewed by Yusuke Suzuki.

JSTests:

  • stress/new_array_with_spread-should-cap-array-size-to-MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH.js: Added.

Source/JavaScriptCore:

We are doing this because:

  1. We expect the array to be densely packed.
  2. SpeculativeJIT::compileAllocateNewArrayWithSize() (and the FTL equivalent) expects the array length to be less than MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH if we don't want to use an ArrayStorage shape.
  3. There's no reason why an array with spread needs to be that large anyway. MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH is plenty.

In this patch, we also add a debug assert in compileAllocateNewArrayWithSize() and
emitAllocateButterfly() to check for overflows.

  • assembler/AbortReason.h:
  • dfg/DFGOperations.cpp:
  • dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileCreateRest):
(JSC::DFG::SpeculativeJIT::compileNewArrayWithSpread):
(JSC::DFG::SpeculativeJIT::emitAllocateButterfly):
(JSC::DFG::SpeculativeJIT::compileAllocateNewArrayWithSize):

  • ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileNewArrayWithSpread):

  • runtime/ArrayConventions.h:
  • runtime/CommonSlowPaths.cpp:

(JSC::SLOW_PATH_DECL):

Location:
trunk
Files:
1 added
8 edited

Legend:

Unmodified
Added
Removed
  • trunk/JSTests/ChangeLog

    r243277 r243280  
     12019-03-21  Mark Lam  <mark.lam@apple.com>
     2
     3        Cap length of an array with spread to MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH.
     4        https://bugs.webkit.org/show_bug.cgi?id=196055
     5        <rdar://problem/49067448>
     6
     7        Reviewed by Yusuke Suzuki.
     8
     9        * stress/new_array_with_spread-should-cap-array-size-to-MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH.js: Added.
     10
    1112019-03-20  Saam Barati  <sbarati@apple.com>
    212
  • trunk/Source/JavaScriptCore/ChangeLog

    r243279 r243280  
     12019-03-21  Mark Lam  <mark.lam@apple.com>
     2
     3        Cap length of an array with spread to MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH.
     4        https://bugs.webkit.org/show_bug.cgi?id=196055
     5        <rdar://problem/49067448>
     6
     7        Reviewed by Yusuke Suzuki.
     8
     9        We are doing this because:
     10        1. We expect the array to be densely packed.
     11        2. SpeculativeJIT::compileAllocateNewArrayWithSize() (and the FTL equivalent)
     12           expects the array length to be less than MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH
     13           if we don't want to use an ArrayStorage shape.
     14        3. There's no reason why an array with spread needs to be that large anyway.
     15           MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH is plenty.
     16
     17        In this patch, we also add a debug assert in compileAllocateNewArrayWithSize() and
     18        emitAllocateButterfly() to check for overflows.
     19
     20        * assembler/AbortReason.h:
     21        * dfg/DFGOperations.cpp:
     22        * dfg/DFGSpeculativeJIT.cpp:
     23        (JSC::DFG::SpeculativeJIT::compileCreateRest):
     24        (JSC::DFG::SpeculativeJIT::compileNewArrayWithSpread):
     25        (JSC::DFG::SpeculativeJIT::emitAllocateButterfly):
     26        (JSC::DFG::SpeculativeJIT::compileAllocateNewArrayWithSize):
     27        * ftl/FTLLowerDFGToB3.cpp:
     28        (JSC::FTL::DFG::LowerDFGToB3::compileNewArrayWithSpread):
     29        * runtime/ArrayConventions.h:
     30        * runtime/CommonSlowPaths.cpp:
     31        (JSC::SLOW_PATH_DECL):
     32
    1332019-03-20  Yusuke Suzuki  <ysuzuki@apple.com>
    234
  • trunk/Source/JavaScriptCore/assembler/AbortReason.h

    r219172 r243280  
    11/*
    2  * Copyright (C) 2014-2016 Apple Inc. All rights reserved.
     2 * Copyright (C) 2014-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    7474    TGInvalidPointer                                  = 320,
    7575    TGNotSupported                                    = 330,
     76    UncheckedOverflow                                 = 335,
    7677    YARRNoInputConsumed                               = 340,
    7778};
  • trunk/Source/JavaScriptCore/dfg/DFGOperations.cpp

    r243232 r243280  
    11/*
    2  * Copyright (C) 2011-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2011-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    27222722
    27232723    unsigned length = checkedLength.unsafeGet();
     2724    if (UNLIKELY(length >= MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH)) {
     2725        throwOutOfMemoryError(exec, scope);
     2726        return nullptr;
     2727    }
     2728
    27242729    JSGlobalObject* globalObject = exec->lexicalGlobalObject();
    27252730    Structure* structure = globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous);
  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

    r243232 r243280  
    76557655        GPRReg arrayResultGPR = arrayResult.gpr();
    76567656
     7657        // We can tell compileAllocateNewArrayWithSize() that it does not need to check
     7658        // for large arrays and use ArrayStorage structure because arrayLength here will
     7659        // always be bounded by stack size. Realistically, we won't be able to push enough
     7660        // arguments to have arrayLength exceed MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH.
    76577661        bool shouldAllowForArrayStorageStructureForLargeArrays = false;
    76587662        ASSERT(m_jit.graph().globalObjectFor(node->origin.semantic)->restParameterStructure()->indexingMode() == ArrayWithContiguous || m_jit.graph().globalObjectFor(node->origin.semantic)->isHavingABadTime());
     
    80088012            }
    80098013
    8010 
     8014            speculationCheck(Overflow, JSValueRegs(), nullptr, m_jit.branch32(MacroAssembler::AboveOrEqual, lengthGPR, TrustedImm32(MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH)));
     8015
     8016            // We can tell compileAllocateNewArrayWithSize() that it does not need to
     8017            // check for large arrays and use ArrayStorage structure because we already
     8018            // ensured above that the spread array length will definitely fit in a
     8019            // non-ArrayStorage shaped array.
    80118020            bool shouldAllowForArrayStorageStructureForLargeArrays = false;
    80128021            ASSERT(m_jit.graph().globalObjectFor(node->origin.semantic)->restParameterStructure()->indexingType() == ArrayWithContiguous || m_jit.graph().globalObjectFor(node->origin.semantic)->isHavingABadTime());
     
    1167511684    m_jit.lshift32(TrustedImm32(3), scratch1);
    1167611685    m_jit.add32(TrustedImm32(sizeof(IndexingHeader)), scratch1, scratch2);
     11686#if !ASSERT_DISABLED
     11687    MacroAssembler::Jump didNotOverflow = m_jit.branch32(MacroAssembler::AboveOrEqual, scratch2, sizeGPR);
     11688    m_jit.abortWithReason(UncheckedOverflow);
     11689    didNotOverflow.link(&m_jit);
     11690#endif
    1167711691    m_jit.emitAllocateVariableSized(
    1167811692        storageResultGPR, m_jit.vm()->jsValueGigacageAuxiliarySpace, scratch2, scratch1, scratch3, slowCases);
     
    1297412988    if (shouldConvertLargeSizeToArrayStorage)
    1297512989        slowCases.append(m_jit.branch32(MacroAssembler::AboveOrEqual, sizeGPR, TrustedImm32(MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH)));
     12990#if !ASSERT_DISABLED
     12991    else {
     12992        MacroAssembler::Jump lengthIsWithinLimits;
     12993        lengthIsWithinLimits = m_jit.branch32(MacroAssembler::Below, sizeGPR, TrustedImm32(MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH));
     12994        m_jit.abortWithReason(UncheckedOverflow);
     12995        lengthIsWithinLimits.link(&m_jit);
     12996    }
     12997#endif
    1297612998
    1297712999    // We can use resultGPR as a scratch right now.
  • trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

    r243232 r243280  
    58405840                }
    58415841            }
     5842
     5843            LValue exceedsMaxAllowedLength = m_out.aboveOrEqual(length, m_out.constInt32(MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH));
     5844            blessSpeculation(m_out.speculate(exceedsMaxAllowedLength), Overflow, noValue(), nullptr, m_origin);
    58425845
    58435846            RegisteredStructure structure = m_graph.registerStructure(m_graph.globalObjectFor(m_node->origin.semantic)->originalArrayStructureForIndexingType(ArrayWithContiguous));
  • trunk/Source/JavaScriptCore/runtime/ArrayConventions.h

    r228576 r243280  
    11/*
    22 *  Copyright (C) 1999-2000 Harri Porten (porten@kde.org)
    3  *  Copyright (C) 2003-2017 Apple Inc. All rights reserved.
     3 *  Copyright (C) 2003-2019 Apple Inc. All rights reserved.
    44 *
    55 *  This library is free software; you can redistribute it and/or
     
    6666// If you try to allocate a contiguous array larger than this, then we will allocate an ArrayStorage
    6767// array instead. We allow for an array that occupies 1GB of VM.
    68 #define MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH 1024 * 1024 * 1024 / 8
     68#define MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH (1024 * 1024 * 1024 / 8)
    6969#define MAX_STORAGE_VECTOR_INDEX (MAX_STORAGE_VECTOR_LENGTH - 1)
    7070// 0xFFFFFFFF is a bit weird -- is not an array index even though it's an integer.
  • trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp

    r242715 r243280  
    12591259
    12601260    unsigned arraySize = checkedArraySize.unsafeGet();
     1261    if (UNLIKELY(arraySize >= MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH))
     1262        THROW(createOutOfMemoryError(exec));
     1263
    12611264    JSGlobalObject* globalObject = exec->lexicalGlobalObject();
    12621265    Structure* structure = globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous);
Note: See TracChangeset for help on using the changeset viewer.