2011-03-18 Geoffrey Garen <ggaren@apple.com>

        Reviewed by Oliver Hunt.

        [GTK] JSC crashes in 32bit Release bots after r80743
        https://bugs.webkit.org/show_bug.cgi?id=56180

        The crash was caused by referencing GC memory from a GC destructor. This
        is not safe because destruction time / order is not guaranteed.

        * profiler/ProfileGenerator.cpp:
        (JSC::ProfileGenerator::create):
        (JSC::ProfileGenerator::ProfileGenerator):
        (JSC::ProfileGenerator::willExecute):
        (JSC::ProfileGenerator::didExecute):
        * profiler/ProfileGenerator.h:
        (JSC::ProfileGenerator::origin): Made ExecState* the first argument,
        to match the rest of this class and JSC.

        Use a JSGlobalObject* instead of an ExecState* with an indirect reference
        to a JSGlobalObject* to track our origin. This is simpler and more
        efficient, and it removes the destruction order dependency that was causing
        our crash.

        * profiler/Profiler.cpp:
        (JSC::Profiler::startProfiling): Updated for change to JSGlobalObject*.
        (JSC::Profiler::stopProfiling): New function for stopping all profiles
        for a given global object. This is more straight-forward than multiplexing
        through the old function.

        (JSC::dispatchFunctionToProfiles): Updated for change to JSGlobalObject*.
        * profiler/Profiler.h: Ditto.

        * runtime/JSGlobalObject.cpp:
        (JSC::JSGlobalObject::~JSGlobalObject): Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@81525 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index feb22af50343064cb72a249bfae5600c6bf6f47b..3e6ec3157d0426cb15cb23350d8c317491ace9d0 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,39 @@
+2011-03-18  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        [GTK] JSC crashes in 32bit Release bots after r80743
+        https://bugs.webkit.org/show_bug.cgi?id=56180
+        
+        The crash was caused by referencing GC memory from a GC destructor. This
+        is not safe because destruction time / order is not guaranteed.
+
+        * profiler/ProfileGenerator.cpp:
+        (JSC::ProfileGenerator::create):
+        (JSC::ProfileGenerator::ProfileGenerator):
+        (JSC::ProfileGenerator::willExecute):
+        (JSC::ProfileGenerator::didExecute):
+        * profiler/ProfileGenerator.h:
+        (JSC::ProfileGenerator::origin): Made ExecState* the first argument,
+        to match the rest of this class and JSC.
+        
+        Use a JSGlobalObject* instead of an ExecState* with an indirect reference
+        to a JSGlobalObject* to track our origin. This is simpler and more
+        efficient, and it removes the destruction order dependency that was causing
+        our crash.
+
+        * profiler/Profiler.cpp:
+        (JSC::Profiler::startProfiling): Updated for change to JSGlobalObject*.
+        (JSC::Profiler::stopProfiling): New function for stopping all profiles
+        for a given global object. This is more straight-forward than multiplexing
+        through the old function.
+
+        (JSC::dispatchFunctionToProfiles): Updated for change to JSGlobalObject*.
+        * profiler/Profiler.h: Ditto.
+
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::~JSGlobalObject): Ditto.
+

 2011-03-17  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Oliver Hunt.
 2011-03-17  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Oliver Hunt.

