Skip to content

Commit d9d1363

Browse files
geoffreygarengeoffreygaren
committed
2011-03-14 Geoffrey Garen <ggaren@apple.com>
 Reviewed by Oliver Hunt. Made JSWrapperObject and subclasses moving-GC-safe https://bugs.webkit.org/show_bug.cgi?id=56346 SunSpider reports no change. * runtime/BooleanObject.cpp: (JSC::BooleanObject::BooleanObject): * runtime/DateInstance.cpp: (JSC::DateInstance::DateInstance): No more need for JSGlobalData, since we don't initialize the wrapped value in our constructor. * runtime/DateInstance.h: Don't set the OverridesMarkChildren flag because we do not in fact override markChildren. * runtime/DatePrototype.h: Declare an anonymous slot, since wrapper object no longer does so for us. Also added an ASSERT to catch a latent bug, where DatePrototype stomped on its base class's anonymous slot. Hard-coded anonymous slots are a plague on our code. This doesn't cause any problems in our existing code since the base class never reads the anonymous slot it declares, but it caused crashes when I tried to start using the slot in an initial version of this patch. * runtime/JSWrapperObject.h: (JSC::JSWrapperObject::JSWrapperObject): (JSC::JSWrapperObject::internalValue): (JSC::JSWrapperObject::setInternalValue): Resolved a problem where our internal value was stored in two places: an anonymous slot, and a data member which was not always visited during GC. Now, we only use the data member, and we always visit it. (Instead of relying on certain subclasses to set the OverridesMarkChildren bit, we set it ourselves.) * runtime/NumberObject.cpp: (JSC::NumberObject::NumberObject): No more need for JSGlobalData, since we don't initialize the wrapped value in our constructor. * runtime/NumberObject.h: Removed meaningless declaration. * runtime/StringObject.cpp: (JSC::StringObject::StringObject): No more need for JSGlobalData, since we don't initialize the wrapped value in our constructor. * runtime/StringObject.h: Don't set the OverridesMarkChildren flag because we do not in fact override markChildren. * runtime/StringPrototype.h: Declare an anonymous slot, since wrapper object no longer does so for us. Also added an ASSERT to catch a latent bug, where DatePrototype stomped on its base class's anonymous slot. Hard-coded anonymous slots are a plague on our code. Canonical link: https://commits.webkit.org/70966@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@81086 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent c8d747c commit d9d1363

11 files changed

Lines changed: 82 additions & 24 deletions

‎Source/JavaScriptCore/ChangeLog‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,56 @@
1+
2011-03-14 Geoffrey Garen <ggaren@apple.com>
2+
3+
Reviewed by Oliver Hunt.
4+
5+
Made JSWrapperObject and subclasses moving-GC-safe
6+
https://bugs.webkit.org/show_bug.cgi?id=56346
7+
8+
SunSpider reports no change.
9+
10+
* runtime/BooleanObject.cpp:
11+
(JSC::BooleanObject::BooleanObject):
12+
* runtime/DateInstance.cpp:
13+
(JSC::DateInstance::DateInstance): No more need for JSGlobalData, since
14+
we don't initialize the wrapped value in our constructor.
15+
16+
* runtime/DateInstance.h: Don't set the OverridesMarkChildren flag because
17+
we do not in fact override markChildren.
18+
19+
* runtime/DatePrototype.h: Declare an anonymous slot, since wrapper object
20+
no longer does so for us. Also added an ASSERT to catch a latent bug,
21+
where DatePrototype stomped on its base class's anonymous slot. Hard-coded
22+
anonymous slots are a plague on our code. This doesn't cause any problems
23+
in our existing code since the base class never reads the anonymous slot
24+
it declares, but it caused crashes when I tried to start using the slot
25+
in an initial version of this patch.
26+
27+
* runtime/JSWrapperObject.h:
28+
(JSC::JSWrapperObject::JSWrapperObject):
29+
(JSC::JSWrapperObject::internalValue):
30+
(JSC::JSWrapperObject::setInternalValue): Resolved a problem where
31+
our internal value was stored in two places: an anonymous slot, and a
32+
data member which was not always visited during GC. Now, we only use the
33+
data member, and we always visit it. (Instead of relying on certain
34+
subclasses to set the OverridesMarkChildren bit, we set it ourselves.)
35+
36+
* runtime/NumberObject.cpp:
37+
(JSC::NumberObject::NumberObject): No more need for JSGlobalData, since
38+
we don't initialize the wrapped value in our constructor.
39+
40+
* runtime/NumberObject.h: Removed meaningless declaration.
41+
42+
* runtime/StringObject.cpp:
43+
(JSC::StringObject::StringObject): No more need for JSGlobalData, since
44+
we don't initialize the wrapped value in our constructor.
45+
46+
* runtime/StringObject.h: Don't set the OverridesMarkChildren flag because
47+
we do not in fact override markChildren.
48+
49+
* runtime/StringPrototype.h: Declare an anonymous slot, since wrapper object
50+
no longer does so for us. Also added an ASSERT to catch a latent bug,
51+
where DatePrototype stomped on its base class's anonymous slot. Hard-coded
52+
anonymous slots are a plague on our code.
53+
154
2011-03-14 Michael Saboff <msaboff@apple.com>
255

