Changeset 207849 in webkit
- Timestamp:
- Oct 25, 2016 3:19:37 PM (8 years ago)
- Location:
- trunk
- Files:
-
- 1 added
- 6 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/JSTests/ChangeLog
r207846 r207849 1 2016-10-25 Mark Lam <mark.lam@apple.com> 2 3 JSStringJoiner::joinedLength() should limit joined string lengths to INT_MAX. 4 https://bugs.webkit.org/show_bug.cgi?id=163937 5 <rdar://problem/28642990> 6 7 Reviewed by Geoffrey Garen. 8 9 * stress/joined-strings-should-not-exceed-max-string-length.js: Added. 10 1 11 2016-10-25 JF Bastien <jfbastien@apple.com> 2 12 -
trunk/Source/JavaScriptCore/ChangeLog
r207843 r207849 1 2016-10-25 Mark Lam <mark.lam@apple.com> 2 3 JSStringJoiner::joinedLength() should limit joined string lengths to INT_MAX. 4 https://bugs.webkit.org/show_bug.cgi?id=163937 5 <rdar://problem/28642990> 6 7 Reviewed by Geoffrey Garen. 8 9 JSStringJoiner::joinedLength() was previously limiting it to UINT_MAX. This is 10 inconsistent with other parts of string code which expects a max length of INT_MAX. 11 This is now fixed. 12 13 Also fixed jsMakeNontrivialString() to ensure that the resultant string length 14 is valid. It was previously allowing lengths greater than INT_MAX. This was 15 caught by the new assertion in JSString::setLength(). 16 17 There are already pre-existing assertions in various JSString::finishCreation() 18 which do RELEASE_ASSERTs on the string length. To be consistent, I'm making the 19 assertion in JSString::setLength() a RELEASE_ASSERT. If this proves to be a 20 performance issue, I'll change this to a debug ASSERT later. 21 22 * runtime/JSString.cpp: 23 (JSC::JSRopeString::resolveRopeInternal8): 24 (JSC::JSRopeString::resolveRopeInternal8NoSubstring): 25 (JSC::JSRopeString::resolveRopeInternal16): 26 (JSC::JSRopeString::resolveRopeInternal16NoSubstring): 27 (JSC::JSRopeString::resolveRopeToAtomicString): 28 (JSC::JSRopeString::resolveRopeToExistingAtomicString): 29 (JSC::JSRopeString::resolveRope): 30 (JSC::JSRopeString::resolveRopeSlowCase8): 31 (JSC::JSRopeString::resolveRopeSlowCase): 32 (JSC::JSString::getStringPropertyDescriptor): 33 * runtime/JSString.h: 34 (JSC::JSString::finishCreation): 35 (JSC::JSString::length): 36 (JSC::JSString::isValidLength): 37 (JSC::JSString::toBoolean): 38 (JSC::JSString::canGetIndex): 39 (JSC::JSString::setLength): 40 (JSC::JSString::getStringPropertySlot): 41 (JSC::JSRopeString::unsafeView): 42 (JSC::JSRopeString::viewWithUnderlyingString): 43 * runtime/JSStringBuilder.h: 44 (JSC::jsMakeNontrivialString): 45 * runtime/JSStringJoiner.cpp: 46 (JSC::JSStringJoiner::joinedLength): 47 1 48 2016-10-25 JF Bastien <jfbastien@apple.com> 2 49 -
trunk/Source/JavaScriptCore/runtime/JSString.cpp
r206386 r207849 121 121 if (isSubstring()) { 122 122 StringImpl::copyChars( 123 buffer, substringBase()->m_value.characters8() + substringOffset(), m_length);123 buffer, substringBase()->m_value.characters8() + substringOffset(), length()); 124 124 return; 125 125 } … … 144 144 position += length; 145 145 } 146 ASSERT((buffer + m_length) == position);146 ASSERT((buffer + length()) == position); 147 147 } 148 148 … … 151 151 if (isSubstring()) { 152 152 StringImpl::copyChars( 153 buffer, substringBase()->m_value.characters16() + substringOffset(), m_length);153 buffer, substringBase()->m_value.characters16() + substringOffset(), length()); 154 154 return; 155 155 } … … 177 177 position += length; 178 178 } 179 ASSERT((buffer + m_length) == position);179 ASSERT((buffer + length()) == position); 180 180 } 181 181 182 182 void JSRopeString::resolveRopeToAtomicString(ExecState* exec) const 183 183 { 184 if ( m_length> maxLengthForOnStackResolve) {184 if (length() > maxLengthForOnStackResolve) { 185 185 resolveRope(exec); 186 186 m_value = AtomicString(m_value); … … 192 192 LChar buffer[maxLengthForOnStackResolve]; 193 193 resolveRopeInternal8(buffer); 194 m_value = AtomicString(buffer, m_length);194 m_value = AtomicString(buffer, length()); 195 195 setIs8Bit(m_value.impl()->is8Bit()); 196 196 } else { 197 197 UChar buffer[maxLengthForOnStackResolve]; 198 198 resolveRopeInternal16(buffer); 199 m_value = AtomicString(buffer, m_length);199 m_value = AtomicString(buffer, length()); 200 200 setIs8Bit(m_value.impl()->is8Bit()); 201 201 } … … 216 216 RefPtr<AtomicStringImpl> JSRopeString::resolveRopeToExistingAtomicString(ExecState* exec) const 217 217 { 218 if ( m_length> maxLengthForOnStackResolve) {218 if (length() > maxLengthForOnStackResolve) { 219 219 resolveRope(exec); 220 220 if (RefPtr<AtomicStringImpl> existingAtomicString = AtomicStringImpl::lookUp(m_value.impl())) { … … 230 230 LChar buffer[maxLengthForOnStackResolve]; 231 231 resolveRopeInternal8(buffer); 232 if (RefPtr<AtomicStringImpl> existingAtomicString = AtomicStringImpl::lookUp(buffer, m_length)) {232 if (RefPtr<AtomicStringImpl> existingAtomicString = AtomicStringImpl::lookUp(buffer, length())) { 233 233 m_value = *existingAtomicString; 234 234 setIs8Bit(m_value.impl()->is8Bit()); … … 239 239 UChar buffer[maxLengthForOnStackResolve]; 240 240 resolveRopeInternal16(buffer); 241 if (RefPtr<AtomicStringImpl> existingAtomicString = AtomicStringImpl::lookUp(buffer, m_length)) {241 if (RefPtr<AtomicStringImpl> existingAtomicString = AtomicStringImpl::lookUp(buffer, length())) { 242 242 m_value = *existingAtomicString; 243 243 setIs8Bit(m_value.impl()->is8Bit()); … … 256 256 if (isSubstring()) { 257 257 ASSERT(!substringBase()->isRope()); 258 m_value = substringBase()->m_value.substringSharingImpl(substringOffset(), m_length);258 m_value = substringBase()->m_value.substringSharingImpl(substringOffset(), length()); 259 259 substringBase().clear(); 260 260 return; … … 263 263 if (is8Bit()) { 264 264 LChar* buffer; 265 if (auto newImpl = StringImpl::tryCreateUninitialized( m_length, buffer)) {265 if (auto newImpl = StringImpl::tryCreateUninitialized(length(), buffer)) { 266 266 Heap::heap(this)->reportExtraMemoryAllocated(newImpl->cost()); 267 267 m_value = WTFMove(newImpl); … … 277 277 278 278 UChar* buffer; 279 if (auto newImpl = StringImpl::tryCreateUninitialized( m_length, buffer)) {279 if (auto newImpl = StringImpl::tryCreateUninitialized(length(), buffer)) { 280 280 Heap::heap(this)->reportExtraMemoryAllocated(newImpl->cost()); 281 281 m_value = WTFMove(newImpl); … … 302 302 void JSRopeString::resolveRopeSlowCase8(LChar* buffer) const 303 303 { 304 LChar* position = buffer + m_length; // We will be working backwards over the rope.304 LChar* position = buffer + length(); // We will be working backwards over the rope. 305 305 Vector<JSString*, 32, UnsafeVectorOverflow> workQueue; // Putting strings into a Vector is only OK because there are no GC points in this method. 306 306 … … 338 338 void JSRopeString::resolveRopeSlowCase(UChar* buffer) const 339 339 { 340 UChar* position = buffer + m_length; // We will be working backwards over the rope.340 UChar* position = buffer + length(); // We will be working backwards over the rope. 341 341 Vector<JSString*, 32, UnsafeVectorOverflow> workQueue; // These strings are kept alive by the parent rope, so using a Vector is OK. 342 342 … … 431 431 { 432 432 if (propertyName == exec->propertyNames().length) { 433 descriptor.setDescriptor(jsNumber( m_length), DontEnum | DontDelete | ReadOnly);433 descriptor.setDescriptor(jsNumber(length()), DontEnum | DontDelete | ReadOnly); 434 434 return true; 435 435 } 436 436 437 437 Optional<uint32_t> index = parseIndex(propertyName); 438 if (index && index.value() < m_length) {438 if (index && index.value() < length()) { 439 439 descriptor.setDescriptor(getIndex(exec, index.value()), DontDelete | ReadOnly); 440 440 return true; -
trunk/Source/JavaScriptCore/runtime/JSString.h
r206555 r207849 107 107 ASSERT(!m_value.isNull()); 108 108 Base::finishCreation(vm); 109 m_length = length;109 setLength(length); 110 110 setIs8Bit(m_value.impl()->is8Bit()); 111 111 } … … 115 115 ASSERT(!m_value.isNull()); 116 116 Base::finishCreation(vm); 117 m_length = length;117 setLength(length); 118 118 setIs8Bit(m_value.impl()->is8Bit()); 119 119 Heap::heap(this)->reportExtraMemoryAllocated(cost); … … 124 124 { 125 125 Base::finishCreation(vm); 126 m_length = 0;126 setLength(0); 127 127 setIs8Bit(true); 128 128 } … … 159 159 const String& tryGetValue() const; 160 160 const StringImpl* tryGetValueImpl() const; 161 unsigned length() const { return m_length; } 161 ALWAYS_INLINE unsigned length() const { return m_length; } 162 ALWAYS_INLINE static bool isValidLength(size_t length) 163 { 164 // While length is of type unsigned, the runtime and compilers are all 165 // expecting that m_length is a positive value <= INT_MAX. 166 // FIXME: Look into making the max length UINT_MAX to match StringImpl's max length. 167 // https://bugs.webkit.org/show_bug.cgi?id=163955 168 return length <= std::numeric_limits<int32_t>::max(); 169 } 162 170 163 171 JSValue toPrimitive(ExecState*, PreferredPrimitiveType) const; 164 bool toBoolean() const { return !! m_length; }172 bool toBoolean() const { return !!length(); } 165 173 bool getPrimitiveNumber(ExecState*, double& number, JSValue&) const; 166 174 JSObject* toObject(ExecState*, JSGlobalObject*) const; … … 171 179 bool getStringPropertyDescriptor(ExecState*, PropertyName, PropertyDescriptor&); 172 180 173 bool canGetIndex(unsigned i) { return i < m_length; }181 bool canGetIndex(unsigned i) { return i < length(); } 174 182 JSString* getIndex(ExecState*, unsigned); 175 183 … … 205 213 } 206 214 215 ALWAYS_INLINE void setLength(unsigned length) 216 { 217 RELEASE_ASSERT(isValidLength(length)); 218 m_length = length; 219 } 220 221 private: 207 222 mutable unsigned m_flags; 208 223 … … 211 226 mutable String m_value; 212 227 213 private:214 228 friend class LLIntOffsetsExtractor; 215 229 … … 258 272 } 259 273 260 unsigned length() const { return m_jsString-> m_length; }274 unsigned length() const { return m_jsString->length(); } 261 275 262 276 private: … … 278 292 Base::finishCreation(vm); 279 293 ASSERT(!sumOverflows<int32_t>(s1->length(), s2->length())); 280 m_length = s1->length() + s2->length();294 setLength(s1->length() + s2->length()); 281 295 setIs8Bit(s1->is8Bit() && s2->is8Bit()); 282 296 setIsSubstring(false); … … 290 304 Base::finishCreation(vm); 291 305 ASSERT(!sumOverflows<int32_t>(s1->length(), s2->length(), s3->length())); 292 m_length = s1->length() + s2->length() + s3->length();306 setLength(s1->length() + s2->length() + s3->length()); 293 307 setIs8Bit(s1->is8Bit() && s2->is8Bit() && s3->is8Bit()); 294 308 setIsSubstring(false); … … 303 317 RELEASE_ASSERT(!sumOverflows<int32_t>(offset, length)); 304 318 RELEASE_ASSERT(offset + length <= base->length()); 305 m_length = length;319 setLength(length); 306 320 setIs8Bit(base->is8Bit()); 307 321 setIsSubstring(true); … … 327 341 RELEASE_ASSERT(!sumOverflows<int32_t>(offset, length)); 328 342 RELEASE_ASSERT(offset + length <= base->length()); 329 m_length = length;343 setLength(length); 330 344 setIs8Bit(base->is8Bit()); 331 345 setIsSubstring(true); … … 346 360 { 347 361 fiber(index).set(vm, this, jsString); 348 m_length += jsString->m_length; 349 RELEASE_ASSERT(static_cast<int32_t>(m_length) >= 0); 362 setLength(length() + jsString->length()); 350 363 setIs8Bit(is8Bit() && jsString->is8Bit()); 351 364 } … … 675 688 { 676 689 if (propertyName == exec->propertyNames().length) { 677 slot.setValue(this, DontEnum | DontDelete | ReadOnly, jsNumber( m_length));690 slot.setValue(this, DontEnum | DontDelete | ReadOnly, jsNumber(length())); 678 691 return true; 679 692 } 680 693 681 694 Optional<uint32_t> index = parseIndex(propertyName); 682 if (index && index.value() < m_length) {695 if (index && index.value() < length()) { 683 696 slot.setValue(this, DontDelete | ReadOnly, getIndex(exec, index.value())); 684 697 return true; … … 690 703 ALWAYS_INLINE bool JSString::getStringPropertySlot(ExecState* exec, unsigned propertyName, PropertySlot& slot) 691 704 { 692 if (propertyName < m_length) {705 if (propertyName < length()) { 693 706 slot.setValue(this, DontDelete | ReadOnly, getIndex(exec, propertyName)); 694 707 return true; … … 712 725 if (isSubstring()) { 713 726 if (is8Bit()) 714 return StringView(substringBase()->m_value.characters8() + substringOffset(), m_length);715 return StringView(substringBase()->m_value.characters16() + substringOffset(), m_length);727 return StringView(substringBase()->m_value.characters8() + substringOffset(), length()); 728 return StringView(substringBase()->m_value.characters16() + substringOffset(), length()); 716 729 } 717 730 resolveRope(&state); … … 724 737 auto& base = substringBase()->m_value; 725 738 if (is8Bit()) 726 return { { base.characters8() + substringOffset(), m_length}, base };727 return { { base.characters16() + substringOffset(), m_length}, base };739 return { { base.characters8() + substringOffset(), length() }, base }; 740 return { { base.characters16() + substringOffset(), length() }, base }; 728 741 } 729 742 resolveRope(&state); -
trunk/Source/JavaScriptCore/runtime/JSStringBuilder.h
r206525 r207849 132 132 auto scope = DECLARE_THROW_SCOPE(vm); 133 133 String result = WTF::tryMakeString(string, strings...); 134 if ( !result)134 if (UNLIKELY(!result || !JSString::isValidLength(result.length()))) 135 135 return throwOutOfMemoryError(exec, scope); 136 136 return jsNontrivialString(exec, WTFMove(result)); -
trunk/Source/JavaScriptCore/runtime/JSStringJoiner.cpp
r206386 r207849 89 89 return 0; 90 90 91 Checked< unsigned, RecordOverflow> separatorLength = m_separator.length();92 Checked< unsigned, RecordOverflow> totalSeparatorsLength = separatorLength * (numberOfStrings - 1);93 Checked< unsigned, RecordOverflow> totalLength = totalSeparatorsLength + m_accumulatedStringsLength;91 Checked<int32_t, RecordOverflow> separatorLength = m_separator.length(); 92 Checked<int32_t, RecordOverflow> totalSeparatorsLength = separatorLength * (numberOfStrings - 1); 93 Checked<int32_t, RecordOverflow> totalLength = totalSeparatorsLength + m_accumulatedStringsLength; 94 94 95 unsignedresult;95 int32_t result; 96 96 if (totalLength.safeGet(result) == CheckedState::DidOverflow) { 97 97 throwOutOfMemoryError(&state, scope);
Note: See TracChangeset
for help on using the changeset viewer.