servo / app_units

https://crates.io/crates/app_units
3 stars 17 forks source link

Au::scale_by can overflow #25

Closed Manishearth closed 7 years ago

Manishearth commented 7 years ago

We just round and cast. Casting can overflow.

Gecko seems to clamp at +/- 0x40000000 app units (NSToCoordRoundWithClamp). We should do the same.

See also: https://github.com/servo/app_units/issues/22

Manishearth commented 7 years ago

Should we use the same max value here? Or just INT_MAX?

cc @SimonSapin @emilio

SimonSapin commented 7 years ago

I don’t know. How was that max value picked?

Manishearth commented 7 years ago
10:18 < bz> Manishearth: 1<<30 means you can add two nscoords directly and check for overflow directly without ever triggering
            undefined behavior

Makes sense.

We should probably have this check in all operations.