Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiling Waterfox with clang causes extension conflict that crashes tabs #875

Open
laniakea64 opened this Issue Mar 3, 2019 · 36 comments

Comments

Projects
None yet
3 participants
@laniakea64
Copy link
Contributor

commented Mar 3, 2019

As reported on NoScript forum: https://forums.informaction.com/viewtopic.php?f=10&t=25441

Running Waterfox on Xubuntu 18.04 64-bit. Build machine is also Xubuntu 18.04 64-bit.

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 3, 2019

… clang 5.0.1)

Can a more recent version be used?

8 here, on FreeBSD, as far as I can tell:

Compiler /usr/local/bin/clang80 -std=gnu99 version 8.0.0 compiler flags:

-Qunused-arguments -Wall -Wempty-body -Wignored-qualifiers -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wclass-varargs -Wloop-analysis -Wstring-conversion -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat -Wno-gnu-zero-variadic-macro-arguments -Wformat-security -O2 -pipe -O3 -DLIBICONV_PLUG -fstack-protector -isystem /usr/local/include -fno-strict-aliasing -fno-strict-aliasing -ffunction-sections -fdata-sections -fno-math-errno -pipe -I/usr/local/include

Compiler /usr/local/bin/clang++80 -std=gnu++11 version 8.0.0 compiler flags:

-Qunused-arguments -DLIBICONV_PLUG -isystem /usr/local/include -Qunused-arguments -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wloop-analysis -Wc++11-compat-pedantic -Wc++14-compat -Wc++14-compat-pedantic -Wc++1z-compat -Wcomma -Wimplicit-fallthrough -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat -Wno-gnu-zero-variadic-macro-arguments -Wformat-security -Wno-unknown-warning-option -Wno-return-type-c-linkage -O2 -pipe -O3 -DLIBICONV_PLUG -fstack-protector -isystem /usr/local/include -fno-strict-aliasing -DLIBICONV_PLUG -isystem /usr/local/include -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pipe -I/usr/local/include -O2 -O3 -fomit-frame-pointer -DLIBICONV_PLUG -isystem /usr/local/include

@laniakea64

This comment has been minimized.

Copy link
Contributor Author

commented Mar 3, 2019

Can a more recent version be used?

Not sure. I don't think the standard Ubuntu repositories have anything newer. Trying to install clang 8 from https://apt.llvm.org/ now, will report back later.

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

Cool.

I should add, version 8 might be pushing the boat out. On FreeBSD it's a release candidate, not yet the default.

root@momh167-gjp4-8570p:~ # pkg query '%o %v %R' llvm60 llvm70 llvm80
devel/llvm60 6.0.1_6 FreeBSD
devel/llvm80 8.0.0.r2_1 poudriere
root@momh167-gjp4-8570p:~ # pkg remove llvm60
Checking integrity... done (0 conflicting)
Deinstallation has been requested for the following 1 packages (of 0 packages in the universe):

Installed packages to be REMOVED:
        llvm60-6.0.1_6

Number of packages to be removed: 1

The operation will free 717 MiB.

Proceed with deinstalling packages? [y/N]: y
[1/1] Deinstalling llvm60-6.0.1_6...
[1/1] Deleting files for llvm60-6.0.1_6: 100%
remove symlink for clang60
remove symlink for clang60 (world)
remove symlink for clang++60
remove symlink for clang++60 (world)
root@momh167-gjp4-8570p:~ # 

235215 – Bump LLVM_DEFAULT to 80

FreshPorts -- devel/llvm80

@hawkeye116477

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

Official tar package uses Clang 3.9.1. Version from my apt repository uses Clang 6 for Ubuntu and Clang 4 for Debian.
On Bionic newest Clang version is 7, Xenial - 6 and Trusty - 3.9.

@laniakea64

This comment has been minimized.

Copy link
Contributor Author

commented Mar 4, 2019

