Bug 42004 - bufferSubData causes crash in WebGLBuffer::associateBufferSubData
Summary: bufferSubData causes crash in WebGLBuffer::associateBufferSubData
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Kenneth Russell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-09 16:44 PDT by Kenneth Russell
Modified: 2010-07-09 17:48 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.40 KB, patch)
2010-07-09 16:55 PDT, Kenneth Russell
japhet: review+
kbr: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Russell 2010-07-09 16:44:55 PDT
If an element array buffer object is initialized via bufferData(ELEMENT_ARRAY_BUFFER, size, usage) and then filled with bufferSubData(ELEMENT_ARRAY_BUFFER, offset, ArrayBufferView), the index validation code (WebGLBuffer::associateBufferSubData) crashes because the m_elementArrayBuffer has not been allocated.
Comment 1 Kenneth Russell 2010-07-09 16:55:27 PDT
Created attachment 61114 [details]
Patch

From the ChangeLog:

Allocate m_elementArrayBuffer for entry point taking only size. Guard against allocation failures of m_elementArrayBuffer. Guard against any possibility of crashes due to m_elementArrayBuffer being NULL.
Comment 2 Nate Chapin 2010-07-09 17:21:09 PDT
Comment on attachment 61114 [details]
Patch

>      case GraphicsContext3D::ELEMENT_ARRAY_BUFFER:
>      case GraphicsContext3D::ARRAY_BUFFER:
>          m_byteLength = size;
> +        if (m_target == GraphicsContext3D::ELEMENT_ARRAY_BUFFER) {
> +            clearCachedMaxIndices();
> +            m_elementArrayBuffer = ArrayBuffer::create(size, 1);
> +            if (!m_elementArrayBuffer) {
> +                m_byteLength = 0;
> +                return false;
> +            }
> +        }
>          return true;
>      default:
>          return false;

Style nit: exit early if m_target is 0, and remove the switch.
Comment 3 Kenneth Russell 2010-07-09 17:26:58 PDT
(In reply to comment #2)
> (From update of attachment 61114 [details])
> >      case GraphicsContext3D::ELEMENT_ARRAY_BUFFER:
> >      case GraphicsContext3D::ARRAY_BUFFER:
> >          m_byteLength = size;
> > +        if (m_target == GraphicsContext3D::ELEMENT_ARRAY_BUFFER) {
> > +            clearCachedMaxIndices();
> > +            m_elementArrayBuffer = ArrayBuffer::create(size, 1);
> > +            if (!m_elementArrayBuffer) {
> > +                m_byteLength = 0;
> > +                return false;
> > +            }
> > +        }
> >          return true;
> >      default:
> >          return false;
> 
> Style nit: exit early if m_target is 0, and remove the switch.

Will make this change in the landed version.
Comment 4 Kenneth Russell 2010-07-09 17:48:19 PDT
Committed r63017: <http://trac.webkit.org/changeset/63017>