diff --git a/Source/JavaScriptCore/profiler/ProfileGenerator.cpp b/Source/JavaScriptCore/profiler/ProfileGenerator.cpp
index 68d17330a210b85869bafdd6cc462d5050d397ec..5db38bca8d56a5ef01d28a5f942b0fd93027fbe2 100644 (file)
--- a/Source/JavaScriptCore/profiler/ProfileGenerator.cpp
+++ b/Source/JavaScriptCore/profiler/ProfileGenerator.cpp
@@ -40,19 +40,19 @@ namespace JSC {
 
 static const char* NonJSExecution = "(idle)";
 
 
 static const char* NonJSExecution = "(idle)";
 

-PassRefPtr<ProfileGenerator> ProfileGenerator::create(const UString& title, ExecState* originatingExec, unsigned uid)
+PassRefPtr<ProfileGenerator> ProfileGenerator::create(ExecState* exec, const UString& title, unsigned uid)

 {
 {

-    return adoptRef(new ProfileGenerator(title, originatingExec, uid));
+    return adoptRef(new ProfileGenerator(exec, title, uid));

 }
 
 }
 

-ProfileGenerator::ProfileGenerator(const UString& title, ExecState* originatingExec, unsigned uid)
-    : m_originatingGlobalExec(originatingExec ? originatingExec->lexicalGlobalObject()->globalExec() : 0)
-    , m_profileGroup(originatingExec ? originatingExec->lexicalGlobalObject()->profileGroup() : 0)
+ProfileGenerator::ProfileGenerator(ExecState* exec, const UString& title, unsigned uid)
+    : m_origin(exec ? exec->lexicalGlobalObject() : 0)
+    , m_profileGroup(exec ? exec->lexicalGlobalObject()->profileGroup() : 0)

 {
     m_profile = Profile::create(title, uid);
     m_currentNode = m_head = m_profile->head();
 {
     m_profile = Profile::create(title, uid);
     m_currentNode = m_head = m_profile->head();

-    if (originatingExec)
-        addParentForConsoleStart(originatingExec);
+    if (exec)
+        addParentForConsoleStart(exec);

 }
 
 void ProfileGenerator::addParentForConsoleStart(ExecState* exec)
 }
 
 void ProfileGenerator::addParentForConsoleStart(ExecState* exec)


@@ -80,7 +80,7 @@ void ProfileGenerator::willExecute(ExecState* callerCallFrame, const CallIdentif
         JAVASCRIPTCORE_PROFILE_WILL_EXECUTE(m_profileGroup, const_cast<char*>(name.data()), const_cast<char*>(url.data()), callIdentifier.m_lineNumber);
     }
 
         JAVASCRIPTCORE_PROFILE_WILL_EXECUTE(m_profileGroup, const_cast<char*>(name.data()), const_cast<char*>(url.data()), callIdentifier.m_lineNumber);
     }
 

-    if (!m_originatingGlobalExec)
+    if (!m_origin)

         return;
 
     ASSERT(m_currentNode);
         return;
 
     ASSERT(m_currentNode);


@@ -95,7 +95,7 @@ void ProfileGenerator::didExecute(ExecState* callerCallFrame, const CallIdentifi
         JAVASCRIPTCORE_PROFILE_DID_EXECUTE(m_profileGroup, const_cast<char*>(name.data()), const_cast<char*>(url.data()), callIdentifier.m_lineNumber);
     }
 
         JAVASCRIPTCORE_PROFILE_DID_EXECUTE(m_profileGroup, const_cast<char*>(name.data()), const_cast<char*>(url.data()), callIdentifier.m_lineNumber);
     }
 

-    if (!m_originatingGlobalExec)
+    if (!m_origin)

         return;
 
     ASSERT(m_currentNode);
         return;
 
     ASSERT(m_currentNode);

