[PATCH] D35085: Respect exitcode sanitizer option in UBSan

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

In https://reviews.llvm.org/D35085#804183, @fjricci wrote:

> It might be possible to change the default exitcode for UBSan, but I still think it should be possible for the user to make UBSan errors to return failure. Two primary reasons:
>
> - Consistency: As far as I'm aware, all of the other sanitizers return a failing exit code when they detect an error in a program, and I'm not sure why UBSan should be different in that respect.
> - Utility: A common use case I could imagine would be the case where a user wants to prevent UB regressions in their codebase by running a unit test suite with UBSan enabled. If it's not possible to return a failing exit code (either by default or otherwise), the test suite will continue passing just fine when UB is introduced.


Makes sense to me.

> A workaround that can be used currently to force a failing exit code on failure would be to use halt_on_error=1. However, it's not always desirable to limit error output to only one error (for example, brining up a unit test suite in a new codebase, where there may be many UB errors).

Also fair.

Reading the atexit man page, I'm more convinced that calling exit() in the atexit handler is not OK, since it also breaks dlclose():
"If the provided function is located in a library that has been dynamically loaded (e.g. by dlopen()), it will be called when the library is unloaded (due to a call to dlclose())"

One possible solution is to add an interceptor for exit() to ubsan, just like the one tsan has. But this breaks (a|t)san+ubsan compatibility.

Another possible solution is to make the "HasReportedError" bit shared in sanitizer_common. That way, if ubsan reports an error, _and_ is in use with (a|t)san, the exitcode support would work properly. This is just a partial solution, however, since it doesn't improve the situation for standalone ubsan. In practice people tend to run asan+ubsan together, so it would still be a win.


https://reviews.llvm.org/D35085