Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

font_collection_num_fonts_overflow test fails on 32-bit architectures #127

Open
jamessan opened this issue Jul 17, 2023 · 3 comments
Open
Labels
bug Something isn't working

Comments

@jamessan
Copy link

---- font_collection_num_fonts_overflow stdout ----
thread 'font_collection_num_fonts_overflow' panicked at 'attempt to multiply with overflow', src/parser.rs:768:19
stack backtrace:
   0: rust_begin_unwind
             at /usr/src/rustc-1.66.0/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /usr/src/rustc-1.66.0/library/core/src/panicking.rs:65:14
   2: core::panicking::panic
             at /usr/src/rustc-1.66.0/library/core/src/panicking.rs:115:5
   3: ttf_parser::parser::Stream::read_array32
             at ./src/parser.rs:768:19
   4: ttf_parser::RawFace::parse
             at ./src/lib.rs:669:27
   5: ttf_parser::Face::parse
             at ./src/lib.rs:926:24
   6: tables::font_collection_num_fonts_overflow
             at ./tests/tables/main.rs:136:9
   7: tables::font_collection_num_fonts_overflow::{{closure}}
             at ./tests/tables/main.rs:125:1
   8: core::ops::function::FnOnce::call_once
             at /usr/src/rustc-1.66.0/library/core/src/ops/function.rs:251:5
   9: core::ops::function::FnOnce::call_once
             at /usr/src/rustc-1.66.0/library/core/src/ops/function.rs:251:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
@RazrFalcon RazrFalcon added the bug Something isn't working label Jul 17, 2023
@RazrFalcon
Copy link
Owner

Can confirm.

@plugwash
Copy link

The issue is that the code takes a 32-bit number, converts it to a usize and then multiplies it by SIZE. Afaict SIZE is always a small number so on 64-bit systems this can never overflow. However on 32-bit systems it can and does overflow.

The change below makes the test pass, but I'd like some feedback whether this is the correct approach before I go and add it as a distribution patch.

     pub fn read_array32<T: FromData>(&mut self, count: u32) -> Option<LazyArray
-        let len = usize::num_from(count) * T::SIZE;
-        self.read_bytes(len).map(LazyArray32::new)
+        if let Some(len) = usize::num_from(count).checked_mul(T::SIZE) {
+            self.read_bytes(len).map(LazyArray32::new)
+        } else {
+            None
+        }
     }

@RazrFalcon
Copy link
Owner

@plugwash yes, I know that this is because usize is 32bit here, but I want a global solution and not a local one. Will think about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants