rsmith added a comment. In https://reviews.llvm.org/D52386#1254483, @leonardchan wrote:
> In https://reviews.llvm.org/D52386#1254437, @rsmith wrote: > > > I'm not at all convinced that this is a good thing. There isn't such a > > thing as "undefined behavior sanitizer". Rather, there are a whole bunch of > > different checks that fall under the same umbrella. This test seems on the > > surface to be about as meaningless as `__has_feature(warnings)` would be: > > it's useless to ask the question without knowing *which* warnings you're > > talking about. But perhaps there's some use case I've overlooked (and your > > description of the patch doesn't mention why you want this). What is the > > use case you're trying to address with this change? > > > This is part of enabling UBSan for Zircon (the Fuchsia kernel) > (https://fuchsia-review.googlesource.com/c/zircon/+/197017). We enable UBSan > when building musl libc, but libc is dynamically linked first before > sanitizer runtimes, so we need to stub them out in the beginning before the > UBSan runtime is linked. This `__has_feature` is there to check if we build > with `-fsanitize=undefined` since we only want to define these stubs if libc > is built with UBSan. This sounds like something that would be better handled in the build system rather than in source code. In particular, I think you don't actually want to detect "is this translation unit being compiled with ubsan enabled?", you instead want to detect "is the rest of musl libc being compiled with ubsan enabled?" -- you should not compile the stubs themselves with ubsan enabled, because ubsan might insert calls to its runtime at arbitrary places within that translation unit, which means you'll get infinite recursion when one of the ubsan callbacks ends up calling itself. Repository: rL LLVM https://reviews.llvm.org/D52386 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
