Bug 1538580 Comment 3 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

IMO this looks like a false positive from Valgrind due to LLVM's x86 backend:
- It's a known non-bug/feature of llvm:
  - [one of the relevant llvm bugs](https://bugs.llvm.org/show_bug.cgi?id=12319).
  - [one of the relevant rust rust issues](https://github.com/rust-lang/rust/issues/5856)
- We've seen stuff like this in [other rust code](https://bugzilla.mozilla.org/show_bug.cgi?id=1394696), and added suppressions for it.

This can be reproduced with a smaller rust program using the following profile in `cargo.toml`:
```toml
[profile.release]
panic = "abort"
codegen-units = 1
debug = true #Not required but makes debugging easier

[dependencies]
mp4parse = "0.11.2"
```

and the following` main.rs` (where `testcase.mp4` is the test file from this bug):
```rust
extern crate mp4parse;

use std::fs::File;

use mp4parse::MediaContext;
use mp4parse::read_mp4;

fn main() {
    let mut file = File::open("testcase.mp4").unwrap();
    let mut context = MediaContext::new();
    match read_mp4(&mut file, &mut context) {
        Err(e) => println!("{:?}", e),
        Ok(_) => println!("Okay"),
    }
}
```

build and run with `cargo build --release -vv && valgrind -q --leak-check=no --track-origins=yes target/release/<project_name>`

I've been unable to reduce the test case significantly beyond the above.

It looks like the llvm optimization being done on [this](https://searchfox.org/mozilla-central/rev/201450283cddc9e409cec707acb65ba6cf6037b1/media/mp4parse-rust/mp4parse/src/lib.rs#2078) code results in the report. Based on the previous reports, I expect that what's happening is a reordering of CMP and jumps which upsets valgrind, but is still correct. I've been digging at the assembly, and the area where valgrind complains indeed has a number of jumps -- I suspect all the different checks from the linked line. However I think fully verifying the source of the data for the comparisons on that line is doing is beyond my abilities to do in a timely fashion.

NI: kinetik, have you seen anything like this before? I'd appreciate a second set of eyes to sanity check my working.
IMO this looks like a false positive from Valgrind due to LLVM's x86 backend:
- It's a known non-bug/feature of llvm:
  - [one of the relevant llvm bugs](https://bugs.llvm.org/show_bug.cgi?id=12319).
  - [one of the relevant rust rust issues](https://github.com/rust-lang/rust/issues/5856).
- We've seen stuff like this in [other rust code](https://bugzilla.mozilla.org/show_bug.cgi?id=1394696), and added suppressions for it.

This can be reproduced with a smaller rust program using the following profile in `cargo.toml`:
```toml
[profile.release]
panic = "abort"
codegen-units = 1
debug = true #Not required but makes debugging easier

[dependencies]
mp4parse = "0.11.2"
```

and the following` main.rs` (where `testcase.mp4` is the test file from this bug):
```rust
extern crate mp4parse;

use std::fs::File;

use mp4parse::MediaContext;
use mp4parse::read_mp4;

fn main() {
    let mut file = File::open("testcase.mp4").unwrap();
    let mut context = MediaContext::new();
    match read_mp4(&mut file, &mut context) {
        Err(e) => println!("{:?}", e),
        Ok(_) => println!("Okay"),
    }
}
```

build and run with `cargo build --release -vv && valgrind -q --leak-check=no --track-origins=yes target/release/<project_name>`

I've been unable to reduce the test case significantly beyond the above.

It looks like the llvm optimization being done on [this](https://searchfox.org/mozilla-central/rev/201450283cddc9e409cec707acb65ba6cf6037b1/media/mp4parse-rust/mp4parse/src/lib.rs#2078) code results in the report. Based on the llvm and rust reports above, I suspect that what's happening is a reordering of CMP and jumps which upsets valgrind, but which results in code that is still 'correct'. I've been digging at the assembly, and the area where valgrind complains indeed has a number of jumps -- I suspect all the different checks from the linked line. However I think fully verifying the source of the data for the comparisons on that line is doing is beyond my abilities to do in a timely fashion.

NI: kinetik, have you seen anything like this before? I'd appreciate a second set of eyes to sanity check my working.

Back to Bug 1538580 Comment 3