Bug 1581584 Comment 0 Edit History

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

I have noticed that on my machine, when compiling with ASan (with or without UBSan) using the Clang provided by mozbuild, the compiler manages to merge multiple `MOZ_CRASH `locations into one. It also managed to merge a non-`MOZ_CRASH` location with a `MOZ_CRASH` location, giving me a crash trace that indicated a null-deref inside a `Length()` function call on `nsTArray` rather than pointing to the `MOZ_CRASH` a few lines after.

We explicitly try to prevent this from happening as stated in the comments here [1]:

`
 * We use __LINE__ to prevent the compiler from folding multiple crash sites
 * together, which would make crash reports hard to understand.
`

This trick apparently doesn't work anymore. I've come up with a gtest that demonstrates the problem (patch attached). You can compile with ASan and then run `./mach gtest TestCrash.ASanBogusStack`. I derived this from a fuzzing target in Necko and this is not necessarily minimal but I thought it is good enough to demonstrate the issue.

Note that this also affects `MOZ_ASSERT`. If you take a look at bug 1577574 you will see that it is about a fatal assertion being hit, but the attached ASan crash information points to a null-deref in `StaticPtr`.


This bug is harmful to automated testing with ASan, but it might in fact not be limited to ASan at all. We probably need to prevent the compiler from re-ordering things here properly. It is unclear to me if Windows is affected because Windows has `MOZ_NoReturn` which is marked as `noinline`. Still that does not seem like a 100% safe guarantee that it will be fine.


If the bug affects non-ASan builds, this might also influence our crash reports.

[1] https://searchfox.org/mozilla-central/rev/7ed8e2d3d1d7a1464ba42763a33fd2e60efcaedc/mfbt/Assertions.h#199
I have noticed that on my machine, when compiling with ASan (with or without UBSan) using the Clang provided by mozbuild, the compiler manages to merge multiple `MOZ_CRASH `locations into one. It also managed to merge a non-`MOZ_CRASH` location with a `MOZ_CRASH` location, giving me a crash trace that indicated a null-deref inside a `Length()` function call on `nsTArray` rather than pointing to the `MOZ_CRASH` a few lines after.

We explicitly try to prevent this from happening as stated in the comments here [1]:

     * We use __LINE__ to prevent the compiler from folding multiple crash sites
     * together, which would make crash reports hard to understand.

This trick apparently doesn't work anymore. I've come up with a gtest that demonstrates the problem (patch attached). You can compile with ASan and then run `./mach gtest TestCrash.ASanBogusStack`. I derived this from a fuzzing target in Necko and this is not necessarily minimal but I thought it is good enough to demonstrate the issue.

Note that this also affects `MOZ_ASSERT`. If you take a look at bug 1577574 you will see that it is about a fatal assertion being hit, but the attached ASan crash information points to a null-deref in `StaticPtr`.


This bug is harmful to automated testing with ASan, but it might in fact not be limited to ASan at all. We probably need to prevent the compiler from re-ordering things here properly. It is unclear to me if Windows is affected because Windows has `MOZ_NoReturn` which is marked as `noinline`. Still that does not seem like a 100% safe guarantee that it will be fine.


If the bug affects non-ASan builds, this might also influence our crash reports.

[1] https://searchfox.org/mozilla-central/rev/7ed8e2d3d1d7a1464ba42763a33fd2e60efcaedc/mfbt/Assertions.h#199

Back to Bug 1581584 Comment 0