search

OSSystems / meta-browser

OpenEmbedded/Yocto BSP layer for Web Browsers
MIT License
183 stars 189 forks source link

chromium: update to 117.0.5938.132 #757

Closed MaxIhlenfeldt closed 9 months ago

MaxIhlenfeldt commented 10 months ago

Release notes: https://chromereleases.googleblog.com/2023/09/stable-channel-update-for-desktop_12.html https://chromereleases.googleblog.com/2023/09/stable-channel-update-for-desktop_15.html https://chromereleases.googleblog.com/2023/09/stable-channel-update-for-desktop_21.html https://chromereleases.googleblog.com/2023/09/stable-channel-update-for-desktop_27.html

Build and patch changes:

Drop 0032-Backport-ozone-wayland-Fix-nullptr-deref-in-WaylandW.patch as it's included upstream now.

Drop 0036-Avoid-std-ranges-find_if.patch in favour of the added 0036-Backport-Remove-std-ranges-usage.patch.

Add 0039-Fix-implicitly-deleted-default-constructor-build-err.patch and 0040-Backport-IWYU-for-ui-events-gesture_detection-motion.patch to fix build errors with clang 14.

Add some more build error fixes to 0021-Add-missing-typename-s.patch, 0028-Avoid-capturing-structured-bindings.patch, 0034-Avoid-parenthesized-initialization-of-aggregates.patch

Rebase remaining patches.

License changes:

Added licenses:

Removed licenses:

Updated licenses:

Test-built (big thanks to @nrpt-m):

** Please note that Chromium requires below set-up when on dunfell branch.

*** Please note that there currently is a problem on RPi4/dunfell where Chromium crashes because of an illegal instruction error.

Signed-off-by: Max Ihlenfeldt max@igalia.com

MaxIhlenfeldt commented 10 months ago

@nrpt-m as always, testing would be greatly appreciated!

nrpt-m commented 10 months ago

@MaxIhlenfeldt @rakuco Have completed the builds & testing in master branch for all these below systems and chromium worked well in all these systems.

** Have built the rpi4 image successfully & @rwmacleod is testing it on rpi4 board, once done will update here.

Next, starting with kirkstone, dunfell & mickledore branches.

kraj commented 10 months ago

@MaxIhlenfeldt @rakuco Have completed the builds & testing in master branch for all these below systems and chromium worked well in all these systems.

  • chromium-ozone-wayland: qemux86-64
  • chromium-x11: qemux86-64, qemuarm, qemuarm64, raspberrypi4-64**

** Have built the rpi4 image successfully & @rwmacleod is testing it on rpi4 board, once done will update here.

Next, starting with kirkstone, dunfell & mickledore branches.

is musl build also part of your test matrix ?

rwmacleod commented 10 months ago

On my RPi4 with 4 GB ram, chromium-117 fails due to an illegal instruction when started as chromium or "chromium 1.1.1.1". I adjusted ulimit -c to allow core dumping but this image doesn't have gdb or gdb-server.

@MaxIhlenfeldt What do you suggest that @nrpt-m and I do to debug the issue?

FYI, this is the 6.1 kernel and glibc: $ uname -a: Linux raspberrypi4-64 6.1.54-v8 #1 SMP PREEMPT Wed Sep 20 14:13:53 UTC 2023 aarch64 GNU/Linux and there's nothing interesting on the chromium logs or in dmesg or /var/log/messages.

nrpt-m commented 10 months ago

is musl build also part of your test matrix ?

No.

nrpt-m commented 10 months ago

@MaxIhlenfeldt @rakuco Have completed the builds & testing in kirkstone branch for all these below systems and chromium worked well in all these systems.

Have built the rpi4 image successfully & @rwmacleod is testing it on rpi4 board, once done will update here. Build is failing for qemuarm machine with below errors:

../../v8/src/maglev/maglev-code-generator.cc:422:7: error: static_assert failed
../../v8/src/maglev/maglev-code-generator.cc:465:7: error: static_assert failed

Have added more error logs in attachment. chromium-117.0.5938.132-qemuarm-kirkstone-build-errors-log.txt

Next, will start with dunfell & mickledore branches.

nrpt-m commented 10 months ago

@MaxIhlenfeldt @rakuco Have completed the builds & testing in dunfell branch for all these below systems and chromium worked well in all these systems.

Have built the rpi4 image successfully & @rwmacleod is testing it on rpi4 board, once done will update here. Build is failing for qemuarm machine with same errors as in kirkstone branch:

../../v8/src/maglev/maglev-code-generator.cc:422:7: error: static_assert failed
../../v8/src/maglev/maglev-code-generator.cc:465:7: error: static_assert failed

