Opened 2 years ago

Closed 2 years ago

#22909 closed defect (invalid)

Our Rust code is always built in debug mode

Reported by: isis Owned by:
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: rust, rust-pilot, tor-build
Cc: chelseakomlo Actual Points:
Parent ID: Points: .1
Reviewer: Sponsor: SponsorZ

Description

In src/rust/Cargo.toml:

[profile.release]
debug = true

This enables certain things, e.g. debug_assert!(false = true) will panic. It also does stuff like bounds checks for every access and integer overflow/underflow checks every time any number is used. These are great things to do, but they are usually done in debug builds which are used in testing, not in release. This will make our code literally hundreds of times slower, so we should probably remove this.

Child Tickets

Change History (11)

comment:1 Changed 2 years ago by infinity0

https://doc.crates.io/manifest.html#the-profile-sections

debug = true is for generating debuginfo. asserts are something else, debug-assertions.

comment:2 Changed 2 years ago by Sebastian

Resolution: invalid
Status: newclosed

comment:3 Changed 2 years ago by Sebastian

Reason this is invalid: We're matching Tor's C code behaviour here, which always includes debug symbols, even in releases.

comment:4 in reply to:  3 Changed 2 years ago by isis

Resolution: invalid
Status: closedreopened

Replying to Sebastian:

Reason this is invalid: We're matching Tor's C code behaviour here, which always includes debug symbols, even in releases.


That makes sense to try to match the C behaviour. Although, I think building Rust code in debug mode isn't really comparable to a C binary with debugging symbols… there's extra stuff going on there (like the over/under-flow checks), and it's not like we can just run strip on it to make it what it would've been without debugging enabled. It's more like building a C binary with UBSan and whatever other sanitisers enabled, which IIRC we don't do (--enable-expensive-hardening is off by default).

comment:5 Changed 2 years ago by chelseakomlo

On http://doc.crates.io/guide.html:

"Compiling in debug mode is the default for development-- compilation time is shorter since the compiler doesn't do optimizations, but the code will run slower. Release mode takes longer to compile, but the code will run faster."

comment:6 Changed 2 years ago by chelseakomlo

Cc: chelseakomlo added

comment:7 Changed 2 years ago by nickm

Milestone: Tor: 0.3.1.x-finalTor: 0.3.2.x-final

Defer these rust tickets to 0.3.2

comment:8 Changed 2 years ago by infinity0

I should have been explicit earlier but this ticket really should be closed, it is invalid.

debug mode is [profile.debug], i.e. what happens when you leave out --release when invoking cargo.

debug = true is not "debug mode", it is to tell rustc to add DWARF2 debuginfo to the ELF binaries and is equivalent to -g flags to C compilers.

comment:9 Changed 2 years ago by infinity0

Whoops, that should have been [profile.dev] not [profile.debug], see the first link I posted. The rest of the comment is correct.

comment:10 Changed 2 years ago by nickm

Milestone: Tor: 0.3.2.x-finalTor: 0.3.3.x-final

comment:11 in reply to:  8 Changed 2 years ago by isis

Resolution: invalid
Status: reopenedclosed

Replying to infinity0:

I should have been explicit earlier but this ticket really should be closed, it is invalid.

debug mode is [profile.debug], i.e. what happens when you leave out --release when invoking cargo.

debug = true is not "debug mode", it is to tell rustc to add DWARF2 debuginfo to the ELF binaries and is equivalent to -g flags to C compilers.


Ah, I'm sorry, I understand what you meant now. -g just means it's passing -C debuginfo=2. Closing as invalid.

Note: See TracTickets for help on using tickets.