Patch forthcoming.
Thus, if we have a GetByOffset with a given offset, we can check if the object was proved to have a structure that has that offset; if it does, then the GetByOffset can proceed.
Created attachment 207054 [details] the patch
Created attachment 207110 [details] the patch
Created attachment 207125 [details] the patch Just improved the changelog.
Comment on attachment 207125 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=207125&action=review > Source/JavaScriptCore/ChangeLog:35 > + that could transitively try to compute the hash from the source code. The source > + code is a string that may be lazily computed, and that involves all manner of thread > + unsafe things. Do we have any assertions that would catch this?
(In reply to comment #5) > (From update of attachment 207125 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=207125&action=review > > > Source/JavaScriptCore/ChangeLog:35 > > + that could transitively try to compute the hash from the source code. The source > > + code is a string that may be lazily computed, and that involves all manner of thread > > + unsafe things. > > Do we have any assertions that would catch this? Yes, that's how I caught it. StringImpl's ref/deref assert that you're not in the compiler thread.
Comment on attachment 207125 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=207125&action=review > Source/JavaScriptCore/runtime/Structure.h:228 > bool isValidOffset(PropertyOffset offset) const > { > - return offset >= firstValidOffset() > - && offset <= lastValidOffset(); > + return JSC::isValidOffset(offset) > + && (offset < m_inlineCapacity > + || (offset >= firstOutOfLineOffset && offset <= m_offset)); > } I think it's worth a comment here explaining that a "valid" offset isn't necessarily a valid JSValue -- it's just an offset that, if you load from it, you won't crash.
Landed in http://trac.webkit.org/changeset/152909
(In reply to comment #7) > (From update of attachment 207125 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=207125&action=review > > > Source/JavaScriptCore/runtime/Structure.h:228 > > bool isValidOffset(PropertyOffset offset) const > > { > > - return offset >= firstValidOffset() > > - && offset <= lastValidOffset(); > > + return JSC::isValidOffset(offset) > > + && (offset < m_inlineCapacity > > + || (offset >= firstOutOfLineOffset && offset <= m_offset)); > > } > > I think it's worth a comment here explaining that a "valid" offset isn't necessarily a valid JSValue -- it's just an offset that, if you load from it, you won't crash. Interesting. While that's not what I intended for this method, its users are indeed file with the "you won't crash" part. I'll change it though, in a separate bug.
(In reply to comment #9) > (In reply to comment #7) > > (From update of attachment 207125 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=207125&action=review > > > > > Source/JavaScriptCore/runtime/Structure.h:228 > > > bool isValidOffset(PropertyOffset offset) const > > > { > > > - return offset >= firstValidOffset() > > > - && offset <= lastValidOffset(); > > > + return JSC::isValidOffset(offset) > > > + && (offset < m_inlineCapacity > > > + || (offset >= firstOutOfLineOffset && offset <= m_offset)); > > > } > > > > I think it's worth a comment here explaining that a "valid" offset isn't necessarily a valid JSValue -- it's just an offset that, if you load from it, you won't crash. > > Interesting. While that's not what I intended for this method, its users are indeed file with the "you won't crash" part. I'll change it though, in a separate bug. https://bugs.webkit.org/show_bug.cgi?id=118911