diff --git a/Source/JavaScriptCore/profiler/ProfileGenerator.h b/Source/JavaScriptCore/profiler/ProfileGenerator.h
index cbed73b6302433b56edb29b4a5033340835ab3b9..8c8b81731535619d14233dff0fdb7d1b22880adf 100644 (file)
--- a/Source/JavaScriptCore/profiler/ProfileGenerator.h
+++ b/Source/JavaScriptCore/profiler/ProfileGenerator.h
@@ -34,6 +34,7 @@
 namespace JSC {
 
     class ExecState;
 namespace JSC {
 
     class ExecState;

+    class JSGlobalObject;

     class Profile;
     class ProfileNode;
     class UString;
     class Profile;
     class ProfileNode;
     class UString;


@@ -41,12 +42,12 @@ namespace JSC {
 
     class ProfileGenerator : public RefCounted<ProfileGenerator>  {
     public:
 
     class ProfileGenerator : public RefCounted<ProfileGenerator>  {
     public:

-        static PassRefPtr<ProfileGenerator> create(const UString& title, ExecState* originatingExec, unsigned uid);
+        static PassRefPtr<ProfileGenerator> create(ExecState*, const UString& title, unsigned uid);

 
         // Members
         const UString& title() const;
         PassRefPtr<Profile> profile() const { return m_profile; }
 
         // Members
         const UString& title() const;
         PassRefPtr<Profile> profile() const { return m_profile; }

-        ExecState* originatingGlobalExec() const { return m_originatingGlobalExec; }
+        JSGlobalObject* origin() const { return m_origin; }

         unsigned profileGroup() const { return m_profileGroup; }
 
         // Collecting
         unsigned profileGroup() const { return m_profileGroup; }
 
         // Collecting


@@ -61,14 +62,14 @@ namespace JSC {
         typedef void (ProfileGenerator::*ProfileFunction)(ExecState* callerOrHandlerCallFrame, const CallIdentifier& callIdentifier);
 
     private:
         typedef void (ProfileGenerator::*ProfileFunction)(ExecState* callerOrHandlerCallFrame, const CallIdentifier& callIdentifier);
 
     private:

-        ProfileGenerator(const UString& title, ExecState* originatingExec, unsigned uid);
+        ProfileGenerator(ExecState*, const UString& title, unsigned uid);

         void addParentForConsoleStart(ExecState*);
 
         void removeProfileStart();
         void removeProfileEnd();
 
         RefPtr<Profile> m_profile;
         void addParentForConsoleStart(ExecState*);
 
         void removeProfileStart();
         void removeProfileEnd();
 
         RefPtr<Profile> m_profile;

-        ExecState* m_originatingGlobalExec;
+        JSGlobalObject* m_origin;

         unsigned m_profileGroup;
         RefPtr<ProfileNode> m_head;
         RefPtr<ProfileNode> m_currentNode;
         unsigned m_profileGroup;
         RefPtr<ProfileNode> m_head;
         RefPtr<ProfileNode> m_currentNode;

diff --git a/Source/JavaScriptCore/profiler/Profiler.cpp b/Source/JavaScriptCore/profiler/Profiler.cpp
index 301dc0c1cf092c36af8c8f36f638e5351d496c1c..bcaaaacbf217b08425f4327ca8bd37bf23a14a26 100644 (file)
--- a/Source/JavaScriptCore/profiler/Profiler.cpp
+++ b/Source/JavaScriptCore/profiler/Profiler.cpp
@@ -66,25 +66,25 @@ void Profiler::startProfiling(ExecState* exec, const UString& title)
 
     // Check if we currently have a Profile for this global ExecState and title.
     // If so return early and don't create a new Profile.
 
     // Check if we currently have a Profile for this global ExecState and title.
     // If so return early and don't create a new Profile.

-    ExecState* globalExec = exec ? exec->lexicalGlobalObject()->globalExec() : 0;
+    JSGlobalObject* origin = exec ? exec->lexicalGlobalObject() : 0;

 
     for (size_t i = 0; i < m_currentProfiles.size(); ++i) {
         ProfileGenerator* profileGenerator = m_currentProfiles[i].get();
 
     for (size_t i = 0; i < m_currentProfiles.size(); ++i) {
         ProfileGenerator* profileGenerator = m_currentProfiles[i].get();

-        if (profileGenerator->originatingGlobalExec() == globalExec && profileGenerator->title() == title)
+        if (profileGenerator->origin() == origin && profileGenerator->title() == title)

             return;
     }
 
     s_sharedEnabledProfilerReference = this;
             return;
     }
 
     s_sharedEnabledProfilerReference = this;

-    RefPtr<ProfileGenerator> profileGenerator = ProfileGenerator::create(title, exec, ++ProfilesUID);
+    RefPtr<ProfileGenerator> profileGenerator = ProfileGenerator::create(exec, title, ++ProfilesUID);

     m_currentProfiles.append(profileGenerator);
 }
 
 PassRefPtr<Profile> Profiler::stopProfiling(ExecState* exec, const UString& title)
 {
     m_currentProfiles.append(profileGenerator);
 }
 
 PassRefPtr<Profile> Profiler::stopProfiling(ExecState* exec, const UString& title)
 {

-    ExecState* globalExec = exec ? exec->lexicalGlobalObject()->globalExec() : 0;
+    JSGlobalObject* origin = exec ? exec->lexicalGlobalObject() : 0;

     for (ptrdiff_t i = m_currentProfiles.size() - 1; i >= 0; --i) {
         ProfileGenerator* profileGenerator = m_currentProfiles[i].get();
     for (ptrdiff_t i = m_currentProfiles.size() - 1; i >= 0; --i) {
         ProfileGenerator* profileGenerator = m_currentProfiles[i].get();

-        if (profileGenerator->originatingGlobalExec() == globalExec && (title.isNull() || profileGenerator->title() == title)) {
+        if (profileGenerator->origin() == origin && (title.isNull() || profileGenerator->title() == title)) {

             profileGenerator->stopProfiling();
             RefPtr<Profile> returnProfile = profileGenerator->profile();
 
             profileGenerator->stopProfiling();
             RefPtr<Profile> returnProfile = profileGenerator->profile();
 


@@ -99,10 +99,23 @@ PassRefPtr<Profile> Profiler::stopProfiling(ExecState* exec, const UString& titl
     return 0;
 }
 
     return 0;
 }
 

+void Profiler::stopProfiling(JSGlobalObject* origin)
+{
+    for (ptrdiff_t i = m_currentProfiles.size() - 1; i >= 0; --i) {
+        ProfileGenerator* profileGenerator = m_currentProfiles[i].get();
+        if (profileGenerator->origin() == origin) {
+            profileGenerator->stopProfiling();
+            m_currentProfiles.remove(i);
+            if (!m_currentProfiles.size())
+                s_sharedEnabledProfilerReference = 0;
+        }
+    }
+}
+

 static inline void dispatchFunctionToProfiles(ExecState* callerOrHandlerCallFrame, const Vector<RefPtr<ProfileGenerator> >& profiles, ProfileGenerator::ProfileFunction function, const CallIdentifier& callIdentifier, unsigned currentProfileTargetGroup)
 {
     for (size_t i = 0; i < profiles.size(); ++i) {
 static inline void dispatchFunctionToProfiles(ExecState* callerOrHandlerCallFrame, const Vector<RefPtr<ProfileGenerator> >& profiles, ProfileGenerator::ProfileFunction function, const CallIdentifier& callIdentifier, unsigned currentProfileTargetGroup)
 {
     for (size_t i = 0; i < profiles.size(); ++i) {

-        if (profiles[i]->profileGroup() == currentProfileTargetGroup || !profiles[i]->originatingGlobalExec())
+        if (profiles[i]->profileGroup() == currentProfileTargetGroup || !profiles[i]->origin())

             (profiles[i].get()->*function)(callerOrHandlerCallFrame, callIdentifier);
     }
 }
             (profiles[i].get()->*function)(callerOrHandlerCallFrame, callIdentifier);
     }
 }

diff --git a/Source/JavaScriptCore/profiler/Profiler.h b/Source/JavaScriptCore/profiler/Profiler.h
index f88746d6b767f06436a05be7a9a55dbbfc797a20..86366c1c4f9198d3df116229e55c0c9935fdea1d 100644 (file)
--- a/Source/JavaScriptCore/profiler/Profiler.h
+++ b/Source/JavaScriptCore/profiler/Profiler.h
@@ -38,6 +38,7 @@ namespace JSC {
 
     class ExecState;
     class JSGlobalData;
 
     class ExecState;
     class JSGlobalData;

+    class JSGlobalObject;

     class JSObject;
     class JSValue;
     class ProfileGenerator;
     class JSObject;
     class JSValue;
     class ProfileGenerator;


@@ -57,6 +58,7 @@ namespace JSC {
 
         void startProfiling(ExecState*, const UString& title);
         PassRefPtr<Profile> stopProfiling(ExecState*, const UString& title);
 
         void startProfiling(ExecState*, const UString& title);
         PassRefPtr<Profile> stopProfiling(ExecState*, const UString& title);

+        void stopProfiling(JSGlobalObject*);

 
         void willExecute(ExecState* callerCallFrame, JSValue function);
         void willExecute(ExecState* callerCallFrame, const UString& sourceURL, int startingLineNumber);
 
         void willExecute(ExecState* callerCallFrame, JSValue function);
         void willExecute(ExecState* callerCallFrame, const UString& sourceURL, int startingLineNumber);

diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
index ade054b1be35841fdf5cdfffa239d3fab9266d9b..89bbe804ed165c3a0c2cc9241c8447ceed34945b 100644 (file)
--- a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
@@ -99,7 +99,7 @@ JSGlobalObject::~JSGlobalObject()
 
     Profiler** profiler = Profiler::enabledProfilerReference();
     if (UNLIKELY(*profiler != 0)) {
 
     Profiler** profiler = Profiler::enabledProfilerReference();
     if (UNLIKELY(*profiler != 0)) {

-        (*profiler)->stopProfiling(globalExec(), UString());
+        (*profiler)->stopProfiling(this);

     }
 }
 
     }
 }