356
Reviewed by Gavin Barraclough.

‎Source/JavaScriptCore/runtime/BooleanObject.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ ASSERT_CLASS_FITS_IN_CELL(BooleanObject);
2727

2828
const ClassInfo BooleanObject::s_info = { "Boolean", &JSWrapperObject::s_info, 0, 0 };
2929

30-
BooleanObject::BooleanObject(JSGlobalData& globalData, NonNullPassRefPtr<Structure> structure)
31-
: JSWrapperObject(globalData, structure)
30+
BooleanObject::BooleanObject(JSGlobalData&, NonNullPassRefPtr<Structure> structure)
31+
: JSWrapperObject(structure)
3232
{
3333
ASSERT(inherits(&s_info));
3434
}

‎Source/JavaScriptCore/runtime/DateInstance.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,21 +35,21 @@ namespace JSC {
3535
const ClassInfo DateInstance::s_info = {"Date", &JSWrapperObject::s_info, 0, 0};
3636

3737
DateInstance::DateInstance(ExecState* exec, NonNullPassRefPtr<Structure> structure)
38-
: JSWrapperObject(exec->globalData(), structure)
38+
: JSWrapperObject(structure)
3939
{
4040
ASSERT(inherits(&s_info));
4141
setInternalValue(exec->globalData(), jsNaN());
4242
}
4343

4444
DateInstance::DateInstance(ExecState* exec, NonNullPassRefPtr<Structure> structure, double time)
45-
: JSWrapperObject(exec->globalData(), structure)
45+
: JSWrapperObject(structure)
4646
{
4747
ASSERT(inherits(&s_info));
4848
setInternalValue(exec->globalData(), jsNumber(timeClip(time)));
4949
}
5050

5151
DateInstance::DateInstance(ExecState* exec, double time)
52-
: JSWrapperObject(exec->globalData(), exec->lexicalGlobalObject()->dateStructure())
52+
: JSWrapperObject(exec->lexicalGlobalObject()->dateStructure())
5353
{
5454
ASSERT(inherits(&s_info));
5555
setInternalValue(exec->globalData(), jsNumber(timeClip(time)));

‎Source/JavaScriptCore/runtime/DateInstance.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,6 @@ namespace JSC {
5858
return Structure::create(prototype, TypeInfo(ObjectType, StructureFlags), AnonymousSlotCount, &s_info);
5959
}
6060

61-
protected:
62-
static const unsigned StructureFlags = OverridesMarkChildren | JSWrapperObject::StructureFlags;
63-
6461
private:
6562
const GregorianDateTime* calculateGregorianDateTime(ExecState*) const;
6663
const GregorianDateTime* calculateGregorianDateTimeUTC(ExecState*) const;

‎Source/JavaScriptCore/runtime/DatePrototype.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ namespace JSC {
4444
protected:
4545
static const unsigned StructureFlags = OverridesGetOwnPropertySlot | DateInstance::StructureFlags;
4646

47+
COMPILE_ASSERT(!DateInstance::AnonymousSlotCount, DatePrototype_stomps_on_your_anonymous_slot);
48+
static const unsigned AnonymousSlotCount = 1;
4749
};
4850

4951
} // namespace JSC

‎Source/JavaScriptCore/runtime/JSWrapperObject.h‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ namespace JSC {
3030
// Number, Boolean and Date which are wrappers for primitive types.
3131
class JSWrapperObject : public JSNonFinalObject {
3232
protected:
33-
explicit JSWrapperObject(JSGlobalData&, NonNullPassRefPtr<Structure>);
33+
explicit JSWrapperObject(NonNullPassRefPtr<Structure>);
3434

3535
public:
36-
JSValue internalValue() const { return m_internalValue.get(); }
36+
JSValue internalValue() const;
3737
void setInternalValue(JSGlobalData&, JSValue);
3838

3939
static PassRefPtr<Structure> createStructure(JSValue prototype)
@@ -42,26 +42,29 @@ namespace JSC {
4242
}
4343

4444
protected:
45-
static const unsigned AnonymousSlotCount = 1 + JSObject::AnonymousSlotCount;
45+
static const unsigned StructureFlags = OverridesMarkChildren | JSNonFinalObject::StructureFlags;
4646

4747
private:
4848
virtual void markChildren(MarkStack&);
4949

5050
WriteBarrier<Unknown> m_internalValue;
5151
};
5252

53-
inline JSWrapperObject::JSWrapperObject(JSGlobalData& globalData, NonNullPassRefPtr<Structure> structure)
53+
inline JSWrapperObject::JSWrapperObject(NonNullPassRefPtr<Structure> structure)
5454
: JSNonFinalObject(structure)
5555
{
56-
putAnonymousValue(globalData, 0, jsNull());
56+
}
57+
58+
inline JSValue JSWrapperObject::internalValue() const
59+
{
60+
return m_internalValue.get();
5761
}
5862

5963
inline void JSWrapperObject::setInternalValue(JSGlobalData& globalData, JSValue value)
6064
{
6165
ASSERT(value);
6266
ASSERT(!value.isObject());
6367
m_internalValue.set(globalData, this, value);
64-
putAnonymousValue(globalData, 0, value);
6568
}
6669

6770
} // namespace JSC

‎Source/JavaScriptCore/runtime/NumberObject.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ ASSERT_CLASS_FITS_IN_CELL(NumberObject);
3131

3232
const ClassInfo NumberObject::s_info = { "Number", &JSWrapperObject::s_info, 0, 0 };
3333

34-
NumberObject::NumberObject(JSGlobalData& globalData, NonNullPassRefPtr<Structure> structure)
35-
: JSWrapperObject(globalData, structure)
34+
NumberObject::NumberObject(JSGlobalData&, NonNullPassRefPtr<Structure> structure)
35+
: JSWrapperObject(structure)
3636
{
3737
ASSERT(inherits(&s_info));
3838
}

‎Source/JavaScriptCore/runtime/NumberObject.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,6 @@ namespace JSC {
3636
return Structure::create(prototype, TypeInfo(ObjectType, StructureFlags), AnonymousSlotCount, &s_info);
3737
}
3838

39-
protected:
40-
static const unsigned StructureFlags = JSWrapperObject::StructureFlags;
41-
4239
private:
4340
virtual JSValue getJSNumber();
4441
};

