Diff
Modified: trunk/JSTests/ChangeLog (237576 => 237577)
--- trunk/JSTests/ChangeLog 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/JSTests/ChangeLog 2018-10-30 02:58:51 UTC (rev 237577)
@@ -1,3 +1,15 @@
+2018-10-29 Mark Lam <mark....@apple.com>
+
+ Correctly detect string overflow when using the 'Function' constructor.
+ https://bugs.webkit.org/show_bug.cgi?id=184883
+ <rdar://problem/36320331>
+
+ Reviewed by Saam Barati.
+
+ I've verified that this passes on 32-bit as well.
+
+ * slowMicrobenchmarks/function-constructor-with-huge-strings.js: Added.
+
2018-10-29 Tadeu Zagallo <tzaga...@apple.com>
Add support for GetStack FlushedDouble
Added: trunk/JSTests/slowMicrobenchmarks/function-constructor-with-huge-strings.js (0 => 237577)
--- trunk/JSTests/slowMicrobenchmarks/function-constructor-with-huge-strings.js (rev 0)
+++ trunk/JSTests/slowMicrobenchmarks/function-constructor-with-huge-strings.js 2018-10-30 02:58:51 UTC (rev 237577)
@@ -0,0 +1,19 @@
+var hugeString = "x";
+for (i = 0; i < 25; ++i) {
+ hugeString += hugeString;
+}
+
+var exception;
+var weird = 'Â';
+try {
+ var f = new Function(hugeString, hugeString, hugeString, hugeString, hugeString, hugeString, hugeString,
+ hugeString, hugeString, hugeString, hugeString, hugeString, hugeString, hugeString,
+ hugeString, hugeString, hugeString, hugeString, hugeString, hugeString, hugeString,
+ () => 42,
+ "return 42;");
+} catch (e) {
+ exception = e;
+}
+
+if (exception != "Error: Out of memory")
+ throw "FAIL";
Modified: trunk/Source/_javascript_Core/ChangeLog (237576 => 237577)
--- trunk/Source/_javascript_Core/ChangeLog 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-10-30 02:58:51 UTC (rev 237577)
@@ -1,3 +1,23 @@
+2018-10-29 Mark Lam <mark....@apple.com>
+
+ Correctly detect string overflow when using the 'Function' constructor.
+ https://bugs.webkit.org/show_bug.cgi?id=184883
+ <rdar://problem/36320331>
+
+ Reviewed by Saam Barati.
+
+ Added StringBuilder::hasOverflowed() checks, and throwing OutOfMemoryErrors if
+ we detect an overflow.
+
+ * runtime/FunctionConstructor.cpp:
+ (JSC::constructFunctionSkippingEvalEnabledCheck):
+ * runtime/JSGlobalObjectFunctions.cpp:
+ (JSC::encode):
+ (JSC::decode):
+ * runtime/JSONObject.cpp:
+ (JSC::Stringifier::stringify):
+ (JSC::Stringifier::appendStringifiedValue):
+
2018-10-29 Tadeu Zagallo <tzaga...@apple.com>
Unreviewed, fix JSC on arm64e after r237547
Modified: trunk/Source/_javascript_Core/runtime/FunctionConstructor.cpp (237576 => 237577)
--- trunk/Source/_javascript_Core/runtime/FunctionConstructor.cpp 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/_javascript_Core/runtime/FunctionConstructor.cpp 2018-10-30 02:58:51 UTC (rev 237577)
@@ -123,21 +123,25 @@
RETURN_IF_EXCEPTION(scope, nullptr);
program = makeString("{", prefix, functionName.string(), "() {\n", body, "\n}}");
} else {
- StringBuilder builder;
+ StringBuilder builder(StringBuilder::OverflowHandler::RecordOverflow);
builder.append('{');
builder.append(prefix);
builder.append(functionName.string());
builder.append('(');
- StringBuilder parameterBuilder;
+ StringBuilder parameterBuilder(StringBuilder::OverflowHandler::RecordOverflow);
auto viewWithString = args.at(0).toString(exec)->viewWithUnderlyingString(exec);
RETURN_IF_EXCEPTION(scope, nullptr);
parameterBuilder.append(viewWithString.view);
- for (size_t i = 1; i < args.size() - 1; i++) {
+ for (size_t i = 1; !parameterBuilder.hasOverflowed() && i < args.size() - 1; i++) {
parameterBuilder.appendLiteral(", ");
auto viewWithString = args.at(i).toString(exec)->viewWithUnderlyingString(exec);
RETURN_IF_EXCEPTION(scope, nullptr);
parameterBuilder.append(viewWithString.view);
}
+ if (parameterBuilder.hasOverflowed()) {
+ throwOutOfMemoryError(exec, scope);
+ return nullptr;
+ }
auto body = args.at(args.size() - 1).toWTFString(exec);
RETURN_IF_EXCEPTION(scope, nullptr);
@@ -164,6 +168,10 @@
RETURN_IF_EXCEPTION(scope, nullptr);
builder.append(body);
builder.appendLiteral("\n}}");
+ if (builder.hasOverflowed()) {
+ throwOutOfMemoryError(exec, scope);
+ return nullptr;
+ }
program = builder.toString();
}
Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp (237576 => 237577)
--- trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp 2018-10-30 02:58:51 UTC (rev 237577)
@@ -1,7 +1,7 @@
/*
* Copyright (C) 1999-2002 Harri Porten (por...@kde.org)
* Copyright (C) 2001 Peter Kelly (p...@post.com)
- * Copyright (C) 2003-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2003-2018 Apple Inc. All rights reserved.
* Copyright (C) 2007 Cameron Zwarich (cwzwar...@uwaterloo.ca)
* Copyright (C) 2007 Maks Orlovich
*
@@ -87,7 +87,7 @@
return JSC::throwException(exec, scope, createURIError(exec, "String contained an illegal UTF-16 sequence."_s));
};
- StringBuilder builder;
+ StringBuilder builder(StringBuilder::OverflowHandler::RecordOverflow);
builder.reserveCapacity(length);
// 4. Repeat
@@ -151,6 +151,8 @@
}
}
+ if (UNLIKELY(builder.hasOverflowed()))
+ return throwOutOfMemoryError(exec, scope);
return jsString(exec, builder.toString());
}
@@ -170,7 +172,7 @@
VM& vm = exec->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
- StringBuilder builder;
+ StringBuilder builder(StringBuilder::OverflowHandler::RecordOverflow);
int k = 0;
UChar u = 0;
while (k < length) {
@@ -229,6 +231,8 @@
k++;
builder.append(c);
}
+ if (UNLIKELY(builder.hasOverflowed()))
+ return throwOutOfMemoryError(exec, scope);
RELEASE_AND_RETURN(scope, jsString(&vm, builder.toString()));
}
Modified: trunk/Source/_javascript_Core/runtime/JSONObject.cpp (237576 => 237577)
--- trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2018-10-30 02:58:51 UTC (rev 237577)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2009-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2009-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -273,7 +273,7 @@
object->putDirect(vm, vm.propertyNames->emptyIdentifier, value);
}
- StringBuilder result;
+ StringBuilder result(StringBuilder::OverflowHandler::RecordOverflow);
Holder root(Holder::RootHolder, object);
auto stringifyResult = appendStringifiedValue(result, value, root, emptyPropertyName);
EXCEPTION_ASSERT(!scope.exception() || (stringifyResult != StringifySucceeded));
@@ -358,10 +358,12 @@
if (value.isString()) {
const String& string = asString(value)->value(m_exec);
RETURN_IF_EXCEPTION(scope, StringifyFailed);
- if (builder.appendQuotedJSONString(string))
- return StringifySucceeded;
- throwOutOfMemoryError(m_exec, scope);
- return StringifyFailed;
+ builder.appendQuotedJSONString(string);
+ if (UNLIKELY(builder.hasOverflowed())) {
+ throwOutOfMemoryError(m_exec, scope);
+ return StringifyFailed;
+ }
+ return StringifySucceeded;
}
if (value.isNumber()) {
Modified: trunk/Source/WTF/ChangeLog (237576 => 237577)
--- trunk/Source/WTF/ChangeLog 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/WTF/ChangeLog 2018-10-30 02:58:51 UTC (rev 237577)
@@ -1,3 +1,107 @@
+2018-10-29 Mark Lam <mark....@apple.com>
+
+ Correctly detect string overflow when using the 'Function' constructor.
+ https://bugs.webkit.org/show_bug.cgi?id=184883
+ <rdar://problem/36320331>
+
+ Reviewed by Saam Barati.
+
+ 1. Enhanced StringBuilder so that it supports 2 modes of behavior:
+ a. StringBuilder::OverflowHandler::CrashOnOverflow.
+ b. StringBuilder::OverflowHandler::RecordOverflow.
+
+ CrashOnOverflow will crash the moment an overflow or failure to allocate
+ memory is detected.
+
+ RecordOverflow will make StringBuilder::hasOverflowed() return true when an
+ overflow or failure to allocate memory is detected. However, if the user
+ invokes toString(), toStringPreserveCapacity(), toAtomicString(), length(),
+ capacity(), isEmpty(), characters8(), or characters16(), then the StringBuilder
+ will crash regardless because these methods can only meaningfully do their
+ work and return a result if and only if the builder has not overflowed.
+
+ By default, the StringBuilder crashes on overflow (the old behavior) unless it
+ is explicitly constructed with the StringBuilder::OverflowHandler::RecordOverflow
+ enum.
+
+ Design note:
+
+ The StringBuilder can only be put in 1 of the 2 modes at the time of
+ construction. This means that we could have implemented it as a template
+ that is parameterized on an OverflowHandler class (like CheckedArithmetic)
+ instead of using a runtime check in the ConditionalCrashOnOverflow handler.
+
+ However, I was not able to get clang to export the explicitly instantiated
+ instances (despite the methods having being decorated with WTF_EXPORT_PRIVATE).
+ Since the StringBuilder is a transient object (not stored for a long time on
+ the heap), and the runtime check of the boolean is not that costly compared
+ to other work that StringBuilder routinely does (e.g. memcpy), using the
+ new ConditionalCrashOnOverflow (which does a runtime check to determine to
+ crash or not on overflow) is fine.
+
+ When clang is able to export explicitly instantiated template methods, we can
+ templatize StringBuilder and do away with ConditionalCrashOnOverflow.
+ See https://bugs.webkit.org/show_bug.cgi?id=191050.
+
+ To support the above, we also did:
+
+ 2. Enhanced all StringBuilder append and buffer allocation methods to be able to
+ fail without crashing. This also meant that ...
+
+ 3. Added tryReallocate() support to StringImpl. tryRealloc() was added to
+ FastMalloc, and bmalloc (and related peers) to enable this.
+
+ 4. Added ConditionalCrashOnOverflow, which can choose at runtime whether to behave
+ like CrashOnOverflow or RecordOverflow.
+
+ * wtf/CheckedArithmetic.h:
+ (WTF::ConditionalCrashOnOverflow::overflowed):
+ (WTF::ConditionalCrashOnOverflow::shouldCrashOnOverflow const):
+ (WTF::ConditionalCrashOnOverflow::setShouldCrashOnOverflow):
+ (WTF::ConditionalCrashOnOverflow::hasOverflowed const):
+ (WTF::ConditionalCrashOnOverflow::clearOverflow):
+ (WTF::ConditionalCrashOnOverflow::crash):
+ (WTF::RecordOverflow::overflowed):
+ (WTF::Checked::unsafeGet const):
+ * wtf/FastMalloc.cpp:
+ (WTF::tryFastRealloc):
+ * wtf/FastMalloc.h:
+ * wtf/text/StringBuilder.cpp:
+ (WTF::expandedCapacity):
+ (WTF::StringBuilder::reifyString const):
+ (WTF::StringBuilder::resize):
+ (WTF::StringBuilder::allocateBuffer):
+ (WTF::StringBuilder::allocateBufferUpConvert):
+ (WTF::StringBuilder::reallocateBuffer<LChar>):
+ (WTF::StringBuilder::reallocateBuffer<UChar>):
+ (WTF::StringBuilder::reserveCapacity):
+ (WTF::StringBuilder::appendUninitialized):
+ (WTF::StringBuilder::appendUninitializedSlow):
+ (WTF::StringBuilder::append):
+ (WTF::StringBuilder::canShrink const):
+ (WTF::StringBuilder::shrinkToFit):
+ * wtf/text/StringBuilder.h:
+ (WTF::StringBuilder::StringBuilder):
+ (WTF::StringBuilder::didOverflow):
+ (WTF::StringBuilder::hasOverflowed const):
+ (WTF::StringBuilder::crashesOnOverflow const):
+ (WTF::StringBuilder::append):
+ (WTF::StringBuilder::toString):
+ (WTF::StringBuilder::toStringPreserveCapacity const):
+ (WTF::StringBuilder::toAtomicString const):
+ (WTF::StringBuilder::length const):
+ (WTF::StringBuilder::capacity const):
+ (WTF::StringBuilder::operator[] const):
+ (WTF::StringBuilder::swap):
+ (WTF::StringBuilder::getBufferCharacters<UChar>):
+ * wtf/text/StringBuilderJSON.cpp:
+ (WTF::StringBuilder::appendQuotedJSONString):
+ * wtf/text/StringImpl.cpp:
+ (WTF::StringImpl::reallocateInternal):
+ (WTF::StringImpl::reallocate):
+ (WTF::StringImpl::tryReallocate):
+ * wtf/text/StringImpl.h:
+
2018-10-29 Tim Horton <timothy_hor...@apple.com>
Modernize WebKit nibs and lprojs for localization's sake
Modified: trunk/Source/WTF/wtf/CheckedArithmetic.h (237576 => 237577)
--- trunk/Source/WTF/wtf/CheckedArithmetic.h 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/WTF/wtf/CheckedArithmetic.h 2018-10-30 02:58:51 UTC (rev 237577)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011 Apple Inc. All rights reserved.
+ * Copyright (C) 2011-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -70,6 +70,31 @@
DidNotOverflow
};
+class ConditionalCrashOnOverflow {
+public:
+ void overflowed()
+ {
+ m_overflowed = true;
+ if (m_shouldCrashOnOverflow)
+ crash();
+ }
+
+ bool shouldCrashOnOverflow() const { return m_shouldCrashOnOverflow; }
+ void setShouldCrashOnOverflow(bool value) { m_shouldCrashOnOverflow = value; }
+
+ bool hasOverflowed() const { return m_overflowed; }
+ void clearOverflow() { m_overflowed = false; }
+
+ static NO_RETURN_DUE_TO_CRASH void crash()
+ {
+ CRASH();
+ }
+
+private:
+ bool m_overflowed { false };
+ bool m_shouldCrashOnOverflow { true };
+};
+
class CrashOnOverflow {
public:
static NO_RETURN_DUE_TO_CRASH void overflowed()
@@ -95,11 +120,6 @@
{
}
- void overflowed()
- {
- m_overflowed = true;
- }
-
void clearOverflow()
{
m_overflowed = false;
@@ -112,6 +132,7 @@
public:
bool hasOverflowed() const { return m_overflowed; }
+ void overflowed() { m_overflowed = true; }
private:
unsigned char m_overflowed;
@@ -206,6 +227,11 @@
static const CleanType DefaultValue = 0;
};
+template <typename T> struct RemoveChecked<Checked<T, ConditionalCrashOnOverflow>> {
+ using CleanType = typename RemoveChecked<T>::CleanType;
+ static const CleanType DefaultValue = 0;
+};
+
template <typename T> struct RemoveChecked<Checked<T, CrashOnOverflow>> {
typedef typename RemoveChecked<T>::CleanType CleanType;
static const CleanType DefaultValue = 0;
@@ -631,11 +657,12 @@
}
// Value accessors. unsafeGet() will crash if there's been an overflow.
- T unsafeGet() const
+ template<typename U = T>
+ U unsafeGet() const
{
if (this->hasOverflowed())
this->crash();
- return m_value;
+ return static_cast<U>(m_value);
}
inline CheckedState safeGet(T& value) const WARN_UNUSED_RETURN
@@ -914,6 +941,7 @@
using WTF::CheckedInt64;
using WTF::CheckedUint64;
using WTF::CheckedSize;
+using WTF::ConditionalCrashOnOverflow;
using WTF::CrashOnOverflow;
using WTF::RecordOverflow;
using WTF::checkedSum;
Modified: trunk/Source/WTF/wtf/FastMalloc.cpp (237576 => 237577)
--- trunk/Source/WTF/wtf/FastMalloc.cpp 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/WTF/wtf/FastMalloc.cpp 2018-10-30 02:58:51 UTC (rev 237577)
@@ -1,6 +1,6 @@
/*
* Copyright (c) 2005, 2007, Google Inc. All rights reserved.
- * Copyright (C) 2005-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2005-2018 Apple Inc. All rights reserved.
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
@@ -221,6 +221,12 @@
return result;
}
+TryMallocReturnValue tryFastRealloc(void* p, size_t n)
+{
+ FAIL_IF_EXCEEDS_LIMIT(n);
+ return realloc(p, n);
+}
+
void releaseFastMallocFreeMemory() { }
void releaseFastMallocFreeMemoryForThisThread() { }
@@ -341,6 +347,12 @@
return tryFastZeroedMalloc(checkedSize.unsafeGet());
}
+TryMallocReturnValue tryFastRealloc(void* object, size_t newSize)
+{
+ FAIL_IF_EXCEEDS_LIMIT(newSize);
+ return bmalloc::api::tryRealloc(object, newSize);
+}
+
void releaseFastMallocFreeMemoryForThisThread()
{
bmalloc::api::scavengeThisThread();
Modified: trunk/Source/WTF/wtf/FastMalloc.h (237576 => 237577)
--- trunk/Source/WTF/wtf/FastMalloc.h 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/WTF/wtf/FastMalloc.h 2018-10-30 02:58:51 UTC (rev 237577)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2005-2009, 2015-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2005-2018 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -53,6 +53,7 @@
WTF_EXPORT_PRIVATE TryMallocReturnValue tryFastMalloc(size_t);
WTF_EXPORT_PRIVATE TryMallocReturnValue tryFastZeroedMalloc(size_t);
WTF_EXPORT_PRIVATE TryMallocReturnValue tryFastCalloc(size_t numElements, size_t elementSize);
+WTF_EXPORT_PRIVATE TryMallocReturnValue tryFastRealloc(void*, size_t);
WTF_EXPORT_PRIVATE void fastFree(void*);
Modified: trunk/Source/WTF/wtf/text/StringBuilder.cpp (237576 => 237577)
--- trunk/Source/WTF/wtf/text/StringBuilder.cpp 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/WTF/wtf/text/StringBuilder.cpp 2018-10-30 02:58:51 UTC (rev 237577)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010, 2013, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2018 Apple Inc. All rights reserved.
* Copyright (C) 2012 Google Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -34,17 +34,20 @@
namespace WTF {
+static constexpr unsigned maxCapacity = String::MaxLength + 1;
+
static unsigned expandedCapacity(unsigned capacity, unsigned requiredLength)
{
static const unsigned minimumCapacity = 16;
- return std::max(requiredLength, std::max(minimumCapacity, capacity * 2));
+ return std::max(requiredLength, std::max(minimumCapacity, std::min(capacity * 2, maxCapacity)));
}
void StringBuilder::reifyString() const
{
+ ASSERT(!hasOverflowed());
// Check if the string already exists.
if (!m_string.isNull()) {
- ASSERT(m_string.length() == m_length);
+ ASSERT(m_string.length() == m_length.unsafeGet<unsigned>());
return;
}
@@ -55,21 +58,28 @@
}
// Must be valid in the buffer, take a substring (unless string fills the buffer).
- ASSERT(m_buffer && m_length <= m_buffer->length());
- if (m_length == m_buffer->length())
+ ASSERT(m_buffer && m_length.unsafeGet<unsigned>() <= m_buffer->length());
+ if (m_length.unsafeGet<unsigned>() == m_buffer->length())
m_string = m_buffer.get();
else
- m_string = StringImpl::createSubstringSharingImpl(*m_buffer, 0, m_length);
+ m_string = StringImpl::createSubstringSharingImpl(*m_buffer, 0, m_length.unsafeGet());
}
void StringBuilder::resize(unsigned newSize)
{
+ if (hasOverflowed())
+ return;
+
// Check newSize < m_length, hence m_length > 0.
- ASSERT(newSize <= m_length);
- if (newSize == m_length)
+ unsigned oldLength = m_length.unsafeGet();
+ ASSERT(newSize <= oldLength);
+ if (newSize == oldLength)
return;
- ASSERT(m_length);
+ ASSERT(oldLength);
+ m_length = newSize;
+ ASSERT(!hasOverflowed());
+
// If there is a buffer, we only need to duplicate it if it has more than one ref.
if (m_buffer) {
m_string = String(); // Clear the string to remove the reference to m_buffer if any before checking the reference count of m_buffer.
@@ -79,16 +89,14 @@
else
allocateBuffer(m_buffer->characters16(), m_buffer->length());
}
- m_length = newSize;
- ASSERT(m_buffer->length() >= m_length);
+ ASSERT(hasOverflowed() || m_buffer->length() >= m_length.unsafeGet<unsigned>());
return;
}
// Since m_length && !m_buffer, the string must be valid in m_string, and m_string.length() > 0.
ASSERT(!m_string.isEmpty());
- ASSERT(m_length == m_string.length());
+ ASSERT(oldLength == m_string.length());
ASSERT(newSize < m_string.length());
- m_length = newSize;
m_string = StringImpl::createSubstringSharingImpl(*m_string.impl(), 0, newSize);
}
@@ -96,10 +104,13 @@
// or m_buffer, neither will be reassigned until the copy has completed).
void StringBuilder::allocateBuffer(const LChar* currentCharacters, unsigned requiredLength)
{
+ ASSERT(!hasOverflowed());
ASSERT(m_is8Bit);
// Copy the existing data into a new buffer, set result to point to the end of the existing data.
- auto buffer = StringImpl::createUninitialized(requiredLength, m_bufferCharacters8);
- memcpy(m_bufferCharacters8, currentCharacters, static_cast<size_t>(m_length) * sizeof(LChar)); // This can't overflow.
+ auto buffer = StringImpl::tryCreateUninitialized(requiredLength, m_bufferCharacters8);
+ if (UNLIKELY(!buffer))
+ return didOverflow();
+ memcpy(m_bufferCharacters8, currentCharacters, static_cast<size_t>(m_length.unsafeGet()) * sizeof(LChar)); // This can't overflow.
// Update the builder state.
m_buffer = WTFMove(buffer);
@@ -111,10 +122,13 @@
// or m_buffer, neither will be reassigned until the copy has completed).
void StringBuilder::allocateBuffer(const UChar* currentCharacters, unsigned requiredLength)
{
+ ASSERT(!hasOverflowed());
ASSERT(!m_is8Bit);
// Copy the existing data into a new buffer, set result to point to the end of the existing data.
- auto buffer = StringImpl::createUninitialized(requiredLength, m_bufferCharacters16);
- memcpy(m_bufferCharacters16, currentCharacters, static_cast<size_t>(m_length) * sizeof(UChar)); // This can't overflow.
+ auto buffer = StringImpl::tryCreateUninitialized(requiredLength, m_bufferCharacters16);
+ if (UNLIKELY(!buffer))
+ return didOverflow();
+ memcpy(m_bufferCharacters16, currentCharacters, static_cast<size_t>(m_length.unsafeGet()) * sizeof(UChar)); // This can't overflow.
// Update the builder state.
m_buffer = WTFMove(buffer);
@@ -126,11 +140,15 @@
// from either m_string or m_buffer, neither will be reassigned until the copy has completed).
void StringBuilder::allocateBufferUpConvert(const LChar* currentCharacters, unsigned requiredLength)
{
+ ASSERT(!hasOverflowed());
ASSERT(m_is8Bit);
- ASSERT(requiredLength >= m_length);
+ unsigned length = m_length.unsafeGet();
+ ASSERT(requiredLength <= maxCapacity && requiredLength >= length);
// Copy the existing data into a new buffer, set result to point to the end of the existing data.
- auto buffer = StringImpl::createUninitialized(requiredLength, m_bufferCharacters16);
- for (unsigned i = 0; i < m_length; ++i)
+ auto buffer = StringImpl::tryCreateUninitialized(requiredLength, m_bufferCharacters16);
+ if (UNLIKELY(!buffer))
+ return didOverflow(); // Treat a failure to allcoate as an overflow.
+ for (unsigned i = 0; i < length; ++i)
m_bufferCharacters16[i] = currentCharacters[i];
m_is8Bit = false;
@@ -151,11 +169,14 @@
ASSERT(m_is8Bit);
ASSERT(m_buffer->is8Bit());
- if (m_buffer->hasOneRef())
- m_buffer = StringImpl::reallocate(m_buffer.releaseNonNull(), requiredLength, m_bufferCharacters8);
- else
+ if (m_buffer->hasOneRef()) {
+ auto expectedStringImpl = StringImpl::tryReallocate(m_buffer.releaseNonNull(), requiredLength, m_bufferCharacters8);
+ if (UNLIKELY(!expectedStringImpl))
+ return didOverflow();
+ m_buffer = WTFMove(expectedStringImpl.value());
+ } else
allocateBuffer(m_buffer->characters8(), requiredLength);
- ASSERT(m_buffer->length() == requiredLength);
+ ASSERT(hasOverflowed() || m_buffer->length() == requiredLength);
}
template <>
@@ -167,15 +188,21 @@
if (m_buffer->is8Bit())
allocateBufferUpConvert(m_buffer->characters8(), requiredLength);
- else if (m_buffer->hasOneRef())
- m_buffer = StringImpl::reallocate(m_buffer.releaseNonNull(), requiredLength, m_bufferCharacters16);
- else
+ else if (m_buffer->hasOneRef()) {
+ auto expectedStringImpl = StringImpl::tryReallocate(m_buffer.releaseNonNull(), requiredLength, m_bufferCharacters16);
+ if (UNLIKELY(!expectedStringImpl))
+ return didOverflow();
+ m_buffer = WTFMove(expectedStringImpl.value());
+ } else
allocateBuffer(m_buffer->characters16(), requiredLength);
- ASSERT(m_buffer->length() == requiredLength);
+ ASSERT(hasOverflowed() || m_buffer->length() == requiredLength);
}
void StringBuilder::reserveCapacity(unsigned newCapacity)
{
+ if (hasOverflowed())
+ return;
+ ASSERT(newCapacity <= String::MaxLength);
if (m_buffer) {
// If there is already a buffer, then grow if necessary.
if (newCapacity > m_buffer->length()) {
@@ -186,8 +213,9 @@
}
} else {
// Grow the string, if necessary.
- if (newCapacity > m_length) {
- if (!m_length) {
+ unsigned length = m_length.unsafeGet();
+ if (newCapacity > length) {
+ if (!length) {
LChar* nullPlaceholder = 0;
allocateBuffer(nullPlaceholder, newCapacity);
} else if (m_string.is8Bit())
@@ -196,11 +224,12 @@
allocateBuffer(m_string.characters16(), newCapacity);
}
}
- ASSERT(!newCapacity || m_buffer->length() >= newCapacity);
+ ASSERT(hasOverflowed() || !newCapacity || m_buffer->length() >= newCapacity);
}
// Make 'length' additional capacity be available in m_buffer, update m_string & m_length,
// return a pointer to the newly allocated storage.
+// Returns nullptr if the size of the new builder would have overflowed
template <typename CharType>
ALWAYS_INLINE CharType* StringBuilder::appendUninitialized(unsigned length)
{
@@ -207,20 +236,22 @@
ASSERT(length);
// Calculate the new size of the builder after appending.
- unsigned requiredLength = length + m_length;
- if (requiredLength < length)
- CRASH();
+ CheckedInt32 requiredLength = m_length + length;
+ if (requiredLength.hasOverflowed()) {
+ didOverflow();
+ return nullptr;
+ }
- if ((m_buffer) && (requiredLength <= m_buffer->length())) {
+ if (m_buffer && (requiredLength.unsafeGet<unsigned>() <= m_buffer->length())) {
// If the buffer is valid it must be at least as long as the current builder contents!
- ASSERT(m_buffer->length() >= m_length);
- unsigned currentLength = m_length;
+ ASSERT(m_buffer->length() >= m_length.unsafeGet<unsigned>());
+ unsigned currentLength = m_length.unsafeGet();
m_string = String();
m_length = requiredLength;
return getBufferCharacters<CharType>() + currentLength;
}
- return appendUninitializedSlow<CharType>(requiredLength);
+ return appendUninitializedSlow<CharType>(requiredLength.unsafeGet());
}
// Make 'length' additional capacity be available in m_buffer, update m_string & m_length,
@@ -228,27 +259,31 @@
template <typename CharType>
CharType* StringBuilder::appendUninitializedSlow(unsigned requiredLength)
{
+ ASSERT(!hasOverflowed());
ASSERT(requiredLength);
if (m_buffer) {
// If the buffer is valid it must be at least as long as the current builder contents!
- ASSERT(m_buffer->length() >= m_length);
+ ASSERT(m_buffer->length() >= m_length.unsafeGet<unsigned>());
reallocateBuffer<CharType>(expandedCapacity(capacity(), requiredLength));
} else {
- ASSERT(m_string.length() == m_length);
+ ASSERT(m_string.length() == m_length.unsafeGet<unsigned>());
allocateBuffer(m_length ? m_string.characters<CharType>() : 0, expandedCapacity(capacity(), requiredLength));
}
-
- CharType* result = getBufferCharacters<CharType>() + m_length;
+ if (UNLIKELY(hasOverflowed()))
+ return nullptr;
+
+ CharType* result = getBufferCharacters<CharType>() + m_length.unsafeGet();
m_length = requiredLength;
- ASSERT(m_buffer->length() >= m_length);
+ ASSERT(!hasOverflowed());
+ ASSERT(m_buffer->length() >= m_length.unsafeGet<unsigned>());
return result;
}
void StringBuilder::append(const UChar* characters, unsigned length)
{
- if (!length)
+ if (!length || hasOverflowed())
return;
ASSERT(characters);
@@ -257,40 +292,50 @@
if (length == 1 && !(*characters & ~0xff)) {
// Append as 8 bit character
LChar lChar = static_cast<LChar>(*characters);
- append(&lChar, 1);
- return;
+ return append(&lChar, 1);
}
// Calculate the new size of the builder after appending.
- unsigned requiredLength = length + m_length;
- if (requiredLength < length)
- CRASH();
+ CheckedInt32 requiredLength = m_length + length;
+ if (requiredLength.hasOverflowed())
+ return didOverflow();
if (m_buffer) {
// If the buffer is valid it must be at least as long as the current builder contents!
- ASSERT(m_buffer->length() >= m_length);
-
- allocateBufferUpConvert(m_buffer->characters8(), expandedCapacity(capacity(), requiredLength));
+ ASSERT(m_buffer->length() >= m_length.unsafeGet<unsigned>());
+ allocateBufferUpConvert(m_buffer->characters8(), expandedCapacity(capacity(), requiredLength.unsafeGet()));
} else {
- ASSERT(m_string.length() == m_length);
- allocateBufferUpConvert(m_string.isNull() ? 0 : m_string.characters8(), expandedCapacity(capacity(), requiredLength));
+ ASSERT(m_string.length() == m_length.unsafeGet<unsigned>());
+ allocateBufferUpConvert(m_string.isNull() ? 0 : m_string.characters8(), expandedCapacity(capacity(), requiredLength.unsafeGet()));
}
+ if (UNLIKELY(hasOverflowed()))
+ return;
- memcpy(m_bufferCharacters16 + m_length, characters, static_cast<size_t>(length) * sizeof(UChar));
+ memcpy(m_bufferCharacters16 + m_length.unsafeGet<unsigned>(), characters, static_cast<size_t>(length) * sizeof(UChar));
m_length = requiredLength;
- } else
- memcpy(appendUninitialized<UChar>(length), characters, static_cast<size_t>(length) * sizeof(UChar));
- ASSERT(m_buffer->length() >= m_length);
+ } else {
+ UChar* dest = appendUninitialized<UChar>(length);
+ if (!dest)
+ return;
+ memcpy(dest, characters, static_cast<size_t>(length) * sizeof(UChar));
+ }
+ ASSERT(!hasOverflowed());
+ ASSERT(m_buffer->length() >= m_length.unsafeGet<unsigned>());
}
void StringBuilder::append(const LChar* characters, unsigned length)
{
- if (!length)
+ if (!length || hasOverflowed())
return;
+
ASSERT(characters);
if (m_is8Bit) {
LChar* dest = appendUninitialized<LChar>(length);
+ if (!dest) {
+ ASSERT(hasOverflowed());
+ return;
+ }
if (length > 8)
memcpy(dest, characters, static_cast<size_t>(length) * sizeof(LChar));
else {
@@ -300,6 +345,10 @@
}
} else {
UChar* dest = appendUninitialized<UChar>(length);
+ if (!dest) {
+ ASSERT(hasOverflowed());
+ return;
+ }
const LChar* end = characters + length;
while (characters < end)
*(dest++) = *(characters++);
@@ -370,8 +419,11 @@
bool StringBuilder::canShrink() const
{
+ if (hasOverflowed())
+ return false;
// Only shrink the buffer if it's less than 80% full. Need to tune this heuristic!
- return m_buffer && m_buffer->length() > (m_length + (m_length >> 2));
+ unsigned length = m_length.unsafeGet();
+ return m_buffer && m_buffer->length() > (length + (length >> 2));
}
void StringBuilder::shrinkToFit()
@@ -378,9 +430,10 @@
{
if (canShrink()) {
if (m_is8Bit)
- reallocateBuffer<LChar>(m_length);
+ reallocateBuffer<LChar>(m_length.unsafeGet());
else
- reallocateBuffer<UChar>(m_length);
+ reallocateBuffer<UChar>(m_length.unsafeGet());
+ ASSERT(!hasOverflowed());
m_string = WTFMove(m_buffer);
}
}
Modified: trunk/Source/WTF/wtf/text/StringBuilder.h (237576 => 237577)
--- trunk/Source/WTF/wtf/text/StringBuilder.h 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/WTF/wtf/text/StringBuilder.h 2018-10-30 02:58:51 UTC (rev 237577)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2009-2010, 2012-2013, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2009-2018 Apple Inc. All rights reserved.
* Copyright (C) 2012 Google Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -26,6 +26,7 @@
#pragma once
+#include <wtf/CheckedArithmetic.h>
#include <wtf/text/AtomicString.h>
#include <wtf/text/IntegerToStringConversion.h>
#include <wtf/text/StringView.h>
@@ -33,18 +34,39 @@
namespace WTF {
+// StringBuilder currently uses a Checked<int32_t, ConditionalCrashOnOverflow> for m_length.
+// Ideally, we would want to make StringBuilder a template with an OverflowHandler parameter, and
+// m_length can be instantiated based on that OverflowHandler instead. However, currently, we're
+// not able to get clang to export explicitly instantiated template methods (which would be needed
+// if we templatize StringBuilder). As a workaround, we use the ConditionalCrashOnOverflow handler
+// instead to do a runtime check on whether it should crash on overflows or not.
+//
+// When clang is able to export explicitly instantiated template methods, we can templatize
+// StringBuilder and do away with ConditionalCrashOnOverflow.
+// See https://bugs.webkit.org/show_bug.cgi?id=191050.
+
class StringBuilder {
// Disallow copying since it's expensive and we don't want code to do it by accident.
WTF_MAKE_NONCOPYABLE(StringBuilder);
public:
- StringBuilder()
+ enum class OverflowHandler {
+ CrashOnOverflow,
+ RecordOverflow
+ };
+
+ StringBuilder(OverflowHandler handler = OverflowHandler::CrashOnOverflow)
: m_bufferCharacters8(nullptr)
{
+ m_length.setShouldCrashOnOverflow(handler == OverflowHandler::CrashOnOverflow);
}
StringBuilder(StringBuilder&&) = default;
StringBuilder& operator=(StringBuilder&&) = default;
+ ALWAYS_INLINE void didOverflow() { m_length.overflowed(); }
+ ALWAYS_INLINE bool hasOverflowed() const { return m_length.hasOverflowed(); }
+ ALWAYS_INLINE bool crashesOnOverflow() const { return m_length.shouldCrashOnOverflow(); }
+
WTF_EXPORT_PRIVATE void append(const UChar*, unsigned);
WTF_EXPORT_PRIVATE void append(const LChar*, unsigned);
@@ -57,6 +79,9 @@
void append(const String& string)
{
+ if (hasOverflowed())
+ return;
+
if (!string.length())
return;
@@ -77,6 +102,11 @@
void append(const StringBuilder& other)
{
+ if (hasOverflowed())
+ return;
+ if (other.hasOverflowed())
+ return didOverflow();
+
if (!other.m_length)
return;
@@ -89,9 +119,9 @@
}
if (other.is8Bit())
- append(other.characters8(), other.m_length);
+ append(other.characters8(), other.m_length.unsafeGet());
else
- append(other.characters16(), other.m_length);
+ append(other.characters16(), other.m_length.unsafeGet());
}
void append(StringView stringView)
@@ -131,14 +161,19 @@
void append(UChar c)
{
- if (m_buffer && m_length < m_buffer->length() && m_string.isNull()) {
+ if (hasOverflowed())
+ return;
+ unsigned length = m_length.unsafeGet<unsigned>();
+ if (m_buffer && length < m_buffer->length() && m_string.isNull()) {
if (!m_is8Bit) {
- m_bufferCharacters16[m_length++] = c;
+ m_bufferCharacters16[length] = c;
+ m_length++;
return;
}
if (!(c & ~0xff)) {
- m_bufferCharacters8[m_length++] = static_cast<LChar>(c);
+ m_bufferCharacters8[length] = static_cast<LChar>(c);
+ m_length++;
return;
}
}
@@ -147,11 +182,15 @@
void append(LChar c)
{
- if (m_buffer && m_length < m_buffer->length() && m_string.isNull()) {
+ if (hasOverflowed())
+ return;
+ unsigned length = m_length.unsafeGet<unsigned>();
+ if (m_buffer && length < m_buffer->length() && m_string.isNull()) {
if (m_is8Bit)
- m_bufferCharacters8[m_length++] = c;
+ m_bufferCharacters8[length] = c;
else
- m_bufferCharacters16[m_length++] = c;
+ m_bufferCharacters16[length] = c;
+ m_length++;
} else
append(&c, 1);
}
@@ -171,7 +210,7 @@
append(U16_TRAIL(c));
}
- WTF_EXPORT_PRIVATE bool appendQuotedJSONString(const String&);
+ WTF_EXPORT_PRIVATE void appendQuotedJSONString(const String&);
template<unsigned characterCount>
ALWAYS_INLINE void appendLiteral(const char (&characters)[characterCount]) { append(characters, characterCount - 1); }
@@ -188,6 +227,7 @@
String toString()
{
+ RELEASE_ASSERT(!hasOverflowed());
shrinkToFit();
if (m_string.isNull())
reifyString();
@@ -196,6 +236,7 @@
const String& toStringPreserveCapacity() const
{
+ RELEASE_ASSERT(!hasOverflowed());
if (m_string.isNull())
reifyString();
return m_string;
@@ -203,6 +244,7 @@
AtomicString toAtomicString() const
{
+ RELEASE_ASSERT(!hasOverflowed());
if (!m_length)
return emptyAtom();
@@ -217,12 +259,13 @@
return AtomicString(m_string);
ASSERT(m_buffer);
- return AtomicString(m_buffer.get(), 0, m_length);
+ return AtomicString(m_buffer.get(), 0, m_length.unsafeGet());
}
unsigned length() const
{
- return m_length;
+ RELEASE_ASSERT(!hasOverflowed());
+ return m_length.unsafeGet();
}
bool isEmpty() const { return !m_length; }
@@ -231,7 +274,8 @@
unsigned capacity() const
{
- return m_buffer ? m_buffer->length() : m_length;
+ RELEASE_ASSERT(!hasOverflowed());
+ return m_buffer ? m_buffer->length() : m_length.unsafeGet();
}
WTF_EXPORT_PRIVATE void resize(unsigned newSize);
@@ -242,7 +286,7 @@
UChar operator[](unsigned i) const
{
- ASSERT_WITH_SECURITY_IMPLICATION(i < m_length);
+ RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!hasOverflowed() && i < m_length.unsafeGet<unsigned>());
if (m_is8Bit)
return characters8()[i];
return characters16()[i];
@@ -288,7 +332,7 @@
m_buffer.swap(stringBuilder.m_buffer);
std::swap(m_is8Bit, stringBuilder.m_is8Bit);
std::swap(m_bufferCharacters8, stringBuilder.m_bufferCharacters8);
- ASSERT(!m_buffer || m_buffer->length() >= m_length);
+ ASSERT(!m_buffer || hasOverflowed() || m_buffer->length() >= m_length.unsafeGet<unsigned>());
}
private:
@@ -311,7 +355,8 @@
LChar* m_bufferCharacters8;
UChar* m_bufferCharacters16;
};
- unsigned m_length { 0 };
+ static_assert(String::MaxLength == std::numeric_limits<int32_t>::max(), "");
+ Checked<int32_t, ConditionalCrashOnOverflow> m_length;
bool m_is8Bit { true };
};
@@ -327,7 +372,7 @@
{
ASSERT(!m_is8Bit);
return m_bufferCharacters16;
-}
+}
template <typename CharType>
bool equal(const StringBuilder& s, const CharType* buffer, unsigned length)
Modified: trunk/Source/WTF/wtf/text/StringBuilderJSON.cpp (237576 => 237577)
--- trunk/Source/WTF/wtf/text/StringBuilderJSON.cpp 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/WTF/wtf/text/StringBuilderJSON.cpp 2018-10-30 02:58:51 UTC (rev 237577)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010, 2013, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2018 Apple Inc. All rights reserved.
* Copyright (C) 2012 Google Inc. All rights reserved.
* Copyright (C) 2017 Yusuke Suzuki <utatane....@gmail.com>. All rights reserved.
* Copyright (C) 2017 Mozilla Foundation. All rights reserved.
@@ -74,8 +74,10 @@
}
}
-bool StringBuilder::appendQuotedJSONString(const String& string)
+void StringBuilder::appendQuotedJSONString(const String& string)
{
+ if (hasOverflowed())
+ return;
// Make sure we have enough buffer space to append this string without having
// to worry about reallocating in the middle.
// The 2 is for the '"' quotes on each end.
@@ -85,7 +87,7 @@
maximumCapacityRequired += 2 + stringLength * 6;
unsigned allocationSize;
if (CheckedState::DidOverflow == maximumCapacityRequired.safeGet(allocationSize))
- return false;
+ return didOverflow();
// This max() is here to allow us to allocate sizes between the range [2^31, 2^32 - 2] because roundUpToPowerOfTwo(1<<31 + some int smaller than 1<<31) == 0.
// FIXME: roundUpToPowerOfTwo should take Checked<unsigned> and abort if it fails to round up.
// https://bugs.webkit.org/show_bug.cgi?id=176086
@@ -92,24 +94,26 @@
allocationSize = std::max(allocationSize, roundUpToPowerOfTwo(allocationSize));
// Allocating this much will definitely fail.
- if (allocationSize >= 0x80000000)
- return false;
+ if (allocationSize > String::MaxLength)
+ return didOverflow();
if (is8Bit() && !string.is8Bit())
allocateBufferUpConvert(m_bufferCharacters8, allocationSize);
else
reserveCapacity(allocationSize);
+ if (UNLIKELY(hasOverflowed()))
+ return;
ASSERT(m_buffer->length() >= allocationSize);
if (is8Bit()) {
ASSERT(string.is8Bit());
- LChar* output = m_bufferCharacters8 + m_length;
+ LChar* output = m_bufferCharacters8 + m_length.unsafeGet<unsigned>();
*output++ = '"';
appendQuotedJSONStringInternal(output, string.characters8(), string.length());
*output++ = '"';
m_length = output - m_bufferCharacters8;
} else {
- UChar* output = m_bufferCharacters16 + m_length;
+ UChar* output = m_bufferCharacters16 + m_length.unsafeGet<unsigned>();
*output++ = '"';
if (string.is8Bit())
appendQuotedJSONStringInternal(output, string.characters8(), string.length());
@@ -118,8 +122,8 @@
*output++ = '"';
m_length = output - m_bufferCharacters16;
}
- ASSERT(m_buffer->length() >= m_length);
- return true;
+ ASSERT(!hasOverflowed());
+ ASSERT(m_buffer->length() >= m_length.unsafeGet<unsigned>());
}
} // namespace WTF
Modified: trunk/Source/WTF/wtf/text/StringImpl.cpp (237576 => 237577)
--- trunk/Source/WTF/wtf/text/StringImpl.cpp 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/WTF/wtf/text/StringImpl.cpp 2018-10-30 02:58:51 UTC (rev 237577)
@@ -2,7 +2,7 @@
* Copyright (C) 1999 Lars Knoll (kn...@kde.org)
* (C) 1999 Antti Koivisto (koivi...@kde.org)
* (C) 2001 Dirk Mueller ( muel...@kde.org )
- * Copyright (C) 2003-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2003-2018 Apple Inc. All rights reserved.
* Copyright (C) 2006 Andrew Wellington (pro...@wiretapped.net)
*
* This library is free software; you can redistribute it and/or
@@ -211,7 +211,7 @@
return createUninitializedInternal(length, data);
}
-template<typename CharacterType> inline Ref<StringImpl> StringImpl::reallocateInternal(Ref<StringImpl>&& originalString, unsigned length, CharacterType*& data)
+template<typename CharacterType> inline Expected<Ref<StringImpl>, UTF8ConversionError> StringImpl::reallocateInternal(Ref<StringImpl>&& originalString, unsigned length, CharacterType*& data)
{
ASSERT(originalString->hasOneRef());
ASSERT(originalString->bufferOwnership() == BufferInternal);
@@ -218,15 +218,17 @@
if (!length) {
data = ""
- return *empty();
+ return Ref<StringImpl>(*empty());
}
// Same as createUninitialized() except here we use fastRealloc.
if (length > ((std::numeric_limits<unsigned>::max() - sizeof(StringImpl)) / sizeof(CharacterType)))
- CRASH();
+ return makeUnexpected(UTF8ConversionError::OutOfMemory);
originalString->~StringImpl();
- auto* string = static_cast<StringImpl*>(fastRealloc(&originalString.leakRef(), allocationSize<CharacterType>(length)));
+ StringImpl* string;
+ if (!tryFastRealloc(&originalString.leakRef(), allocationSize<CharacterType>(length)).getValue(string))
+ return makeUnexpected(UTF8ConversionError::OutOfMemory);
data = ""
return constructInternal<CharacterType>(*string, length);
@@ -234,11 +236,25 @@
Ref<StringImpl> StringImpl::reallocate(Ref<StringImpl>&& originalString, unsigned length, LChar*& data)
{
+ auto expectedStringImpl = tryReallocate(WTFMove(originalString), length, data);
+ RELEASE_ASSERT(expectedStringImpl);
+ return WTFMove(expectedStringImpl.value());
+}
+
+Ref<StringImpl> StringImpl::reallocate(Ref<StringImpl>&& originalString, unsigned length, UChar*& data)
+{
+ auto expectedStringImpl = tryReallocate(WTFMove(originalString), length, data);
+ RELEASE_ASSERT(expectedStringImpl);
+ return WTFMove(expectedStringImpl.value());
+}
+
+Expected<Ref<StringImpl>, UTF8ConversionError> StringImpl::tryReallocate(Ref<StringImpl>&& originalString, unsigned length, LChar*& data)
+{
ASSERT(originalString->is8Bit());
return reallocateInternal(WTFMove(originalString), length, data);
}
-Ref<StringImpl> StringImpl::reallocate(Ref<StringImpl>&& originalString, unsigned length, UChar*& data)
+Expected<Ref<StringImpl>, UTF8ConversionError> StringImpl::tryReallocate(Ref<StringImpl>&& originalString, unsigned length, UChar*& data)
{
ASSERT(!originalString->is8Bit());
return reallocateInternal(WTFMove(originalString), length, data);
Modified: trunk/Source/WTF/wtf/text/StringImpl.h (237576 => 237577)
--- trunk/Source/WTF/wtf/text/StringImpl.h 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/WTF/wtf/text/StringImpl.h 2018-10-30 02:58:51 UTC (rev 237577)
@@ -254,6 +254,8 @@
// the originalString can't be used after this function.
static Ref<StringImpl> reallocate(Ref<StringImpl>&& originalString, unsigned length, LChar*& data);
static Ref<StringImpl> reallocate(Ref<StringImpl>&& originalString, unsigned length, UChar*& data);
+ static Expected<Ref<StringImpl>, UTF8ConversionError> tryReallocate(Ref<StringImpl>&& originalString, unsigned length, LChar*& data);
+ static Expected<Ref<StringImpl>, UTF8ConversionError> tryReallocate(Ref<StringImpl>&& originalString, unsigned length, UChar*& data);
static unsigned flagsOffset() { return OBJECT_OFFSETOF(StringImpl, m_hashAndFlags); }
static unsigned flagIs8Bit() { return s_hashFlag8BitBuffer; }
@@ -510,7 +512,7 @@
template<typename CharacterType> static Ref<StringImpl> constructInternal(StringImpl&, unsigned);
template<typename CharacterType> static Ref<StringImpl> createUninitializedInternal(unsigned, CharacterType*&);
template<typename CharacterType> static Ref<StringImpl> createUninitializedInternalNonEmpty(unsigned, CharacterType*&);
- template<typename CharacterType> static Ref<StringImpl> reallocateInternal(Ref<StringImpl>&&, unsigned, CharacterType*&);
+ template<typename CharacterType> static Expected<Ref<StringImpl>, UTF8ConversionError> reallocateInternal(Ref<StringImpl>&&, unsigned, CharacterType*&);
template<typename CharacterType> static Ref<StringImpl> createInternal(const CharacterType*, unsigned);
WTF_EXPORT_PRIVATE NEVER_INLINE unsigned hashSlowCase() const;
Modified: trunk/Source/bmalloc/ChangeLog (237576 => 237577)
--- trunk/Source/bmalloc/ChangeLog 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/bmalloc/ChangeLog 2018-10-30 02:58:51 UTC (rev 237577)
@@ -1,3 +1,24 @@
+2018-10-29 Mark Lam <mark....@apple.com>
+
+ Correctly detect string overflow when using the 'Function' constructor.
+ https://bugs.webkit.org/show_bug.cgi?id=184883
+ <rdar://problem/36320331>
+
+ Reviewed by Saam Barati.
+
+ * bmalloc/Allocator.cpp:
+ (bmalloc::Allocator::reallocate):
+ (bmalloc::Allocator::tryReallocate):
+ (bmalloc::Allocator::reallocateImpl):
+ * bmalloc/Allocator.h:
+ * bmalloc/Cache.h:
+ (bmalloc::Cache::tryReallocate):
+ * bmalloc/DebugHeap.cpp:
+ (bmalloc::DebugHeap::realloc):
+ * bmalloc/DebugHeap.h:
+ * bmalloc/bmalloc.h:
+ (bmalloc::api::tryRealloc):
+
2018-10-25 Ross Kirsling <ross.kirsl...@sony.com>
Cleanup: inline constexpr is redundant as constexpr implies inline
Modified: trunk/Source/bmalloc/bmalloc/Allocator.cpp (237576 => 237577)
--- trunk/Source/bmalloc/bmalloc/Allocator.cpp 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/bmalloc/bmalloc/Allocator.cpp 2018-10-30 02:58:51 UTC (rev 237577)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2014-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -95,8 +95,20 @@
void* Allocator::reallocate(void* object, size_t newSize)
{
+ bool crashOnFailure = true;
+ return reallocateImpl(object, newSize, crashOnFailure);
+}
+
+void* Allocator::tryReallocate(void* object, size_t newSize)
+{
+ bool crashOnFailure = false;
+ return reallocateImpl(object, newSize, crashOnFailure);
+}
+
+void* Allocator::reallocateImpl(void* object, size_t newSize, bool crashOnFailure)
+{
if (m_debugHeap)
- return m_debugHeap->realloc(object, newSize);
+ return m_debugHeap->realloc(object, newSize, crashOnFailure);
size_t oldSize = 0;
switch (objectType(m_heap.kind(), object)) {
@@ -121,7 +133,14 @@
}
}
- void* result = allocate(newSize);
+ void* result = nullptr;
+ if (crashOnFailure)
+ result = allocate(newSize);
+ else {
+ result = tryAllocate(newSize);
+ if (!result)
+ return nullptr;
+ }
size_t copySize = std::min(oldSize, newSize);
memcpy(result, object, copySize);
m_deallocator.deallocate(object);
Modified: trunk/Source/bmalloc/bmalloc/Allocator.h (237576 => 237577)
--- trunk/Source/bmalloc/bmalloc/Allocator.h 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/bmalloc/bmalloc/Allocator.h 2018-10-30 02:58:51 UTC (rev 237577)
@@ -47,6 +47,7 @@
void* allocate(size_t);
void* tryAllocate(size_t alignment, size_t);
void* allocate(size_t alignment, size_t);
+ void* tryReallocate(void*, size_t);
void* reallocate(void*, size_t);
void scavenge();
@@ -53,7 +54,8 @@
private:
void* allocateImpl(size_t alignment, size_t, bool crashOnFailure);
-
+ void* reallocateImpl(void*, size_t, bool crashOnFailure);
+
bool allocateFastCase(size_t, void*&);
BEXPORT void* allocateSlowCase(size_t);
Modified: trunk/Source/bmalloc/bmalloc/Cache.h (237576 => 237577)
--- trunk/Source/bmalloc/bmalloc/Cache.h 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/bmalloc/bmalloc/Cache.h 2018-10-30 02:58:51 UTC (rev 237577)
@@ -43,6 +43,7 @@
static void* tryAllocate(HeapKind, size_t alignment, size_t);
static void* allocate(HeapKind, size_t alignment, size_t);
static void deallocate(HeapKind, void*);
+ static void* tryReallocate(HeapKind, void*, size_t);
static void* reallocate(HeapKind, void*, size_t);
static void scavenge(HeapKind);
@@ -103,6 +104,14 @@
return caches->at(mapToActiveHeapKindAfterEnsuringGigacage(heapKind)).deallocator().deallocate(object);
}
+inline void* Cache::tryReallocate(HeapKind heapKind, void* object, size_t newSize)
+{
+ PerHeapKind<Cache>* caches = PerThread<PerHeapKind<Cache>>::getFastCase();
+ if (!caches)
+ return reallocateSlowCaseNullCache(heapKind, object, newSize);
+ return caches->at(mapToActiveHeapKindAfterEnsuringGigacage(heapKind)).allocator().tryReallocate(object, newSize);
+}
+
inline void* Cache::reallocate(HeapKind heapKind, void* object, size_t newSize)
{
PerHeapKind<Cache>* caches = PerThread<PerHeapKind<Cache>>::getFastCase();
Modified: trunk/Source/bmalloc/bmalloc/DebugHeap.cpp (237576 => 237577)
--- trunk/Source/bmalloc/bmalloc/DebugHeap.cpp 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/bmalloc/bmalloc/DebugHeap.cpp 2018-10-30 02:58:51 UTC (rev 237577)
@@ -59,10 +59,10 @@
return result;
}
-void* DebugHeap::realloc(void* object, size_t size)
+void* DebugHeap::realloc(void* object, size_t size, bool crashOnFailure)
{
void* result = malloc_zone_realloc(m_zone, object, size);
- if (!result)
+ if (!result && crashOnFailure)
BCRASH();
return result;
}
@@ -98,10 +98,10 @@
return result;
}
-void* DebugHeap::realloc(void* object, size_t size)
+void* DebugHeap::realloc(void* object, size_t size, bool crashOnFailure)
{
void* result = ::realloc(object, size);
- if (!result)
+ if (!result && crashOnFailure)
BCRASH();
return result;
}
Modified: trunk/Source/bmalloc/bmalloc/DebugHeap.h (237576 => 237577)
--- trunk/Source/bmalloc/bmalloc/DebugHeap.h 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/bmalloc/bmalloc/DebugHeap.h 2018-10-30 02:58:51 UTC (rev 237577)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -41,7 +41,7 @@
void* malloc(size_t);
void* memalign(size_t alignment, size_t, bool crashOnFailure);
- void* realloc(void*, size_t);
+ void* realloc(void*, size_t, bool crashOnFailure);
void free(void*);
void* memalignLarge(size_t alignment, size_t);
Modified: trunk/Source/bmalloc/bmalloc/bmalloc.h (237576 => 237577)
--- trunk/Source/bmalloc/bmalloc/bmalloc.h 2018-10-30 02:51:23 UTC (rev 237576)
+++ trunk/Source/bmalloc/bmalloc/bmalloc.h 2018-10-30 02:58:51 UTC (rev 237577)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2014-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -63,6 +63,12 @@
return Cache::allocate(kind, alignment, size);
}
+// Returns null on failure.
+inline void* tryRealloc(void* object, size_t newSize, HeapKind kind = HeapKind::Primary)
+{
+ return Cache::tryReallocate(kind, object, newSize);
+}
+
// Crashes on failure.
inline void* realloc(void* object, size_t newSize, HeapKind kind = HeapKind::Primary)
{