I was able to build with clang 8 (apparently this requires ac_add_options --disable-elf-hack.. not sure what that option does). Unfortunately this doesn't get rid of the crash.

@hawkeye116477

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

elfhack is a part of the Mozilla build system that optimizes relocations in compiled libraries
https://wiki.mozilla.org/Elfhack
It can improve performance, but rather difference will be small.

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

the crash.

For test purposes, can you provide a copy of the extension?

(I tried, failed to work with the code that's in the forum.)

@laniakea64

This comment has been minimized.

Copy link
Contributor Author

commented Mar 4, 2019

For test purposes, can you provide a copy of the extension?

Sure, here - https://gist.github.com/laniakea64/b41bd96974679aa17d813013b18703ba

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

Thanks.

No crash here, tested both single-process mode and (screenshot below) multi-process.

image

@laniakea64

This comment has been minimized.

Copy link
Contributor Author

commented Mar 4, 2019

Looks like the crash might not happen on all pages that register a ServiceWorker. I tested a Youtube video page and didn't see the crash.

I'll try to come up with a minimal example test page. edit: I assume that would be moot now since this issue can be reproduced consistently with the MDN ServiceWorker demo page.

@hawkeye116477

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

I tested on https://mdn.github.io/sw-test/ and on first time, tab crashed. Then tried again and was fine.

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

Still no crashing here (on FreeBSD-CURRENT):

image

image

I tried reloading repeatedly, with and without overriding the cache, closing then using the History menu to reopen, quitting then starting with the page, and so on.

I'll test in Manjaro in a VirtualBox guest.

@hawkeye116477 is NoScript in your test?

@laniakea64

This comment has been minimized.

Copy link
Contributor Author

commented Mar 4, 2019

on first time, tab crashed. Then tried again and was fine.

Same here on the local test page. Just unregistering the ServiceWorkers from about:serviceworkers, crash doesn't repeat. Closing the page that registered the ServiceWorker, and clearing all browsing history except Site Preferences, makes the crash reproducible again.

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

Build ID 20190202151534

Target x86_64-pc-linux-gnu

Compiler /usr/bin/ccache /usr/bin/clang-3.9 -std=gnu99 version 3.9.1 compiler flags:

-Qunused-arguments -Wall -Wempty-body -Wignored-qualifiers -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wclass-varargs -Wloop-analysis -Wstring-conversion -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat -Wno-gnu-zero-variadic-macro-arguments -Wformat-security -fno-strict-aliasing -ffunction-sections -fdata-sections -fno-math-errno -pthread -pipe

Compiler /usr/bin/ccache /usr/bin/clang++-3.9 -std=gnu++11 version 3.9.1 compiler flags:

-Qunused-arguments -Qunused-arguments -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wloop-analysis -Wc++11-compat-pedantic -Wc++14-compat -Wc++14-compat-pedantic -Wc++1z-compat -Wcomma -Wimplicit-fallthrough -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat -Wno-gnu-zero-variadic-macro-arguments -Wformat-security -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-exceptions -fno-strict-aliasing -static-libstdc++ -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -O3 -fomit-frame-pointer

I couldn't get a crash:

image

@hawkeye116477

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

@grahamperrin Yes, I tested again now a few times with cleared private date and no crash 😄.

@laniakea64

This comment has been minimized.

Copy link
Contributor Author

commented Mar 4, 2019

😕 I can consistently reproduce the crash using the MDN ServiceWorker demo. Same behavior as with the local test page.

@hawkeye116477 Are you testing on Xubuntu 18.04 (or some other flavor of Ubuntu 18.04)?

@hawkeye116477

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

@laniakea64 On Manjaro Linux, maybe I'll check tomorrow on Ubuntu.

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

With the same build ID 20190202151534 on Lubuntu, I got a crash in multi-process mode.

Not reproducible (touch wood) in single process mode then back to multi-process mode and again (touch wood) I can't reproduce the crash.

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2019

2019-03-04 23:05 screen recording.mp4

In the mix: Block Service Workers.

At one point in the recording I got crashing with NoScript disabled.

https://put.re/player?id=A2qVcrmW.mp4

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2019

Crash with the test extension, without NoScript, with Block Service Workers, on Manjaro:

image

@laniakea64 can you use an interactive notification instead of the modal?

@laniakea64

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2019

@grahamperrin Thanks for the suggestion of Block Service Workers. I too can reproduce the crash conflict in a new profile using that extension instead of NoScript:

  1. install Block Service Workers (I tested with the AMO version)

  2. install the test extension

  3. visit the MDN ServiceWorker demo page

  4. Click the button in the Block Service Workers notification to allow the ServiceWorker

can you use an interactive notification instead of the modal?

Sorry, I don't understand this question 😕

@laniakea64

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2019

Not reproducible (touch wood) in single process mode

I tested a new profile in official Waterfox 56.2.7.1 with e10s disabled, with Block Service Workers + the test extension + MDN ServiceWorker demo page. It took a couple reloads of the page to get the notification from Block Service Workers, but when I did get it and allowed the ServiceWorker, the whole browser crashed.

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2019

… allowed the ServiceWorker, …

That's what I mean by an interactive notification; for example, the notification that you clicked, immediately after blockage, to make an allowance.

@laniakea64

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2019

I'm sorry, I still don't understand your question. What interactive notification do you want me to see/use? Or are you asking me to modify the test extension?

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2019

Sorry,

modify the test extension?

Yes, I have a hunch that it'll be less prone to conflict if the modal can be avoided.

@laniakea64

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2019

@grahamperrin Ok, I've updated the gist.

I have a hunch that it'll be less prone to conflict if the modal can be avoided.

The use of alert() is immaterial to this issue.

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2019

Thanks.

With version 2 of the extension and KDE Plasma 5 (on FreeBSD-CURRENT):

  • touch wood, I can no longer produce a crash.

With mdn.github.io I sometimes/often find that a click on the Block Services Workers notification fails to change the block to an allowance. This was noticeable before the upgrade to version 2 of the test extension, I haven't found time to tell whether this failure is reproducible without the test extension.

With WhatsApp Web and version 2 of the extension on Lubuntu, I produced a crash just once, after clicking the Block Service Workers notification in an area that's unusual (to me). It seems that with Lubuntu, this notification sometimes appears at top right as a relatively large, dark, opaque rectangle with a distinctive white button that says 'Activate' or 'Allow' (something like that, I didn't grab a screenshot).

