Assuming the implemented flag naming is ok with everyone, the patch is good. Few questions:
+ unsigned Kind; Do we only allow 32 sanitizers? Why not 64? Do we want to change -faddress-sanitizer in asan/tsan lit tests? projects/compiler-rt/lib/asan/lit_tests projects/compiler-rt/lib/tsan/lit_tests How long do we keep the aliases (-faddress-sanitizer => fsanitizer=address)? (I like the old name more than the new name, but having two names is confusing) Will this allow to easily add *san-specific subflags? E.g. --sanitizer=address,global-init-order,use-after-return,use-after-scope or --sanitizer=memory,memory-origins (The first one indicates 3 asan sub passes that are curretly off by default, the second one indicates that msan should be run with "track origins" feature) --kcc On Fri, Nov 2, 2012 at 10:03 AM, Kostya Serebryany <[email protected]> wrote: > > > On Fri, Nov 2, 2012 at 9:41 AM, Alexander Potapenko <[email protected]>wrote: > >> I haven't seen the patch yet, but here are two thoughts: >> - some tools may be incompatible with each other (e.g. ASan and TSan), so >> we shouldn't allow to use them together; >> - there are many users of TSan and ASan, thus we can't easily rename the >> corresponding command line options. The right thing to do is to make >> -fsanitize fully functional, announce that and then remove the existing >> flags. >> I'm also unsure whether having such an umbrella flag for such different >> tools won't confuse the users. >> > As Richard says, the old flags remain (they become synonyms of the new > flags). That sounds fine. > I'll check the patches later today. > > --kcc > > >> On Nov 2, 2012 4:51 AM, "Richard Smith" <[email protected]> wrote: >> >>> Hi, >>> >>> The attached patch series adds a >>> -fsanitize=sanitizer1,sanitizer2,sanitizer3 command-line option to >>> Clang (and a corresponding -fno-sanitize=...). This works as follows >>> for the driver options: >>> >>> -faddress-sanitizer becomes a synonym for -fsanitize=address >>> -fno-address-sanitizer becomes a synonym for -fno-sanitize=address >>> -fthread-sanitizer becomes a synonym for -fsanitize=thread >>> -fno-thread-sanitizer becomes a synonym for -fno-sanitize=thread >>> -fcatch-undefined-behavior becomes a synonym for -fsanitize=undefined >>> -fsanitize=undefined is expanded by the driver into a list of >>> individual undefined behavior sanitizers >>> >>> The -cc1 interface is deliberately very simplistic: it loses support >>> for the existing flags, and instead only supports -fsanitize=..., and >>> only supports naming the individual checks (and not groups like >>> -fsanitize=undefined). >>> >>> Patch 1 just renames the existing ASan and TSan LangOptions to follow >>> the naming convention used by the later patches. >>> Patch 2 adds support to the driver and frontend for parsing >>> -fsanitize, converting the other options to it, and filling in >>> LangOptions. >>> Patch 3 uses the -fsanitize=... values to enable/disable the existing >>> -fcatch-undefined-behavior checks, and removes frontend support for >>> the other options. >>> >>> Please review! I'd like to get this in for the 3.2 release. >>> >>> Thanks! >>> Richard >>> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
