[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

yln abandoned this revision. yln added a comment. Created new revision for this change: https://reviews.llvm.org/D57278 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56624/new/ https://reviews.llvm.org/D56624 ___

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

vsk added a comment. In D56624#1370607 , @yln wrote: > In D56624#1370579 , @eugenis wrote: > > > Maybe the frontend should insert __asan_handle_noreturn whenever ASan is > > enabled, and then ASan would not care

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

yln added a comment. In D56624#1370579 , @eugenis wrote: > Maybe the frontend should insert __asan_handle_noreturn whenever ASan is > enabled, and then ASan would not care about the attribute? I'd like to avoid > having this logic in two places. +1

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

eugenis added a comment. Maybe the frontend should insert __asan_handle_noreturn whenever ASan is enabled, and then ASan would not care about the attribute? I'd like to avoid having this logic in two places. Repository: rL LLVM CHANGES SINCE LAST ACTION

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

yln added a comment. Seems as if we reached consensus! :) I will change the revision to use an intrinsic. Before I start doing that, just one more quick idea: Would it work if UBsan directly inserts calls to `__asan_handle_no_return` (of course only when ASan is requested). Similar to how it

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

vsk added a comment. In D56624#1370280 , @eugenis wrote: > > Wouldn’t it be preferable to unpoison the stack inside of maybe_longjmp, > > once the opaque condition can be checked? > > Sure, but that's not always possible. That's why we have interceptors.

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

eugenis added a comment. > Wouldn’t it be preferable to unpoison the stack inside of maybe_longjmp, once > the opaque condition can be checked? Sure, but that's not always possible. That's why we have interceptors. >> One possible optimization that I can think of is splitting code after the

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

vsk added a comment. In D56624#1370243 , @eugenis wrote: > > Because "expect_noreturn" calls are allowed to return, the compiler must > > behave as they could. In particular, this means that unpoisoning the stack > > before expect_noreturn calls (given

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

eugenis added a comment. > Because "expect_noreturn" calls are allowed to return, the compiler must > behave as they could. In particular, this means that unpoisoning the stack > before expect_noreturn calls (given the current semantics) is premature. I don't think that's true. A hypothetical

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

vsk added a comment. In D56624#1369940 , @yln wrote: > Note that all of this currently only matters when compiling with > `-fsanitize=unreachable`. The following discussion is within the context of > the current implementation: UBSan removes the

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

yln added a comment. Note that all of this currently only matters when compiling with `-fsanitize=unreachable`. The following discussion is within the context of the current implementation: UBSan removes the `noreturn` so it can instrument `unreachable` without the added instrumentation being

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

vsk added a comment. In D56624#1369767 , @yln wrote: > In D56624#1369635 , @vsk wrote: > > > What are the advantages of a generalized expect_noreturn attribute, vs. a > > narrower attribute or intrinsic? The

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

yln added a comment. @lebedev.ri Thanks for the clarifications! I will split this up into multiple patches once we settled on a design. In D56624#1369635 , @vsk wrote: > What are the advantages of a generalized expect_noreturn attribute, vs. a >

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

vsk added a comment. What are the advantages of a generalized expect_noreturn attribute, vs. a narrower attribute or intrinsic? The expect_noreturn semantics do not provide strong guarantees, and are not really orthogonal from the pre-existing cold attribute. In particular, expect_noreturn

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

lebedev.ri added a comment. In D56624#1369626 , @yln wrote: > In D56624#1368966 , @lebedev.ri > wrote: > > > Please revert this. > > First, this wasn't reviewed. > > Second, the lists weren't subscribed. > > > I

[PATCH] D56624: [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

yln reopened this revision. yln added a comment. In D56624#1368966 , @lebedev.ri wrote: > Please revert this. > First, this wasn't reviewed. > Second, the lists weren't subscribed. I apologize for this. It was not my intention to land the revision