Next, will start with mickledore branch.

MaxIhlenfeldt commented 10 months ago

On my RPi4 with 4 GB ram, chromium-117 fails due to an illegal instruction when started as chromium or "chromium 1.1.1.1". I adjusted ulimit -c to allow core dumping but this image doesn't have gdb or gdb-server.

@MaxIhlenfeldt What do you suggest that @nrpt-m and I do to debug the issue?

Hm, I'm not sure we can do much without the core dump, is there any way to get it?

MaxIhlenfeldt commented 10 months ago

Build is failing for qemuarm machine with below errors:

../../v8/src/maglev/maglev-code-generator.cc:422:7: error: static_assert failed
../../v8/src/maglev/maglev-code-generator.cc:465:7: error: static_assert failed

Have added more error logs in attachment. chromium-117.0.5938.132-qemuarm-kirkstone-build-errors-log.txt

Thanks, I'll take a look!

MaxIhlenfeldt commented 10 months ago

I think I know what's causing the compile error. Here's the relevant code:

static_assert(!DecompressIfNeeded || COMPRESS_POINTERS_BOOL);
// ...
if constexpr (DecompressIfNeeded) {
  static_assert(COMPRESS_POINTERS_BOOL);
  // ...
}

master uses clang 17, which seems to ignore the static_assert when it detects that DecompressIfNeeded == false (thanks to the if constexpr, I suppose), so there it compiles fine. kirkstone and dunfell use clang 14, which apparently still wants that assert to be satisfied, even if DecompressIfNeeded == false.

My suggestion would be to just comment out that static_assert - given that the code compiles fine with clang 17, we know that the logic checks out, even if clang 14 is unable to recognize it. Wdyt?

nrpt-m commented 10 months ago

My suggestion would be to just comment out that static_assert - given that the code compiles fine with clang 17, we know that the logic checks out, even if clang 14 is unable to recognize it. Wdyt?

May it could be fine to comment it for now but, not sure why the build has failed only for qemuarm machine and why not in building other machines ?

MaxIhlenfeldt commented 10 months ago

not sure why the build has failed only for qemuarm machine and why not in building other machines ?

I suppose it's because V8 pointer compression isn't supported on 32-bit ARM (i.e. this is the only platform where COMPRESS_POINTERS_BOOL == false).

nrpt-m commented 10 months ago

@MaxIhlenfeldt @rakuco Have completed the builds & testing in mickeldore branch for all these below systems and chromium worked well in all these systems.

Have built the rpi4 image successfully & @rwmacleod is testing it on rpi4 board, once done will update here. Build is failing for qemuarm machine with same errors as in earlier branches:

../../v8/src/maglev/maglev-code-generator.cc:422:7: error: static_assert failed
../../v8/src/maglev/maglev-code-generator.cc:465:7: error: static_assert failed

Could you please comment out that static_assert and add as patch in this PR so that, I can retest it for qemuarm machine in all the failed branches.

nrpt-m commented 10 months ago

@MaxIhlenfeldt , @rakuco

We have also observed that the chromium-117 version's tarball size is around 3GB but the earlier versions till chromium-116 version's tarball size was around 1.5GB only.

After extracting the tarball, we found that this chromium-117 has rust-src of around 4GB but, earlier versions didn't have it. Attached the screenshot as well. cc: @rwmacleod

Chromium-117
MaxIhlenfeldt commented 10 months ago

Could you please comment out that static_assert and add as patch in this PR so that, I can retest it for qemuarm machine in all the failed branches.

Done

MaxIhlenfeldt commented 10 months ago

After extracting the tarball, we found that this chromium-117 has rust-src of around 4GB but, earlier versions didn't have it.

Thanks for the heads up. I fear this is more or less expected, but I've asked on the Chromium Slack to be sure.

nrpt-m commented 10 months ago

Done

@MaxIhlenfeldt After adding the patching, have built successfully & chromium worked well with qemuarm machine in mickledore branch. Next is with kirkstone & dunfell branches.

nrpt-m commented 10 months ago

Next is with kirkstone & dunfell branches.

@MaxIhlenfeldt Have built successfully & chromium worked well with qemuarm machine in both the branches kirkstone & dunfell.

@rwmacleod Could you please update here the RPI4 test results for all the branches.

kraj commented 10 months ago

@MaxIhlenfeldt , @rakuco

We have also observed that the chromium-117 version's tarball size is around 3GB but the earlier versions till chromium-116 version's tarball size was around 1.5GB only.

After extracting the tarball, we found that this chromium-117 has rust-src of around 4GB but, earlier versions didn't have it. Attached the screenshot as well. cc: @rwmacleod

