⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 244010 in webkit


Ignore:
Timestamp:
Apr 8, 2019, 5:39:23 AM (7 years ago)
Author:
Carlos Garcia Campos
Message:

Merge r243280 - 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:
releases/WebKitGTK/webkit-2.24
Files:
1 added
8 edited

Legend:

Unmodified
Added
Removed

  • releases/WebKitGTK/webkit-2.24/JSTests/ChangeLog

    r244007 r244010  
     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-18  Mark Lam  <mark.lam@apple.com>
    212

  • releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/ChangeLog

    r244007 r244010  
     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-18  Mark Lam  <mark.lam@apple.com>
    234

  • releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/assembler/AbortReason.h

    r219172 r244010  
    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};

  • releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/dfg/DFGOperations.cpp

    r242484 r244010  
    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
    … …  
    27102710
    27112711    unsigned length = checkedLength.unsafeGet();
     2712    if (UNLIKELY(length >= MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH)) {
     2713        throwOutOfMemoryError(exec, scope);
     2714        return nullptr;
     2715    }
     2716
    27122717    JSGlobalObject* globalObject = exec->lexicalGlobalObject();
    27132718    Structure* structure = globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous);

  • releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

    r242488 r244010  
    76137613        GPRReg arrayResultGPR = arrayResult.gpr();
    76147614
     7615        // We can tell compileAllocateNewArrayWithSize() that it does not need to check
     7616        // for large arrays and use ArrayStorage structure because arrayLength here will
     7617        // always be bounded by stack size. Realistically, we won't be able to push enough
     7618        // arguments to have arrayLength exceed MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH.
    76157619        bool shouldAllowForArrayStorageStructureForLargeArrays = false;
    76167620        ASSERT(m_jit.graph().globalObjectFor(node->origin.semantic)->restParameterStructure()->indexingMode() == ArrayWithContiguous || m_jit.graph().globalObjectFor(node->origin.semantic)->isHavingABadTime());
    … …  
    79667970            }
    79677971
    7968 
     7972            speculationCheck(Overflow, JSValueRegs(), nullptr, m_jit.branch32(MacroAssembler::AboveOrEqual, lengthGPR, TrustedImm32(MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH)));
     7973
     7974            // We can tell compileAllocateNewArrayWithSize() that it does not need to
     7975            // check for large arrays and use ArrayStorage structure because we already
     7976            // ensured above that the spread array length will definitely fit in a
     7977            // non-ArrayStorage shaped array.
    79697978            bool shouldAllowForArrayStorageStructureForLargeArrays = false;
    79707979            ASSERT(m_jit.graph().globalObjectFor(node->origin.semantic)->restParameterStructure()->indexingType() == ArrayWithContiguous || m_jit.graph().globalObjectFor(node->origin.semantic)->isHavingABadTime());
    … …  
    1162511634    m_jit.lshift32(TrustedImm32(3), scratch1);
    1162611635    m_jit.add32(TrustedImm32(sizeof(IndexingHeader)), scratch1, scratch2);
     11636#if !ASSERT_DISABLED
     11637    MacroAssembler::Jump didNotOverflow = m_jit.branch32(MacroAssembler::AboveOrEqual, scratch2, sizeGPR);
     11638    m_jit.abortWithReason(UncheckedOverflow);
     11639    didNotOverflow.link(&m_jit);
     11640#endif
    1162711641    m_jit.emitAllocateVariableSized(
    1162811642        storageResultGPR, m_jit.vm()->jsValueGigacageAuxiliarySpace, scratch2, scratch1, scratch3, slowCases);
    … …  
    1292412938    if (shouldConvertLargeSizeToArrayStorage)
    1292512939        slowCases.append(m_jit.branch32(MacroAssembler::AboveOrEqual, sizeGPR, TrustedImm32(MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH)));
     12940#if !ASSERT_DISABLED
     12941    else {
     12942        MacroAssembler::Jump lengthIsWithinLimits;
     12943        lengthIsWithinLimits = m_jit.branch32(MacroAssembler::Below, sizeGPR, TrustedImm32(MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH));
     12944        m_jit.abortWithReason(UncheckedOverflow);
     12945        lengthIsWithinLimits.link(&m_jit);
     12946    }
     12947#endif
    1292612948
    1292712949    // We can use resultGPR as a scratch right now.

  • releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

    r242484 r244010  
    58255825                }
    58265826            }
     5827
     5828            LValue exceedsMaxAllowedLength = m_out.aboveOrEqual(length, m_out.constInt32(MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH));
     5829            blessSpeculation(m_out.speculate(exceedsMaxAllowedLength), Overflow, noValue(), nullptr, m_origin);
    58275830
    58285831            RegisteredStructure structure = m_graph.registerStructure(m_graph.globalObjectFor(m_node->origin.semantic)->originalArrayStructureForIndexingType(ArrayWithContiguous));

  • releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/runtime/ArrayConventions.h

    r228576 r244010  
    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.

  • releases/WebKitGTK/webkit-2.24/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp

    r241533 r244010  
    12501250
    12511251    unsigned arraySize = checkedArraySize.unsafeGet();
     1252    if (UNLIKELY(arraySize >= MIN_ARRAY_STORAGE_CONSTRUCTION_LENGTH))
     1253        THROW(createOutOfMemoryError(exec));
     1254
    12521255    JSGlobalObject* globalObject = exec->lexicalGlobalObject();
    12531256    Structure* structure = globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithContiguous);
Note: See TracChangeset for help on using the changeset viewer.