[PATCH] D96203: [clang][patch] Modify sanitizer options names: renaming blacklist to blocklist

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

In D96203#2559792 <https://reviews.llvm.org/D96203#2559792>, @mibintc wrote:

> In D96203#2559426 <https://reviews.llvm.org/D96203#2559426>, @vitalybuka wrote:
>
>>>> FWIW I would prefer denylist as well. (Also uses of whitelist should be allowlist, but also incremental :)
>>
>> I feel like existing and proposed value do match the meaning of this list.
>> maybe ignorelist, skiplist (can be confused with data structure?), or just asan_no_sanitize.txt or even no_asan.txt
>
> Would you use ignorelist (skiplist) in the option name as well as the filename?  Can you recommend a name for the antonym as well (i.e. replacement for whitelist)

If we change that I would also replaced:
-list to -file to match other

We have attributes for a similar purpose in language:
https://clang.llvm.org/docs/AttributeReference.html#no-sanitize-address-no-address-safety-analysis
So maybe we can use that scheme for files which used for the same purpose?
-fno-sanitize-file=..../no_sanitize_address.txt

However we have -fno-sanitize-blocklist which then should be -fno-no-sanitize-file? :)
But I hope we can just remove that flag D79043 <https://reviews.llvm.org/D79043>



================
Comment at: clang/include/clang/Driver/Options.td:1412-1434
+def fsanitize_blocklist : Joined<["-"], "fsanitize-blocklist=">,
                           Group<f_clang_Group>,
-                          HelpText<"Path to blacklist file for sanitizers">,
-                          MarshallingInfoStringVector<LangOpts<"SanitizerBlacklistFiles">>;
-def fsanitize_system_blacklist : Joined<["-"], "fsanitize-system-blacklist=">,
-  HelpText<"Path to system blacklist file for sanitizers">,
+                          HelpText<"Path to blocklist file for sanitizers">,
+                          MarshallingInfoStringVector<LangOpts<"SanitizerBlocklistFiles">>;
+def fsanitize_blacklist : Joined<["-"], "fsanitize-blacklist=">,
+                          Group<f_clang_Group>, Alias<fsanitize_blocklist>,
+                          Flags<[HelpHidden]>,
----------------
This file has to many independent changes, I'd separate them into a separate patches when possible. At least user visible changes as this one and 	Inputs/resource_dir/share/ into a separate one.
Also it's easier to review.



================
Comment at: clang/unittests/Driver/SanitizerArgsTest.cpp:134-136
               Contains(StrEq("-fxray-always-instrument=" + XRayWhitelist)));
   EXPECT_THAT(Command.getArguments(),
+              Contains(StrEq("-fxray-never-instrument=" + XRayBlocklist)));
----------------
mibintc wrote:
> vitalybuka wrote:
> > always/never?
> Can you say a few more works here? You mean use always as replacement for white, and never as replacement for black? 
This are internal variables of unittest, it can be anything
use use thatever is part of -fxray-*-instrument= flags


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96203/new/

https://reviews.llvm.org/D96203