Can anyone produce a crash with version 2 of the test extension with Manjaro with KDE Plasma 5?

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2019

on Lubuntu, I produced a crash

Again with Lubuntu:

2019-03-06 00 34 24

– the crash followed a click, maybe a second click, on the Activate button, which I do not see in other desktop environments.

The Activate button also appeared for a notification for version 2 of the test extension. Is this intended, or is it a peculiarity of desktop environments such as the one in Lubuntu? I mean, if I see

A service worker is ready

– and if I refrain from clicking Activate, then by inference the service worker should remain dormant (ready but never active). In other words, does this (Lubuntu) style of notification have the potential to mislead end users?

@laniakea64

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

The Activate button also appeared for a notification for version 2 of the test extension. Is this intended,

No, I just don't know how to not have the button in notifications from WebExtensions. (Is it even possible?)
I think clicking that button shouldn't do anything.

I mean, if I see

A service worker is ready

– and if I refrain from clicking Activate, then by inference the service worker should remain dormant (ready but never active). In other words, does this (Lubuntu) style of notification have the potential to mislead end users?

I get your point and I would agree, but this minimal test extension is not intended for end users. It's only for demonstrating this issue. The actual code that prompted all this is part of an entirely different custom WebExtension (for personal use, not publicly available).

If the wording of the test extension notification is making it confusing for you to test this issue, I would be open to a suggestion for changing it.