Chromium-117

Can you try a build with 

diff --git a/meta-chromium/recipes-browser/chromium/chromium-gn.inc b/meta-chromium/recipes-browser/chromium/chromium-gn.inc
index 7d076fe..d9b24c9 100644
--- a/meta-chromium/recipes-browser/chromium/chromium-gn.inc
+++ b/meta-chromium/recipes-browser/chromium/chromium-gn.inc
@@ -193,6 +193,8 @@ GN_ARGS += "is_debug=false is_official_build=true"
 # Use lld linker its quicker see https://lld.llvm.org/#performance
 GN_ARGS += "use_lld=true use_gold=false"

+# Do not use bundled rust it gets included into final package too
+GN_ARGS += "enable_rust=false"
 # By default, passing is_official_build=true to GN causes its symbol_level
 # variable to be set to "2". This means the compiler will be passed "-g2" and
 # we will end up with a very large chrome binary (around 5Gb as of M58)
kraj commented 10 months ago

@MaxIhlenfeldt can you rebase this pr on top of master please ?

MaxIhlenfeldt commented 10 months ago

Can you try a build with

Don't we already set enable_rust=false here?

Will rebase later today, do we want to look into the RPi4 crash @rwmacleod reported before merging?

nrpt-m commented 10 months ago

Can you try a build with

diff --git a/meta-chromium/recipes-browser/chromium/chromium-gn.inc b/meta-chromium/recipes-browser/chromium/chromium-gn.inc
index 7d076fe..d9b24c9 100644
--- a/meta-chromium/recipes-browser/chromium/chromium-gn.inc
+++ b/meta-chromium/recipes-browser/chromium/chromium-gn.inc
@@ -193,6 +193,8 @@ GN_ARGS += "is_debug=false is_official_build=true"
 # Use lld linker its quicker see https://lld.llvm.org/#performance
 GN_ARGS += "use_lld=true use_gold=false"

+# Do not use bundled rust it gets included into final package too
+GN_ARGS += "enable_rust=false"
 # By default, passing is_official_build=true to GN causes its symbol_level
 # variable to be set to "2". This means the compiler will be passed "-g2" and
 # we will end up with a very large chrome binary (around 5Gb as of M58)

It's already set in "chromium-gn.inc" file.

nrpt-m commented 10 months ago

Will rebase later today, do we want to look into the RPi4 crash @rwmacleod reported before merging?

yes. @rwmacleod has informed me about the RPI4 image test results as below:

In dunfell, For the illegal instruction error, we have changed meta browser to build a version of chromium-116 that is known to work before. Then if the error still happens, it's not chromium but meta-clang likely.

Have built the RPI4 images again with chromium-116 for dunfell/kirkstone branches and asked @rwmacleod to test again. May be by today evening, @rwmacleod will be able to provide the test results.

MaxIhlenfeldt commented 10 months ago

can you rebase this pr on top of master please ?

Done.

kraj commented 10 months ago

Will rebase later today, do we want to look into the RPi4 crash @rwmacleod reported before merging?

yes. @rwmacleod has informed me about the RPI4 image test results as below:

  • kirkstone: kernel starts but then system hangs with blank, black screen.
  • dunfell: boots, log in, set date, adduser foo; chromium 1.1.1.1 - illegal instruction.

In dunfell, For the illegal instruction error, we have changed meta browser to build a version of chromium-116 that is known to work before. Then if the error still happens, it's not chromium but meta-clang likely.

Have built the RPI4 images again with chromium-116 for dunfell/kirkstone branches and asked @rwmacleod to test again. May be by today evening, @rwmacleod will be able to provide the test results.

I am seeing illegal instruction issue with master/chromium-116/musl too. However it works on imx8 device which is also arm64, so I think the tune we use for rpi4 might be problem here. Are you using rpi4-64 ? The Raspberry Pi 4 does not have cryptographic extensions and my hunch is somewhere it is being enabled in some assembly code or some such in chromium or its 3rd party dependencies.

kraj commented 10 months ago

Here is backtrace of illegal instruction trap I am seeing

