Title: [263193] trunk/Source/_javascript_Core
Revision
263193
Author
mark....@apple.com
Date
2020-06-17 18:37:28 -0700 (Wed, 17 Jun 2020)

Log Message

StructureIDTable::validate() doesn't work when compiled with GCC.
https://bugs.webkit.org/show_bug.cgi?id=213302
<rdar://problem/64452172>

Reviewed by Yusuke Suzuki.

I was previously using ensureStillAliveHere() to force the validation load to
not be elided.  However, this is not how ensureStillAliveHere() works.  The proper
way to force the load is to use a volatile pointer instead, which is applied in
this patch.

With Clang, the ensureStillAliveHere() happened to do what I expected, but with
GCC it did not.  The compiler is at liberty to elide the load because there is
no memory clobbering operation between the load and the call to
ensureStillAliveHere().  Switching to using the volatile pointer solution.

* runtime/StructureIDTable.h:
(JSC::StructureIDTable::validate):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (263192 => 263193)


--- trunk/Source/_javascript_Core/ChangeLog	2020-06-17 23:57:59 UTC (rev 263192)
+++ trunk/Source/_javascript_Core/ChangeLog	2020-06-18 01:37:28 UTC (rev 263193)
@@ -1,3 +1,24 @@
+2020-06-17  Mark Lam  <mark....@apple.com>
+
+        StructureIDTable::validate() doesn't work when compiled with GCC.
+        https://bugs.webkit.org/show_bug.cgi?id=213302
+        <rdar://problem/64452172>
+
+        Reviewed by Yusuke Suzuki.
+
+        I was previously using ensureStillAliveHere() to force the validation load to
+        not be elided.  However, this is not how ensureStillAliveHere() works.  The proper
+        way to force the load is to use a volatile pointer instead, which is applied in
+        this patch.
+
+        With Clang, the ensureStillAliveHere() happened to do what I expected, but with
+        GCC it did not.  The compiler is at liberty to elide the load because there is
+        no memory clobbering operation between the load and the call to
+        ensureStillAliveHere().  Switching to using the volatile pointer solution.
+
+        * runtime/StructureIDTable.h:
+        (JSC::StructureIDTable::validate):
+
 2020-06-17  Yusuke Suzuki  <ysuz...@apple.com>
 
         [JSC] Freeze JSBigInt when setting it as a constant in AI

Modified: trunk/Source/_javascript_Core/runtime/StructureIDTable.h (263192 => 263193)


--- trunk/Source/_javascript_Core/runtime/StructureIDTable.h	2020-06-17 23:57:59 UTC (rev 263192)
+++ trunk/Source/_javascript_Core/runtime/StructureIDTable.h	2020-06-18 01:37:28 UTC (rev 263193)
@@ -183,8 +183,7 @@
     uint32_t structureIndex = structureID >> s_numberOfEntropyBits;
     Structure* structure = decode(table()[structureIndex].encodedStructureBits, structureID);
     RELEASE_ASSERT(structureIndex < m_capacity);
-    uint64_t value = *bitwise_cast<uint64_t*>(structure);
-    ensureStillAliveHere(value);
+    *bitwise_cast<volatile uint64_t*>(structure);
 }
 
 #else // not USE(JSVALUE64)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to