Messages in this thread | | | From | Marco Elver <> | Date | Wed, 26 May 2021 14:38:21 +0200 | Subject | Re: [PATCH] kcov: add __no_sanitize_coverage to fix noinstr for all architectures |
| |
On Wed, 26 May 2021 at 08:25, Marco Elver <elver@google.com> wrote: > On Wed, 26 May 2021 at 03:54, Miguel Ojeda > <miguel.ojeda.sandonis@gmail.com> wrote: > > On Tue, May 25, 2021 at 9:13 PM Marco Elver <elver@google.com> wrote: > > > > > > Long story short: this is not fixable without more Clang changes. The > > > only way to do it without a version check would be to introduce > > > no_sanitize_coverage attr to Clang, which we probably shouldn't do, > > > and I didn't want to fight it. ;-) > > > > I am not sure I followed why you would not want to support querying > > for the attributes (if they are intended to be used separately). > > Not my decision, but some historical decision in Clang. Somebody > thought "no_sanitize(<string_literal>)" simplifies things. Hence, > Clang only knows about the no_sanitize attribute but not its > "subattributes". > > > But regardless of that, why not the feature flag at least then, to be > > consistent with the others? > > __has_feature(coverage_sanitizer) does not work either (yet). > > > Going back to version checks seems bad -- they should be reserved for > > e.g. known broken versions and things like that. New compiler features > > should come with new feature flags... > > > > In fact, for Clang, I do not see any version checks in code at the > > moment, so this would be the first :( > > In this instance it's absolutely required (for now). But if you don't > like it I'll go back to trying to fix Clang more. I'll check with > Clang folks which one we can implement, the feature check or the > attribute check.
Ok, let's wait for response to: https://reviews.llvm.org/D103159 If that lands in the LLVM repo I'll change to use __has_feature(coverage_sanitizer), and send a v2. That __has_feature() is a bit of a lie though, because fsanitize-coverage has long been supported, but it just so happens that if we get it, then its availability implies availability of the no_sanitize("coverage") attribute.
Thanks, -- Marco
| |