1600521 - 2.49.5 built with gcc > 5 segfaults while executing adsbygoogle.js

Status: RESOLVED WONTFIX

(SeaMonkey :: General, defect)

Description

[Michael Shigorin]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0

Steps to reproduce:

Open http://www.opennet.ru/opennews/art.shtml?num=51826 (or a local reproducer attached).

Actual results:

Browser segfaults.

Expected results:

Page renders.

Assigning to "General" as I see no JS engine related component in the list anymore (who removed that?).

Comment 1

[Michael Shigorin]

Attachment: adsbygoogle.js provoking segfault (and a testpage) in archive

Our power user who reported this has attached these artifacts to ALT bug 37449 (in Russian):

• a local reproducer;
• backtrace;
• patch set, including the actual js/ oneliner.

Comment 2

[Michael Shigorin]

Attachment: backtrace

Comment 3

[Michael Shigorin]

Attachment: js-ProxyObject-New-null-pointer.patch

The proposed patch.

Comment 4

[Michael Shigorin]

Attachment: 2.49.4-2.49.5-updates.no-crmf.fix-segfault.tar.gz

The complete patch set with a few unrelated but hopefully useful fixes including gtk2 one (related to bug 1461307) and libcrmf one (linking against that isn't needed --with-system-nss, bug 1371991).

My translation of the original patch description:

---

The error occurs during passing in ProxyObject::New() JS::NullHandleValue
(defined in mozilla/js/src/vm/Value.cpp:16; no errors seen during computing JS::NullHandleValue itself).

js::CloneObject(), mozilla/js/src/jsobj.cpp:1136:

clone = ProxyObject::New(cx, GetProxyHandler(obj), JS::NullHandleValue,
proto, options); // ---> ...

... ---> ProxyObject::New(), mozilla/js/src/vm/ProxyObject.cpp:50:
...
} else if ((priv.isGCThing() && priv.toGCThing()->isTenured()) ||
!handler->canNurseryAllocate() ||
!handler->finalizeInBackground(priv)) { ...

      ^^^^^^^^^^
     priv.toGCThing() for priv = JS::NullHandleValue equals 0 => segfault at

priv.toGCThing()->isTenured().

If the code is built with gcc 4.8 or 4.9, priv.toGCThing()->isTenured() is not computed, maybe due to the expression within else if ( ... ) brackets getting computed in another order:
in this case both handler->canNurseryAllocate() and
handler->finalizeInBackground(priv) == 0,
so it's enough to compute just one of them.

At any rate, the program built with gcc >= 7 gets down to computing
priv.toGCThing()->isTenured() => segfault.

Fixed by replacing priv.toGCThing()->isTenured() with (priv.toGCThing() &&
priv.toGCThing()->isTenured()), like in the patch attached.

Ask upstream for the regular solution.
The segfault might point at the error in initializing JS::NullHandleValue
which didn't turn up before.

Patch author: Oleg Zenin <[e-mail]>
https://bugzilla.altlinux.org/show_bug.cgi?id=37449

Comment 5

[Frank-Rainer Grahl (:frg)]

Thanks. Does this occur with the official release? If not which compiler? GCC > 5 is basically broken in 52 and not supported. The upcoming 2.53.1 will support gcc 7 and 8 plus clang > 5

Comment 6

[zenin_o]

(In reply to Frank-Rainer Grahl (:frg) from comment #5)

Does this occur with the official release?

No, as far as the official linux-x86_64 release is built with gcc-4.8.5.
As mentioned above, the error didn't show up either in my private builds with gcc-4.8./4.9.

If not which compiler?

The error was reproduced with gcc-7.X/8.X. Unfortunately, I haven't got a chance to test gcc-5/6.

GCC > 5 is basically broken in 52 and not supported.

My empirical observation is that 2.49.5 successfully compiles on linux-x86_64 with gcc-8.3.1 and runs w/o visible errors given a null pointer check from the comment #3 (leaving aside multiple warnings and notes thrown by the compiler with a reasonable set of -W* options; at glance, gcc-4.X complained less)

The upcoming 2.53.1 will support gcc 7 and 8 plus clang > 5

My point is that this particular compiler-specific error in js::ProxyObject::New() may indicate a deeper problem worth being investigated.

Comment 7

[Frank-Rainer Grahl (:frg)]

Sorry for the late reply was a bit busy with release work.

My empirical observation is that 2.49.5 successfully compiles on linux-x86_64 with gcc-8.3.1 and runs w/o visible errors given a null pointer check

I and others exprienced redraw problems and the occasional crash in different places using gcc 7 and up.

My point is that this particular compiler-specific error in js::ProxyObject::New() may indicate a deeper problem worth being investigated.
Best would be to move this over to SpiderMonkey then. But I think it was fixed in Bug 1325075. 2.53.1 does not show it when compiled with gcc 8.3.1 and it still has the same code sequence.

I looked at the tar.gz for a possible port to 2.53.1
IME-changeset-84d7324fb118.rollback.patch: gtk2 support is removed in later releases. So does not apply.
js-ProxyObject-New-null-pointer.patch: If we do a new 2.49 we will look at either including this one or backporting bug 1325075. Very unlikely because 2.53.1 is around the corner. Tested with gcc 8 and works fine.
seamonkey-2.2-alt-machOS-fix.patch: intl.locale.matchOS is dead in 2.57 but need to look into it for 2.53.
xulrunner-noarch-extensions.patch: Is this sitll needed? Binary extensions are dead. Any other use?

Comment 8

[zenin_o]

(In reply to Frank-Rainer Grahl (:frg) from comment #7)

Thanks and sorry for delaying the reply in turn.

I and others exprienced redraw problems and the occasional crash in different places using gcc 7 and up.

Ok, as far as glitches do not occur in gcc-4 official builds of 2.49.5, I can't insist on generalizing my gcc-8/9 experience with 2.49.5 to any extent, being quite happy that my private gcc-8 build works for me.

My point is that this particular compiler-specific error in js::ProxyObject::New() may indicate a deeper problem worth being investigated.
,... But I think it was fixed in Bug 1325075.

Yes, I think it was.

I looked at the tar.gz for a possible port to 2.53.1
IME-changeset-84d7324fb118.rollback.patch: gtk2 support is removed in later releases. So does not apply.

Perfectly fine with me.

js-ProxyObject-New-null-pointer.patch: If we do a new 2.49 we will look at either including this one or backporting bug 1325075. Very unlikely because 2.53.1 is around the corner. Tested with gcc 8 and works fine.

Just out of curiosity, what's the schedule for 2.53.1 official release?

seamonkey-2.2-alt-machOS-fix.patch: intl.locale.matchOS is dead in 2.57 but need to look into it for 2.53.

Thanks.

xulrunner-noarch-extensions.patch: Is this sitll needed? Binary extensions are dead. Any other use?

To my shame, I can't comment on binary extensions as I never used them myself.
I guess this patch is distribution specific and was around since ~2.17, so it can be ignored for sure.

Given the above, I think none of the reported problems can be considered critical in view of forthcoming 2.53.1 release.
Please feel free to close the bug.

Regards,
Oleg.

Comment 9

[zenin_o]

P.S. It's worth changing the title to less misleading ``2.49.5 built with gcc > 5 segfaults while executing adsbygoogle.js'', but apparently I have no permissions to edit the bug description.
Thanks!

Comment 10

[Frank-Rainer Grahl (:frg)]

Just out of curiosity, what's the schedule for 2.53.1 official release?
We are finalizing l10n right now. Hope it to get a beta 1 out before 12/24. I am not sure if we make this date but in any case as fast as possible.
Beta 1 because it contains so much new stuff. For me it is already production ready but not everyone is not doing everything the same way or uses the same functionality. So there might be bugs but all devs are on it.