‎Source/JavaScriptCore/runtime/StringObject.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,21 +30,21 @@ ASSERT_CLASS_FITS_IN_CELL(StringObject);
3030
const ClassInfo StringObject::s_info = { "String", &JSWrapperObject::s_info, 0, 0 };
3131

3232
StringObject::StringObject(ExecState* exec, NonNullPassRefPtr<Structure> structure)
33-
: JSWrapperObject(exec->globalData(), structure)
33+
: JSWrapperObject(structure)
3434
{
3535
ASSERT(inherits(&s_info));
3636
setInternalValue(exec->globalData(), jsEmptyString(exec));
3737
}
3838

3939
StringObject::StringObject(JSGlobalData& globalData, NonNullPassRefPtr<Structure> structure, JSString* string)
40-
: JSWrapperObject(globalData, structure)
40+
: JSWrapperObject(structure)
4141
{
4242
ASSERT(inherits(&s_info));
4343
setInternalValue(globalData, string);
4444
}
4545

4646
StringObject::StringObject(ExecState* exec, NonNullPassRefPtr<Structure> structure, const UString& string)
47-
: JSWrapperObject(exec->globalData(), structure)
47+
: JSWrapperObject(structure)
4848
{
4949
ASSERT(inherits(&s_info));
5050
setInternalValue(exec->globalData(), jsString(exec, string));

‎Source/JavaScriptCore/runtime/StringObject.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,9 @@ namespace JSC {
5151
}
5252

5353
protected:
54-
static const unsigned StructureFlags = OverridesGetOwnPropertySlot | OverridesMarkChildren | OverridesGetPropertyNames | JSWrapperObject::StructureFlags;
54+
static const unsigned StructureFlags = OverridesGetOwnPropertySlot | OverridesGetPropertyNames | JSWrapperObject::StructureFlags;
5555
StringObject(JSGlobalData&, NonNullPassRefPtr<Structure>, JSString*);
56-
};
56+
};
5757

5858
StringObject* asStringObject(JSValue);
5959

0 commit comments

Comments
 (0)