PSA: Avoid invoking Debug formatters in release-mode Rust

TL;DR: To prevent code bloat, avoid {:?} in format strings for panic!(),
unreachable!(), error!(), warn!(), and info!() for Rust code that ships in
Gecko.

Longer version:

One nice thing about Rust is that you can #[derive(Debug)] for a type, and
the compiler will generate a stringification method, which may in turn be
inductively defined in terms of the Debug implementations of member types.
This is great for logging and debugging, but the implementations can be
quite large depending on the types involved.

The linker will generally eliminate unused Debug implementations as dead
code, but can't do so if they might be invoked in release builds. The most
common way this seems to happen is in panic!() messages, where it can be
tempting to include a stringified value to make the message more
informative. It can also happen for the logging macros that don't get
compiled out of release builds, which (at least for stylo) are info!(),
warn!(), and error!() [1].

To demonstrate what's at stake here, this trivial patch eliminates more
than 80K from libxul: https://github.com/servo/servo/pull/19756

Given how easy it is to mess this up and pull tons of unnecessary code into
Firefox, and given that it's rather time-consuming to notice the problem
and track down the culprit, I think we're best off categorically avoiding
this pattern.

Comments and alternative proposals welcome.

bholley


[1]
https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/servo/ports/geckolib/Cargo.toml#21
0
Bobby
1/13/2018 5:07:21 AM
mozilla.dev.platform 6467 articles. 0 followers. Post Follow

1 Replies
54 Views

Similar Articles

[PageSpeed] 39

On 2018-01-12 9:07 PM, Bobby Holley wrote:
> The most
> common way this seems to happen is in panic!() messages, where it can be
> tempting to include a stringified value to make the message more
> informative.

Just a friendly reminder: panic messages that are parameterized to 
include debug data might expose PII in Firefox crash reports. Patches 
that add new parameterized panic messages should probably be reviewed by 
a data steward:

https://wiki.mozilla.org/Firefox/Data_Collection
0
Chris
1/16/2018 8:08:57 AM
Reply: