[Webkit-unassigned] [Bug 65399] StackBounds checker fails on custom stack implementations (typically in a coroutine setting)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Jul 30 10:44:45 PDT 2011


https://bugs.webkit.org/show_bug.cgi?id=65399


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #102423|review?                     |review-
               Flag|                            |




--- Comment #4 from Darin Adler <darin at apple.com>  2011-07-30 10:44:45 PST ---
(From update of attachment 102423)
View in context: https://bugs.webkit.org/attachment.cgi?id=102423&action=review

I think that Geoff Garen or Oliver Hunt should take a look at this before we land it. Geoff will be away for a while because he is getting married today and is going on a honeymoon, so Oliver may be the best bet.

Mark Rowe may also be a good person to help us properly add new API getting the version checking right and having it exported properly.

> Source/JavaScriptCore/API/JSContextRef.cpp:70
> +    RefPtr<JSGlobalData> globalData = PassRefPtr<JSGlobalData>(toJS(group));
> +    globalData->stack()->setBounds(origin, bound);

There is no reason to put this into a local variable or use a RefPtr. It’s also not helpful and wrong style to use PassRefPtr here at all. It should just be toJS(group)->stack()->setBounds

> Source/JavaScriptCore/API/JSContextRef.h:80
> +JS_EXPORT void JSSetStackBounds(JSContextGroupRef, void *origin, void *bound) AVAILABLE_IN_WEBKIT_VERSION_4_0;

AVAILABLE_IN_WEBKIT_VERSION_4_0 is incorrect. This is something we just added, so it wasn’t available before.

In this file arguments need names, because it’s a C header file, not C++, so JSContextGroupRef needs the name "group" as you see above in other functions.

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:588
> -        StackBounds m_stack;
> +        StackBounds *m_stack;

Formatting here is incorrect. The * goes next to the type.

> Source/JavaScriptCore/runtime/JSGlobalData.h:294
> -        StackBounds m_stack;
> +        StackBounds *m_stack;

Same problem with placement of the *.

> Source/JavaScriptCore/wtf/StackBounds.h:81
> +    void setBounds(void *origin, void *bound)

Incorrect formatting here. It's void* not void * in WebKit code.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


More information about the webkit-unassigned mailing list