1831242 - [rust 1.70] browser/extensions/formautofill/test/unit/heuristics/test_autocomplete_off_on_inputs.js | application crashed [@ core::sync::atomic::atomic_load]

Status: RESOLVED FIXED

(Firefox Build System :: Toolchains, defect)

Description

[Treeherder Bug Filer]

Filed by: mh [at] glandium.org
Parsed log: https://treeherder.mozilla.org/logviewer?job_id=414453240&repo=try
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/CwFPAHeFR52XxtqezROsew/runs/1/artifacts/public/logs/live_backing.log

---

[task 2023-05-03T07:55:39.856Z] 07:55:39  WARNING -  PROCESS-CRASH | browser/extensions/formautofill/test/unit/heuristics/test_autocomplete_off_on_inputs.js | application crashed [@ core::sync::atomic::atomic_load]
[task 2023-05-03T07:55:39.857Z] 07:55:39     INFO -  Crash dump filename: /tmp/xpc-other-l3lblkx8/1b57acb9-53c2-8f0f-2f13-bf0e02cd1450.dmp
[task 2023-05-03T07:55:39.857Z] 07:55:39     INFO -  Operating system: Linux
[task 2023-05-03T07:55:39.857Z] 07:55:39     INFO -                    4.4.0-1014-aws #14taskcluster1-Ubuntu SMP Tue Apr 3 10:27:00 UTC 2018
[task 2023-05-03T07:55:39.858Z] 07:55:39     INFO -  CPU: amd64
[task 2023-05-03T07:55:39.858Z] 07:55:39     INFO -       family 6 model 85 stepping 7
[task 2023-05-03T07:55:39.858Z] 07:55:39     INFO -       2 CPUs
[task 2023-05-03T07:55:39.858Z] 07:55:39     INFO -  Linux Ubuntu 18.04 - bionic (Ubuntu 18.04.6 LTS)
[task 2023-05-03T07:55:39.858Z] 07:55:39     INFO -  Crash reason:  SIGSEGV / SI_KERNEL
[task 2023-05-03T07:55:39.858Z] 07:55:39     INFO -  Crash address: 0xe5e5e5e5e5e5e5e5
[task 2023-05-03T07:55:39.859Z] 07:55:39     INFO -  Crashing instruction: `mov rax, qword [r14]`
[task 2023-05-03T07:55:39.863Z] 07:55:39     INFO -  Memory accessed by instruction:
[task 2023-05-03T07:55:39.863Z] 07:55:39     INFO -    0. Address: 0xe5e5e5e5e5e5e5e5
[task 2023-05-03T07:55:39.863Z] 07:55:39     INFO -       Size: 8
[task 2023-05-03T07:55:39.864Z] 07:55:39     INFO -  Process uptime: not available
[task 2023-05-03T07:55:39.864Z] 07:55:39     INFO -  Thread 0 xpcshell (crashed)
[task 2023-05-03T07:55:39.864Z] 07:55:39     INFO -   0  libxul.so!core::sync::atomic::atomic_load [atomic.rs:071f14baae931e17a5a99366bf216e76384cc4f6 : 3159]
[task 2023-05-03T07:55:39.865Z] 07:55:39     INFO -      Found by: inlining
[task 2023-05-03T07:55:39.865Z] 07:55:39     INFO -   1  libxul.so!core::sync::atomic::AtomicUsize::load [atomic.rs:071f14baae931e17a5a99366bf216e76384cc4f6 : 2264]
[task 2023-05-03T07:55:39.865Z] 07:55:39     INFO -      Found by: inlining
[task 2023-05-03T07:55:39.865Z] 07:55:39     INFO -   2  libxul.so!<servo_arc::Arc<T> as core::clone::Clone>::clone [lib.rs:57c3354635c006fa75357e3ca19a98c1b1087037 : 404]
[task 2023-05-03T07:55:39.866Z] 07:55:39     INFO -      Found by: inlining
[task 2023-05-03T07:55:39.866Z] 07:55:39     INFO -   3  libxul.so!core::option::Option<&T>::cloned [option.rs:071f14baae931e17a5a99366bf216e76384cc4f6 : 1931]
[task 2023-05-03T07:55:39.866Z] 07:55:39     INFO -      Found by: inlining
[task 2023-05-03T07:55:39.867Z] 07:55:39     INFO -   4  libxul.so!style::custom_properties::CustomPropertiesBuilder::build [custom_properties.rs:57c3354635c006fa75357e3ca19a98c1b1087037 : 773 + 0x8]
[task 2023-05-03T07:55:39.867Z] 07:55:39     INFO -       rax = 0x00007fd62df9d590    rdx = 0x0000000000000090
[task 2023-05-03T07:55:39.867Z] 07:55:39     INFO -       rcx = 0x000000004f0a0610    rbx = 0x00007fff23cc56d0
[task 2023-05-03T07:55:39.868Z] 07:55:39     INFO -       rsi = 0x00007fff23cc53f0    rdi = 0x00007fff23cc56d0
[task 2023-05-03T07:55:39.868Z] 07:55:39     INFO -       rbp = 0x00007fff23cc51d0    rsp = 0x00007fff23cc5060
[task 2023-05-03T07:55:39.868Z] 07:55:39     INFO -        r8 = 0x0000000000000000     r9 = 0x0000000000000000
[task 2023-05-03T07:55:39.868Z] 07:55:39     INFO -       r10 = 0x0000000000000000    r11 = 0x0000000000000000
[task 2023-05-03T07:55:39.868Z] 07:55:39     INFO -       r12 = 0x00007fd620788b98    r13 = 0x0000000000000000
[task 2023-05-03T07:55:39.869Z] 07:55:39     INFO -       r14 = 0xe5e5e5e5e5e5e5e5    r15 = 0x00007fff23cc5b00
[task 2023-05-03T07:55:39.869Z] 07:55:39     INFO -       rip = 0x00007fd64e4a453c
[task 2023-05-03T07:55:39.869Z] 07:55:39     INFO -      Found by: given as instruction pointer in context
[task 2023-05-03T07:55:39.869Z] 07:55:39     INFO -   5  libxul.so!style::properties::cascade::apply_declarations [cascade.rs:57c3354635c006fa75357e3ca19a98c1b1087037 : 291]
[task 2023-05-03T07:55:39.869Z] 07:55:39     INFO -      Found by: inlining
[task 2023-05-03T07:55:39.870Z] 07:55:39     INFO -   6  libxul.so!style::properties::cascade::cascade_rules [cascade.rs:57c3354635c006fa75357e3ca19a98c1b1087037 : 198 + 0x4df]
[task 2023-05-03T07:55:39.870Z] 07:55:39     INFO -       rbx = 0x0000000000000001    rbp = 0x00007fff23cc5a30
[task 2023-05-03T07:55:39.870Z] 07:55:39     INFO -       rsp = 0x00007fff23cc51e0    r12 = 0x00007fd620788b98
[task 2023-05-03T07:55:39.870Z] 07:55:39     INFO -       r13 = 0x0000000000000000    r14 = 0x00007fff23cc56d0
[task 2023-05-03T07:55:39.870Z] 07:55:39     INFO -       r15 = 0x00007fff23cc5b00    rip = 0x00007fd64dee4aa7
[task 2023-05-03T07:55:39.871Z] 07:55:39     INFO -      Found by: call frame info
[task 2023-05-03T07:55:39.871Z] 07:55:39     INFO -   7  libxul.so!style::properties::cascade::cascade [cascade.rs:57c3354635c006fa75357e3ca19a98c1b1087037 : 74]
[task 2023-05-03T07:55:39.871Z] 07:55:39     INFO -      Found by: inlining
[task 2023-05-03T07:55:39.871Z] 07:55:39     INFO -   8  libxul.so!style::stylist::Stylist::cascade_style_and_visited [stylist.rs:57c3354635c006fa75357e3ca19a98c1b1087037 : 1099 + 0x35]
[task 2023-05-03T07:55:39.871Z] 07:55:39     INFO -       rbx = 0x00007fff23cc5b30    rbp = 0x00007fff23cc5ac0
[task 2023-05-03T07:55:39.872Z] 07:55:39     INFO -       rsp = 0x00007fff23cc5a40    r12 = 0x00007fff23cc5b70
[task 2023-05-03T07:55:39.872Z] 07:55:39     INFO -       r13 = 0x0000000000000000    r14 = 0x0000000052340000
[task 2023-05-03T07:55:39.872Z] 07:55:39     INFO -       r15 = 0x0000000000000000    rip = 0x00007fd64dee43af
[task 2023-05-03T07:55:39.872Z] 07:55:39     INFO -      Found by: call frame info
[task 2023-05-03T07:55:39.873Z] 07:55:39     INFO -   9  libxul.so!style::stylist::Stylist::compute_pseudo_element_style_with_inputs [stylist.rs:57c3354635c006fa75357e3ca19a98c1b1087037 : 1036]
[task 2023-05-03T07:55:39.873Z] 07:55:39     INFO -      Found by: inlining
[task 2023-05-03T07:55:39.873Z] 07:55:39     INFO -  10  libxul.so!style::stylist::Stylist::precomputed_values_for_pseudo_with_rule_node [stylist.rs:57c3354635c006fa75357e3ca19a98c1b1087037 : 899]
[task 2023-05-03T07:55:39.874Z] 07:55:39     INFO -      Found by: inlining
[task 2023-05-03T07:55:39.874Z] 07:55:39     INFO -  11  libxul.so!Servo_ComputedValues_GetForAnonymousBox [glue.rs:57c3354635c006fa75357e3ca19a98c1b1087037 : 3998 + 0x4d]
[task 2023-05-03T07:55:39.875Z] 07:55:39     INFO -       rbx = 0x00007fd6207b9000    rbp = 0x00007fff23cc5ba0
[task 2023-05-03T07:55:39.875Z] 07:55:39     INFO -       rsp = 0x00007fff23cc5ad0    r12 = 0x00007fff23cc5b00
[task 2023-05-03T07:55:39.876Z] 07:55:39     INFO -       r13 = 0x00007fff23cc5d48    r14 = 0x0000000000000000
[task 2023-05-03T07:55:39.876Z] 07:55:39     INFO -       r15 = 0x00007fd6207b9040    rip = 0x00007fd64df62260
[task 2023-05-03T07:55:39.876Z] 07:55:39     INFO -      Found by: call frame info
[task 2023-05-03T07:55:39.877Z] 07:55:39     INFO -  12  libxul.so!nsCSSFrameConstructor::ConstructRootFrame() [nsCSSFrameConstructor.cpp:57c3354635c006fa75357e3ca19a98c1b1087037 : 2624 + 0xb]
[task 2023-05-03T07:55:39.877Z] 07:55:39     INFO -       rbx = 0x00007fd632ffea80    rbp = 0x00007fff23cc5be0
[task 2023-05-03T07:55:39.877Z] 07:55:39     INFO -       rsp = 0x00007fff23cc5bb0    r12 = 0x0000000000000006
[task 2023-05-03T07:55:39.877Z] 07:55:39     INFO -       r13 = 0x00007fff23cc5d48    r14 = 0x00007fd63b071040
[task 2023-05-03T07:55:39.878Z] 07:55:39     INFO -       r15 = 0x0000000000000009    rip = 0x00007fd64be6bdd8
[task 2023-05-03T07:55:39.878Z] 07:55:39     INFO -      Found by: call frame info

