third_party/rust/libc is too old, out of sync with rustc

RESOLVED FIXED in Firefox 55

Status

()

Core
General
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: Jan Beich, Assigned: Jan Beich)

Tracking

Trunk
mozilla56
ARM
FreeBSD
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox54 wontfix, firefox55 fixed, firefox56 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

5 months ago
Despite bug 1374807 bumping Rust version the bundled libc crate hasn't been updated since bug 1332465. This severly limits support for new platforms, especially Tier3 ones. For one, FreeBSD aarch64 support appeared in Rust 1.17.0.

gmake[6]: Entering directory '/wrkdirs/usr/ports/www/firefox/work/firefox-54.0/obj-aarch64-unknown-freebsd11.0/toolkit/library/rust'
force-cargo-library-build
env  CARGO_TARGET_DIR=/wrkdirs/usr/ports/www/firefox/work/firefox-54.0/obj-aarch64-unknown-freebsd11.0/toolkit/library RUSTC=/usr/local/bin/rustc MOZ_DIST=/wrkdirs/usr/ports/www/firefox/work/firefox-54.0/obj-aarch64-unknown-freebsd11.0/dist LIBCLANG_PATH= CLANG_PATH= PKG_CONFIG_ALLOW_CROSS=1 /usr/local/bin/cargo build  --release --frozen --manifest-path /wrkdirs/usr/ports/www/firefox/work/firefox-54.0/toolkit/library/rust/Cargo.toml --lib --target=aarch64-unknown-freebsd 
   Compiling libc v0.2.20
   Compiling byteorder v1.0.0
   Compiling matches v0.1.4
   Compiling nsstring v0.1.0 (file:///wrkdirs/usr/ports/www/firefox/work/firefox-54.0/xpcom/rust/nsstring)
   Compiling bitreader v0.2.0
   Compiling unicode-normalization v0.1.4
   Compiling unicode-bidi v0.2.5
error[E0412]: cannot find type `c_long` in this scope
   --> /wrkdirs/usr/ports/www/firefox/work/firefox-54.0/third_party/rust/libc/src/lib.rs:192:45
    |
192 |     pub fn fseek(stream: *mut FILE, offset: c_long, whence: c_int) -> c_int;
    |                                             ^^^^^^ not found in this scope
    |
    = help: possible candidate is found in another module, you can import it into scope:
              `use core::os::raw::c_long;`
[...]

http://thunderx1.nyi.freebsd.org/data/110arm64-default/444205/logs/errors/firefox-54.0,1.log
https://github.com/rust-lang/libc/commit/1363fe4a03b7
Comment hidden (mozreview-request)
(Assignee)

Updated

5 months ago
status-firefox54: affected → wontfix
(Assignee)

Comment 2

5 months ago
Comment on attachment 8882152 [details]
Bug 1376411 - Update libc to 0.2.24 to get support for more Tier3 platforms.

Generated by

$ fgrep -m1 libc **/Cargo.lock
js/src/Cargo.lock: "libc 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)",
servo/Cargo.lock: "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)",
testing/geckodriver/Cargo.lock: "libc 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)",
toolkit/library/gtest/rust/Cargo.lock: "libc 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)",
toolkit/library/rust/Cargo.lock: "libc 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)",

$ for f (**/Cargo.lock~servo/*) (cd $f:h; cargo update -p libc)
$ ./mach vendor rust
(Assignee)

Comment 3

5 months ago
Created attachment 8882157 [details] [diff] [review]
Rebased for Firefox 55

$ hg up beta
$ fetch -qo- https://reviewboard-hg.mozilla.org/gecko/raw-rev/a3e3810cb479 | sed 1d | patch -CEfsp1
6 out of 18 hunks failed while patching toolkit/library/gtest/rust/Cargo.lock
6 out of 18 hunks failed while patching toolkit/library/rust/Cargo.lock

Comment 4

5 months ago
mozreview-review
Comment on attachment 8882152 [details]
Bug 1376411 - Update libc to 0.2.24 to get support for more Tier3 platforms.

https://reviewboard.mozilla.org/r/153272/#review158524

Looks correct, and should be compatible with our current builds. Thanks for the update!
Attachment #8882152 - Flags: review?(giles) → review+

Comment 5

5 months ago
Pushed by rgiles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e8d9d1520395
Update libc to 0.2.24 to get support for more Tier3 platforms. r=rillian

Comment 6

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e8d9d1520395
Status: NEW → RESOLVED
Last Resolved: 5 months ago
status-firefox56: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Assignee: nobody → jbeich
status-firefox55: affected → fix-optional
(Assignee)

Comment 7

5 months ago
Comment on attachment 8882157 [details] [diff] [review]
Rebased for Firefox 55

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1284816
[User impact if declined]: Broken build at least on FreeBSD aarch64
[Is this code covered by automated tests?]: Yes but I'm not sure exactly which ones.
[Has the fix been verified in Nightly?]: No, I don't have hardware. Only FF54 backport was verified downstream:
http://thunderx1.nyi.freebsd.org/data/110arm64-default/444857/logs/firefox-54.0.1_1,1.log
https://svnweb.freebsd.org/ports/head/www/firefox/files/patch-bug1376411?revision=444651&view=markup
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Unlikely
[Why is the change risky/not risky?]: Firefox 55 uses Rust only for MP4 parser and bug 1324243, so only those components have a chance to crash if libc bindings have regressed in a backward-incompatible way.
[String changes made/needed]: None
Attachment #8882157 - Flags: approval-mozilla-beta?
(Assignee)

Comment 8

5 months ago
Comment on attachment 8882157 [details] [diff] [review]
Rebased for Firefox 55

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e84f1e012f4
(In reply to Jan Beich from comment #7)

> [Has the fix been verified in Nightly?]: No, I don't have hardware.

The update has been validated on Nightly for Tier-1 platforms, it's just FreeBSD where we don't have nightly testing.

> [Why is the change risky/not risky?]: Firefox 55 uses Rust only for MP4 parser and bug 1324243, so only those components have a chance to crash if libc bindings have regressed in a backward-incompatible way.

Also, the MP4 parser doesn't use the libc crate. This is needed for servo components like webrender and stylo which are pref'd off for 55 release, and the geckodriver testing tool. So long as it builds, the patch is low risk because it doesn't affect shipping code.
Comment on attachment 8882157 [details] [diff] [review]
Rebased for Firefox 55

From comments 7 and 9, sounds like this will be useful for webrender and stylo and isn't high risk. Thanks for rebasing for 55 - let's try this for beta 9.
Attachment #8882157 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Eh, this is very much not about webrender and stylo on 55, it's about a tier3 platform.  Shouldn't hurt though...

Comment 12

5 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/f237f91d8c06
status-firefox55: fix-optional → fixed
(In reply to Jan Beich from comment #7)
> [Is this code covered by automated tests?]: Yes but I'm not sure exactly
> which ones.
> [Has the fix been verified in Nightly?]: No, I don't have hardware. Only
> FF54 backport was verified downstream:
> http://thunderx1.nyi.freebsd.org/data/110arm64-default/444857/logs/firefox-
> 54.0.1_1,1.log
> https://svnweb.freebsd.org/ports/head/www/firefox/files/patch-
> bug1376411?revision=444651&view=markup
> [Needs manual test from QE? If yes, steps to reproduce]: No

Setting qe-verify- based on Jan's assessment on manual testing needs.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.