571065 - TypedArray uses invalid reinterpret_cast to access private pointer

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Joachim Kuebart]

Attachment: Patch that makes the problem of invalid casts apparent

I think that there is a slightly subtle bug in the handling of the private pointer in TypedArray and its subclasses:

TypedArray::fromJSObject performs reinterpret_cast<TypedArray *> on the private pointer. However, TypedArrayTemplate::makeFastWithPrivate calls obj->setPrivate with a pointer of type ThisTypedArray *. As I understand C++98 5.2.10/7 this is not guaranteed to work. It works in this case because ThisTypedArray only has one base class, in which case the cast appears to be the identity for most compilers.

By adding a second base class to TypedArrayTemplate an error can be provoked with the attached patch (tested on MSVC 9 aka VS2008). After this modification, the pointer used in TypedArray::prop_getBuffer, for example, is bogus, as can be seen in the debugger or by using the returned ArrayBuffer object.

Comment 1

[Joachim Kuebart]

Attachment: Test that accesses members of TypedArray as well as its subclass.

This test invokes TypedArray::prop_getBuffer as well as TypedArrayTemplat<Int32>::fun_slice to show different private pointer casts.

Comment 2

[Joachim Kuebart]

Attachment: Proposed fix

This patch fixes the problem by ensuring that the private pointer of a TypedArray subclass's JavaScript wrapper is always of type TypedArray *. The private pointer must always be set by calling the new TypedArray::setJSPrivate method.

Furthermore, TypedArrayTemplate::fromJSObject uses the superclass implementation to get a correctly typed private pointer and then uses static_cast<ThisTypeArray *> to correctly down-cast the result.

Comment 3

[Josh Matthews [:jdm]]

These patches were apparently never looked at by anyone. Do they still address a present problem?

Comment 4

[Joachim Kuebart]

The code has since changed dramatically and the problem I described doesn't exist anymore. This bug can be closed.