Comment 1

[Mike Hommey [:glandium]]

This is the return of bug 1798840, with rustc 1.70 having switched to LLVM 16. As per https://github.com/llvm/llvm-project/issues/58776#issuecomment-1532292071 this is likely a longstanding problem in some servo code (my bet is on servo_arc at the moment) that the LLVM change we backed out from clang unveiled. So per se, it's likely not a toolchains bug, but for now, I'm leaving it here until the actual culprit is identified.

Comment 2

[Mike Hommey [:glandium]]

I identified the culprit, and it's not pretty. Stylo has been relying on undefined behavior on its FFI forever, and LLVM changes now optimizes out things that happen after that, and all sorts of things break (duh).

See https://github.com/rust-lang/rust/issues/111229 for a summary and a reduced testcase.

In Firefox, the problematic code is https://searchfox.org/mozilla-central/rev/85b4f7363292b272eb9b606e00de2c37a6be73f0/servo/components/style/gecko_bindings/sugar/ownership.rs#121
The problem is that RawOffsetArc has interior mutability, but the input FFIType doesn't. The recent LLVM changes make it such that anything that happens with RawOffsetArc that would involve mutability can't happen.

One way to view the problem is that we shouldn't be using bindgen-generated types at the FFI boundary (using the rust types instead, and forward declarations on the Gecko side), and we wouldn't need to have these conversions in the first place? (but I must admit I haven't dived very deep in the related code)

Another way to view the problem is that bindgen should probably add an UnsafeCell (or a PhantomData<UnsafeCell>) in opaque types and forward declarations. I've tested that in a crude way with https://hg.mozilla.org/try/rev/6bb21dd9e7812d77fc26801014448cdbfe553d49 and it works (only dealt with forward declarations).

Even if we do the former, we should think about the latter, considering the broader rust ecosystem.

Emilio, thoughts?

Comment 3

[Mike Hommey [:glandium]]

BTW, bindgen generates types with Copy/Clone derives for these forward declarations, and it probably shouldn't.

Comment 4

[Mike Hommey [:glandium]]

All versions of Firefox will be affected when downstreams start using rustc 1.70 after it's released next month.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I filed https://github.com/rust-lang/rust-bindgen/issues/2521.

I can work on the proper fix on our side.

Comment 6

[Intermittent Failures Robot]

1 failures in 3411 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• try: 1

Platform and build breakdown:

• linux1804-64-qr: 1
  • opt: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1831242&startday=2023-05-01&endday=2023-05-07&tree=all

Comment 7

[Mike Hommey [:glandium]]

With bug 1831539 being fixed, let's reuse this bug to remove the patch we applied to LLVM to work around bug 1798840, which this was a variant of.

Comment 8

[Mike Hommey [:glandium]]

Attachment: Bug 1831242 - Remove revert-llvmorg-16-init-9324-g01859da84bad.patch.

Now that the underlying issue has been found and fixed on our side, we
don't need to workaround the issue at the compiler level anymore.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

I believe we at least need bug 1832065 and bug 1832173 too.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Those have patches now, so clearing ni?

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/21a00f73dd13
Remove revert-llvmorg-16-init-9324-g01859da84bad.patch. r=firefox-build-system-reviewers,sergesanspaille

Comment 12

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/21a00f73dd13

Comment 13

[Mike Hommey [:glandium]]

What do we do about all three dependencies of this bug for beta and esr? (ideally we should fix those)

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Is landing the bindgen hack from comment 2 possible? That might be easier than uplifting all the dependencies.

Comment 15

[Mike Hommey [:glandium]]

Attachment: Bug 1831242 - Patch bindgen to work around issues with some unsound transmutes when compiling with LLVM 16+.

Comment 16

[Mike Hommey [:glandium]]

Attachment: Bug 1831242 - Patch bindgen to work around issues with some unsound transmutes when compiling with LLVM 16+.

Comment 17

[Mike Hommey [:glandium]]

Attachment: Bug 1831242 - Patch bindgen to work around issues with some unsound transmutes when compiling with LLVM 16+.

Comment 18

[Mike Hommey [:glandium]]

Comment on attachment 9334218 [details]
Bug 1831242 - Patch bindgen to work around issues with some unsound transmutes when compiling with LLVM 16+.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Downstream builds with a rust compiler using LLVM 16, or with cross-language LTO with LLVM 16 are broken because of unsound rust code in Firefox.
• User impact if declined: Crashes in some downstream builds.
• Fix Landed on Version: N/A
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The approach here is much narrower than what landed in Nightly. Here we have a limited hack of bindgen to prevent the unsound rust code leading to bad optimizations by LLVM. Practically speaking, this only affects some annotations in the LLVM-IR, preventing them from appearing.

Comment 19

[Mike Hommey [:glandium]]

Comment on attachment 9334461 [details]
Bug 1831242 - Patch bindgen to work around issues with some unsound transmutes when compiling with LLVM 16+.

Beta/Release Uplift Approval Request

• User impact if declined: See esr approval request.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): See esr approval request.
• String changes made/needed:
• Is Android affected?: No