Btw, is there some particular significance to using Manjaro here? Should I try to set up a Manjaro VM for testing Waterfox?

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2019

Well … a few months ago I had (might regain, through data recovery) Ubuntu as a VirtualBox guest. I was never a fan of the DE but I think of Ubuntu-based distros as mainstream and well-supported so there I was. A Linux (virtual) box for occasional test purposes.

More recently with an 'interim' notebook I decided to create a new VirtualBox guest, I chose Lubuntu because it would be relatively light. Until today I didn't know which DE I was using, from https://en.wikipedia.org/wiki/Lubuntu#Lubuntu_18.10 I assume that it's LXQt.

Most recently a couple of people strongly recommended Manjaro so I decided to create a second VirtualBox guest for occasional test purposes. https://www.manjaro.org/download/#downloads a range of distros, I chose KDE because it's my preferred DE on FreeBSD, avoided the developer preview.

@hawkeye116477 which DE do you use with Manjaro?

NB I'm not recommending any particular distro, just aiming to tell which ones are more (or less) prone to crashes of Waterfox with these combinations of extensions.

I had a hunch that KDE on Manjaro will be less prone to crashes than e.g. LXQt (on Lubuntu). Now I'll test that hunch for myself …

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2019

… a hunch that KDE on Manjaro will be less prone to crashes …

Ignore that, I got a crash.

Um no crash reporting, sadly.

This crash was at https://paulkinlan.github.io/videocut/ with Waterfox 56.2.7.1 in single-process mode:

[grahamperrin@momh167-gjp4-virtualbox-manjaro ~]$ date ; uname -a
Wed  6 Mar 04:53:27 GMT 2019
Linux momh167-gjp4-virtualbox-manjaro 4.19.24-1-MANJARO #1 SMP PREEMPT Wed Feb 20 22:59:23 UTC 2019 x86_64 GNU/Linux
[grahamperrin@momh167-gjp4-virtualbox-manjaro ~]$ which waterfox
/usr/bin/waterfox
[grahamperrin@momh167-gjp4-virtualbox-manjaro ~]$ waterfox
1551848019156   addons.webextension.{7f976419-2e7e-4ca5-983c-0fd0187e92ef}      WARN    Please specify whether you want browser_style or not in your page_action options.
1551848019178   addons.webextension.{8e13fae2-8de2-4aee-a942-fc41c911f85e}      WARN    Please specify whether you want browser_style or not in your browser_action options.

(waterfox:2516): GLib-GIO-WARNING **: 04:53:39.685: /etc/xdg/kde-mimeapps.list contains a [Added Associations] group, but it is not permitted here.  Only the non-desktop-specific mimeapps.list file may add or remove associations.
Segmentation fault (core dumped)
[grahamperrin@momh167-gjp4-virtualbox-manjaro ~]$ 

Where might I find the dump?

@hawkeye116477

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2019

@grahamperrin I use KDE.

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 9, 2019

Is this issue still thought to occur only where Waterfox is compiled with Clang?

I might have confused things a little (sorry) at #875 (comment):

With version 2 of the extension and KDE Plasma 5 (on FreeBSD-CURRENT):

  • touch wood, I can no longer produce a crash.

To the best of my recollection, I never produced a crash on FreeBSD-CURRENT.

@laniakea64

This comment has been minimized.

Copy link
Contributor Author

commented Mar 9, 2019

Is this issue still thought to occur only where Waterfox is compiled with Clang?

Yes. Waterfox compiled with gcc doesn't have this crash at all.

@laniakea64

This comment has been minimized.

Copy link
Contributor Author

commented Mar 16, 2019

521afd5 disabled ServiceWorkers by default. This issue is no longer reproducible without manually setting dom.serviceWorkers.enabled to true.

@grahamperrin

This comment has been minimized.

Copy link
Contributor

commented Mar 16, 2019

Interesting … but not entirely surprising. I yearn for useful crash data in situations such as this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.