#0  0x00000055575a454c in aes_hw_set_encrypt_key ()
#1  0x000000555bb7ff84 in aes_ctr_set_key () at ../../third_party/boringssl/src/crypto/fipsmodule/cipher/e_aes.c:296
#2  CTR_DRBG_init () at ../../third_party/boringssl/src/crypto/fipsmodule/rand/ctrdrbg.c:77
#3  RAND_bytes_with_additional_data () at ../../third_party/boringssl/src/crypto/fipsmodule/rand/rand.c:389
#4  0x000000555bbb8218 in RAND_bytes () at ../../third_party/boringssl/src/crypto/fipsmodule/rand/rand.c:477
#5  CRYPTO_BUFFER_POOL_new () at ./../../third_party/boringssl/src/crypto/pool/pool.c:58
#6  0x000000555bedb3a4 in BufferPoolSingleton () at ./../../net/cert/x509_util.cc:86
#7  New () at ../../base/lazy_instance.h:70
#8  New () at ../../base/lazy_instance.h:119
#9  GetOrCreateLazyPointer<net::x509_util::(anonymous namespace)::BufferPoolSingleton> () at ../../base/lazy_instance_helpers.h:82
#10 Pointer () at ../../base/lazy_instance.h:159
#11 Get () at ../../base/lazy_instance.h:150
#12 GetBufferPool () at ./../../net/cert/x509_util.cc:385
#13 CreateCryptoBufferFromStaticDataUnsafe () at ./../../net/cert/x509_util.cc:404
#14 0x000000555c068d40 in TrustStoreChrome () at ./../../net/cert/internal/trust_store_chrome.cc:82
#15 0x000000555a2ea1ec in TrustStoreChrome () at ../../net/cert/internal/trust_store_chrome.cc:62
#16 make_unique<net::TrustStoreChrome> () at /usr/include/c++/v1/__memory/unique_ptr.h:689
#17 CreateNewCertVerifyProc () at ./../../services/cert_verifier/cert_verifier_creation.cc:136
#18 CreateCertVerifyProc () at ./../../services/cert_verifier/cert_verifier_creation.cc:88
#19 0x000000555a2e9e74 in CreateCertVerifier () at ./../../services/cert_verifier/cert_verifier_creation.cc:239
#20 0x000000555a2ebe5c in GetNewCertVerifierImpl () at ./../../services/cert_verifier/cert_verifier_service_factory.cc:61
#21 GetNewCertVerifier () at ./../../services/cert_verifier/cert_verifier_service_factory.cc:128
#22 0x0000005559801ea4 in Accept () at ./gen/services/cert_verifier/public/mojom/cert_verifier_service_factory.mojom.cc:1103
#23 0x000000555c4ca880 in HandleValidatedMessage () at ./../../mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:1016
#24 Accept () at ./../../mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:363
#25 0x000000555c4d1228 in Accept () at ./../../mojo/public/cpp/bindings/lib/message_dispatcher.cc:43
#26 0x000000555c4cc64c in HandleIncomingMessage () at ./../../mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:701
#27 0x000000555c4d4f80 in ProcessIncomingMessage () at ./../../mojo/public/cpp/bindings/lib/multiplex_router.cc:1096
#28 0x000000555c4d46c4 in Accept () at ./../../mojo/public/cpp/bindings/lib/multiplex_router.cc:710
#29 0x000000555c4d1228 in Accept () at ./../../mojo/public/cpp/bindings/lib/message_dispatcher.cc:43
#30 0x000000555c4c8740 in DispatchMessage () at ./../../mojo/public/cpp/bindings/lib/connector.cc:561
#31 0x000000555c4c8f10 in ReadAllAvailableMessages () at ./../../mojo/public/cpp/bindings/lib/connector.cc:618
#32 0x000000555baaec2c in Run () at ../../base/functional/callback.h:152
#33 RunTaskImpl () at ./../../base/task/common/task_annotator.cc:201
#34 0x000000555baca944 in RunTask<(lambda at ../../base/task/sequence_manager/thread_controller_with_message_pump_impl.cc:482:11)> () at ../../base/task/common/task_annotator.h:89
#35 DoWorkImpl () at ./../../base/task/sequence_manager/thread_controller_with_message_pump_impl.cc:480
#36 DoWork () at ./../../base/task/sequence_manager/thread_controller_with_message_pump_impl.cc:345
#37 0x000000555bb281c0 in HandleDispatch () at ../../base/allocator/partition_allocator/pointers/raw_ptr.h:816
#38 WorkSourceDispatch () at ./../../base/message_loop/message_pump_glib.cc:274
#39 0x0000007ff7dfdc80 in g_main_dispatch (context=0x7ff6f1d220) at ../glib-2.78.0/glib/gmain.c:3476
#40 g_main_context_dispatch_unlocked (context=context@entry=0x7ff6f1d220) at ../glib-2.78.0/glib/gmain.c:4284
#41 0x0000007ff7dfe07c in g_main_context_iterate_unlocked (context=context@entry=0x7ff6f1d220, block=block@entry=0, dispatch=dispatch@entry=1, self=<optimized out>)
    at ../glib-2.78.0/glib/gmain.c:4349