Comment 20

[David Lawrence [:dkl]]

Comment on attachment 9334218 [details]
Bug 1831242 - Patch bindgen to work around issues with some unsound transmutes when compiling with LLVM 16+.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined:
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):

Comment 21

[Mike Hommey [:glandium]]

Attachment: Bug 1831242 - Patch bindgen to work around issues with some unsound transmutes when compiling with LLVM 16+.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9334461 [details]
Bug 1831242 - Patch bindgen to work around issues with some unsound transmutes when compiling with LLVM 16+.

Bad bot.

Comment 23

[Donal Meehan [:dmeehan]]

Comment on attachment 9334461 [details]
Bug 1831242 - Patch bindgen to work around issues with some unsound transmutes when compiling with LLVM 16+.

Approved for 114.0b7.

Comment 24

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/424a213f8a13

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9334218 [details]
Bug 1831242 - Patch bindgen to work around issues with some unsound transmutes when compiling with LLVM 16+.

Approved for 102.12esr.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/7a293bbf964c

Comment 27

[Camelia Badau [:cbadau], Desktop QA]

Noticed in Comment 19, that Manual QE is needed for verifying this. Can you please help us with some steps, if that's the case?

Comment 28

[Mike Hommey [:glandium]]

Err, that was a mistake on my part. It should have been No.