Navigation Menu

Skip to content

Commit

Permalink
[gecko] Avoid incorrect compiler optimisation. JB#49864
Browse files Browse the repository at this point in the history
When building using gcc 8.3 and -O3, the null check on
IsInsideNursery() from js/public/HeapAPI.h gets dropped when used inside
the ProxyObject::New() method from js/src/vm/ProxyObject.cpp, called
from "priv.toGCThing()->isTenured()" on line 50 of the same file.

Since it's possible for the cell pointer to be null, the
over-optimisation can cause SIGSERV crashes.

SeaMonkey bug 1584533i details the same issue:
https://bugzilla.mozilla.org/show_bug.cgi?id=1584533

The fix adds the appropriate check back in directly to the
ProxyObject::New() code, which then doesn't get optimised out.
  • Loading branch information
llewelld committed May 13, 2020
1 parent 34e237d commit 1bb461b
Showing 1 changed file with 36 additions and 5 deletions.
41 changes: 36 additions & 5 deletions rpm/0017-gecko-Configuration-option.-JB-49613.patch
@@ -1,7 +1,23 @@
From 9e7510ddfa6954a7480a6d6153b3a1a3aea75ce7 Mon Sep 17 00:00:00 2001
From d4bf2ae67180f4f85b4498bdf49e3cf2a4af0767 Mon Sep 17 00:00:00 2001
From: Raine Makelainen <raine.makelainen@jolla.com>
Date: Tue, 21 Apr 2020 23:14:14 +0300
Subject: [PATCH 1/2] [gecko] Configuration option. JB#49613
Subject: [PATCH] [gecko] Avoid incorrect compiler optimisation. JB#49864

When building using gcc 8.3 and -O3, the null check on
IsInsideNursery() from js/public/HeapAPI.h gets dropped when used inside
the ProxyObject::New() method from js/src/vm/ProxyObject.cpp, called
from "priv.toGCThing()->isTenured()" on line 50 of the same file.

Since it's possible for the cell pointer to be null, the
over-optimisation can cause SIGSERV crashes.

SeaMonkey bug 1584533i details the same issue:
https://bugzilla.mozilla.org/show_bug.cgi?id=1584533

The fix adds the appropriate check back in directly to the
ProxyObject::New() code, which then doesn't get optimised out.

[gecko] Configuration option. JB#49613

All js and core configuration options should be
defined in this patch.
Expand All @@ -12,9 +28,11 @@ Also MacOS builds were reporting similar failures but those were fixed
differently.

Signed-off-by: Raine Makelainen <raine.makelainen@jolla.com>
Signed-off-by: David Llewellyn-Jones <david.llewellyn-jones@jolla.com>
---
js/src/old-configure.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
js/src/old-configure.in | 2 +-
js/src/vm/ProxyObject.cpp | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/js/src/old-configure.in b/js/src/old-configure.in
index 1c5c9e2145f6..3105458994a9 100644
Expand All @@ -29,6 +47,19 @@ index 1c5c9e2145f6..3105458994a9 100644
if test -z "$CLANG_CC"; then
MOZ_OPTIMIZE_FLAGS="-freorder-blocks $MOZ_OPTIMIZE_FLAGS"
fi
diff --git a/js/src/vm/ProxyObject.cpp b/js/src/vm/ProxyObject.cpp
index 49ed5a624129..46147062bf6c 100644
--- a/js/src/vm/ProxyObject.cpp
+++ b/js/src/vm/ProxyObject.cpp
@@ -47,7 +47,7 @@ ProxyObject::New(JSContext* cx, const BaseProxyHandler* handler, HandleValue pri
if (options.singleton()) {
MOZ_ASSERT(priv.isGCThing() && priv.toGCThing()->isTenured());
newKind = SingletonObject;
- } else if ((priv.isGCThing() && priv.toGCThing()->isTenured()) ||
+ } else if ((priv.isGCThing() && priv.toGCThing() && priv.toGCThing()->isTenured()) ||
!handler->canNurseryAllocate() ||
!handler->finalizeInBackground(priv))
{
--
2.25.2
2.17.1

0 comments on commit 1bb461b

Please sign in to comment.