#42 0x0000007ff7dfe168 in g_main_context_iteration (context=0x7ff6f1d220, may_block=0) at ../glib-2.78.0/glib/gmain.c:4414
#43 0x000000555bb27094 in Run () at ./../../base/message_loop/message_pump_glib.cc:680
#44 0x000000555bacb280 in Run () at ./../../base/task/sequence_manager/thread_controller_with_message_pump_impl.cc:645
#45 0x000000555ba93168 in Run () at ./../../base/run_loop.cc:133
#46 0x00000055599636f8 in RunMainMessageLoop () at ./../../content/browser/browser_main_loop.cc:1070
#47 0x000000555995fee8 in Run () at ../../content/browser/browser_main_runner_impl.cc:158
#48 BrowserMain () at ./../../content/browser/browser_main.cc:34
#49 0x000000555ae1e358 in RunBrowserProcessMain () at ./../../content/app/content_main_runner_impl.cc:685
#50 RunBrowser () at ./../../content/app/content_main_runner_impl.cc:1266
#51 0x000000555ae1df14 in Run () at ./../../content/app/content_main_runner_impl.cc:1116
#52 0x000000555ae1afe8 in RunContentProcess () at ./../../content/app/content_main.cc:326
#53 ContentMain () at ./../../content/app/content_main.cc:343
#54 0x00000055577ee1d0 in ChromeMain () at ./../../chrome/app/chrome_main.cc:187
#55 0x0000007ff7f7779c in libc_start_main_stage2 (main=0x55577edf20 <main>, argc=4, argv=0x7ffffffc28) at /usr/src/debug/musl/1.2.4+git-r0/src/env/__libc_start_main.c:95
#56 0x0000000000000000 in ?? ()
nrpt-m commented 10 months ago

I am seeing illegal instruction issue with master/chromium-116/musl too. However it works on imx8 device which is also arm64, so I think the tune we use for rpi4 might be problem here. Are you using rpi4-64 ? The Raspberry Pi 4 does not have cryptographic extensions and my hunch is somewhere it is being enabled in some assembly code or some such in chromium or its 3rd party dependencies.

yes, @rwmacleod is using rpi4-64 for testing.

kraj commented 10 months ago

I am seeing illegal instruction issue with master/chromium-116/musl too. However it works on imx8 device which is also arm64, so I think the tune we use for rpi4 might be problem here. Are you using rpi4-64 ? The Raspberry Pi 4 does not have cryptographic extensions and my hunch is somewhere it is being enabled in some assembly code or some such in chromium or its 3rd party dependencies.

yes, @rwmacleod is using rpi4-64 for testing.

OK. Please test this patch

https://github.com/YoeDistro/meta-browser/commit/05c62357d58a8a4868108fa8cff65558f2d45165

on top of this pull and report back pi4 results

nrpt-m commented 10 months ago

OK. Please test this patch

YoeDistro@05c6235

on top of this pull and report back pi4 results

Have built the dunfell rpi4 image on top of this pull and @rwmacleod tested it on rpi4 board. We are still getting illegal instruction error and chromium terminates.

kraj commented 10 months ago

OK. Please test this patch YoeDistro@05c6235 on top of this pull and report back pi4 results

Have built the dunfell rpi4 image on top of this pull and @rwmacleod tested it on rpi4 board. We are still getting illegal instruction error and chromium terminates.

OK. Atleast this has fixed my problem seen with master branches. So the patch is still good. Perhaps dunfell issue is different. Can you post stack trace?

nrpt-m commented 10 months ago

OK. Atleast this has fixed my problem seen with master branches. So the patch is still good. Perhaps dunfell issue is different. Can you post stack trace?

Sorry, It was tested by @rwmacleod and he is quite busy. So, we couldn't share the stack trace logs. I am working on getting one RPI4 board setup.

kraj commented 9 months ago

FYI, I've just merged #766 to fix the M116 build on arm64 without ASM crypto operations, but the change needs to be reverted for M117 (the original version of the patch targeted M117).

right, I have forward ported patch for it also staged in yoe/mut. I have also rebased the patches here on top of latest master here https://github.com/OSSystems/meta-browser/pull/767

rakuco commented 9 months ago

Thanks, @kraj. This PR is open to edits from repo maintainers, so I've rebased Max's branch and added a git revert of #766. I think it should match your changes in #767.

kraj commented 9 months ago

revert is fine too. lgtm

MaxIhlenfeldt commented 9 months ago

Updated the commit message and PR description to include all the test results, and merged. Sorry for the delay, and thanks for handling the m116/m117 ARM fix situation!