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
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