Thanks! LGTM > -----Original Message----- > From: jkor...@apple.com <jkor...@apple.com> > Sent: Friday, November 8, 2019 3:33 PM > To: Voss, Matthew <matthew.v...@sony.com> > Cc: Jan Korous <llvmlist...@llvm.org>; cfe-commits@lists.llvm.org; > jeremy.morse.l...@gmail.com > Subject: Re: [clang] 03b84e4 - [clang] Report sanitizer blacklist as a > dependency in cc1 > > Yes, sorry, I messed up when relanding the patch. Seems fixed now. > > > On Nov 8, 2019, at 2:24 PM, Voss, Matthew <matthew.v...@sony.com> wrote: > > > > Hi Jan, > > > > Thanks for looking at this. I'm seeing new errors on the PS4 windows > bot. > > > > http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubunt > > u-fast/builds/57835 > > > > Thanks, > > Matthew > > > >> -----Original Message----- > >> From: jkor...@apple.com <jkor...@apple.com> > >> Sent: Friday, November 8, 2019 2:01 PM > >> To: Voss, Matthew <matthew.v...@sony.com> > >> Cc: Jan Korous <llvmlist...@llvm.org>; cfe-commits@lists.llvm.org; > >> jeremy.morse.l...@gmail.com > >> Subject: Re: [clang] 03b84e4 - [clang] Report sanitizer blacklist as > >> a dependency in cc1 > >> > >> Hi Matthew, > >> > >> You were absolutely right - my bad! > >> > >> I relanded the patches plus a fix. > >> > >> 555c6be041d [clang] Fix -fsanitize-system-blacklist processing in cc1 > >> > >> Thanks, > >> > >> Jan > >> > >>> On Nov 8, 2019, at 11:00 AM, Voss, Matthew <matthew.v...@sony.com> > >> wrote: > >>> > >>> Hi Jan, > >>> > >>>> Are you sure it is this commit? > >>> I narrowed it down to your commit on my local machine. Let me know > >>> if it > >> doesn't repro on your end, though. > >>> > >>> Thanks, > >>> Matthew > >>> > >>> > >>>> -----Original Message----- > >>>> From: jkor...@apple.com <jkor...@apple.com> > >>>> Sent: Friday, November 8, 2019 10:55 AM > >>>> To: Voss, Matthew <matthew.v...@sony.com> > >>>> Cc: Jan Korous <llvmlist...@llvm.org>; cfe-commits@lists.llvm.org; > >>>> jeremy.morse.l...@gmail.com > >>>> Subject: Re: [clang] 03b84e4 - [clang] Report sanitizer blacklist > >>>> as a dependency in cc1 > >>>> > >>>> Hi Matthew, > >>>> > >>>> Are you sure it is this commit? It is definitely possible yet > >>>> sounds a bit unlikely that my patch would cause linker errors. > >>>> > >>>> Anyway, I am going to reproduce on a linux box. > >>>> > >>>> Thanks. > >>>> > >>>> Jan > >>>> > >>>>> On Nov 7, 2019, at 4:50 PM, Voss, Matthew <matthew.v...@sony.com> > >> wrote: > >>>>> > >>>>> Hi Jan, > >>>>> > >>>>> It looks like this commit is causing DFSAN failures on the > >>>>> sanitizer > >>>> bots and our internal CI. Could you take a look? > >>>>> > >>>>> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/24 > >>>>> 31 2/ steps/64-bit%20check-dfsan/logs/stdio > >>>>> > >>>>> Thanks, > >>>>> Matthew > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: cfe-commits <cfe-commits-boun...@lists.llvm.org> On Behalf > >>>>>> Of Jan Korous via cfe-commits > >>>>>> Sent: Thursday, November 7, 2019 2:07 PM > >>>>>> To: cfe-commits@lists.llvm.org > >>>>>> Subject: [clang] 03b84e4 - [clang] Report sanitizer blacklist as > >>>>>> a dependency in cc1 > >>>>>> > >>>>>> > >>>>>> Author: Jan Korous > >>>>>> Date: 2019-11-07T14:06:43-08:00 > >>>>>> New Revision: 03b84e4f6d0e1c04f22d69cc445f36e1f713beb4 > >>>>>> > >>>>>> URL: https://github.com/llvm/llvm- > >>>>>> project/commit/03b84e4f6d0e1c04f22d69cc445f36e1f713beb4 > >>>>>> DIFF: https://github.com/llvm/llvm- > >>>>>> project/commit/03b84e4f6d0e1c04f22d69cc445f36e1f713beb4.diff > >>>>>> > >>>>>> LOG: [clang] Report sanitizer blacklist as a dependency in cc1 > >>>>>> > >>>>>> Previously these were reported from the driver which blocked > >>>>>> clang-scan- deps from getting the full set of dependencies from > >>>>>> cc1 > >>>> commands. > >>>>>> > >>>>>> Also the default sanitizer blacklist that is added in driver was > >>>>>> never reported as a dependency. I introduced > >>>>>> -fsanitize-system-blacklist cc1 option to keep track of which > >>>>>> blacklists were user-specified and which were added by driver and > >>>>>> clang -MD now also reports system blacklists as dependencies. > >>>>>> > >>>>>> Differential Revision: https://reviews.llvm.org/D69290 > >>>>>> > >>>>>> Added: > >>>>>> > >>>>>> > >>>>>> Modified: > >>>>>> clang/include/clang/Driver/Options.td > >>>>>> clang/include/clang/Driver/SanitizerArgs.h > >>>>>> clang/lib/Driver/SanitizerArgs.cpp > >>>>>> clang/lib/Frontend/CompilerInvocation.cpp > >>>>>> clang/test/Driver/fsanitize-blacklist.c > >>>>>> clang/test/Frontend/dependency-gen.c > >>>>>> > >>>>>> Removed: > >>>>>> > >>>>>> > >>>>>> > >>>>>> ################################################################# > >>>>>> ## > >>>>>> ## > >>>>>> ##### > >>>>>> ###### > >>>>>> diff --git a/clang/include/clang/Driver/Options.td > >>>>>> b/clang/include/clang/Driver/Options.td > >>>>>> index dcd2976a97f2..c2e30a16b8da 100644 > >>>>>> --- a/clang/include/clang/Driver/Options.td > >>>>>> +++ b/clang/include/clang/Driver/Options.td > >>>>>> @@ -979,6 +979,9 @@ def fno_sanitize_EQ : CommaJoined<["-"], > >>>>>> "fno- sanitize=">, Group<f_clang_Group>, def fsanitize_blacklist : > >>>>>> Joined<["- "], "fsanitize-blacklist=">, > >>>>>> Group<f_clang_Group>, > >>>>>> HelpText<"Path to blacklist file for > >>>>>> sanitizers">; > >>>>>> +def fsanitize_system_blacklist : Joined<["-"], > >>>>>> +"fsanitize-system-blacklist=">, > >>>>>> + HelpText<"Path to system blacklist file for sanitizers">, > >>>>>> + Flags<[CC1Option]>; > >>>>>> def fno_sanitize_blacklist : Flag<["-"], "fno-sanitize-blacklist">, > >>>>>> Group<f_clang_Group>, > >>>>>> HelpText<"Don't use blacklist file for > >>>>>> sanitizers">; > >>>>>> > >>>>>> diff --git a/clang/include/clang/Driver/SanitizerArgs.h > >>>>>> b/clang/include/clang/Driver/SanitizerArgs.h > >>>>>> index c37499e0f201..0aebf8cb225d 100644 > >>>>>> --- a/clang/include/clang/Driver/SanitizerArgs.h > >>>>>> +++ b/clang/include/clang/Driver/SanitizerArgs.h > >>>>>> @@ -25,8 +25,8 @@ class SanitizerArgs { SanitizerSet > >>>>>> RecoverableSanitizers; SanitizerSet TrapSanitizers; > >>>>>> > >>>>>> - std::vector<std::string> BlacklistFiles; > >>>>>> - std::vector<std::string> ExtraDeps; > >>>>>> + std::vector<std::string> UserBlacklistFiles; > >>>>>> + std::vector<std::string> SystemBlacklistFiles; > >>>>>> int CoverageFeatures = 0; > >>>>>> int MsanTrackOrigins = 0; > >>>>>> bool MsanUseAfterDtor = true; > >>>>>> > >>>>>> diff --git a/clang/lib/Driver/SanitizerArgs.cpp > >>>>>> b/clang/lib/Driver/SanitizerArgs.cpp > >>>>>> index cc6c5e6ef438..8937197c253c 100644 > >>>>>> --- a/clang/lib/Driver/SanitizerArgs.cpp > >>>>>> +++ b/clang/lib/Driver/SanitizerArgs.cpp > >>>>>> @@ -557,29 +557,35 @@ SanitizerArgs::SanitizerArgs(const > >>>>>> ToolChain &TC, > >>>>>> > >>>>>> // Setup blacklist files. > >>>>>> // Add default blacklist from resource directory. > >>>>>> - addDefaultBlacklists(D, Kinds, BlacklistFiles); > >>>>>> + addDefaultBlacklists(D, Kinds, SystemBlacklistFiles); > >>>>>> // Parse -f(no-)sanitize-blacklist options. > >>>>>> for (const auto *Arg : Args) { > >>>>>> if (Arg->getOption().matches(options::OPT_fsanitize_blacklist)) { > >>>>>> Arg->claim(); > >>>>>> std::string BLPath = Arg->getValue(); > >>>>>> if (llvm::sys::fs::exists(BLPath)) { > >>>>>> - BlacklistFiles.push_back(BLPath); > >>>>>> - ExtraDeps.push_back(BLPath); > >>>>>> + UserBlacklistFiles.push_back(BLPath); > >>>>>> } else { > >>>>>> D.Diag(clang::diag::err_drv_no_such_file) << BLPath; > >>>>>> } > >>>>>> } else if (Arg- > >>>>>>> getOption().matches(options::OPT_fno_sanitize_blacklist)) { > >>>>>> Arg->claim(); > >>>>>> - BlacklistFiles.clear(); > >>>>>> - ExtraDeps.clear(); > >>>>>> + UserBlacklistFiles.clear(); > >>>>>> + SystemBlacklistFiles.clear(); > >>>>>> } > >>>>>> } > >>>>>> // Validate blacklists format. > >>>>>> { > >>>>>> std::string BLError; > >>>>>> std::unique_ptr<llvm::SpecialCaseList> SCL( > >>>>>> - llvm::SpecialCaseList::create(BlacklistFiles, BLError)); > >>>>>> + llvm::SpecialCaseList::create(UserBlacklistFiles, > BLError)); > >>>>>> + if (!SCL.get()) > >>>>>> + D.Diag(clang::diag::err_drv_malformed_sanitizer_blacklist) > >>>>>> + << BLError; } { > >>>>>> + std::string BLError; > >>>>>> + std::unique_ptr<llvm::SpecialCaseList> SCL( > >>>>>> + llvm::SpecialCaseList::create(SystemBlacklistFiles, > >>>>>> + BLError)); > >>>>>> if (!SCL.get()) > >>>>>> D.Diag(clang::diag::err_drv_malformed_sanitizer_blacklist) << > >>>>>> BLError; } @@ -952,15 +958,15 @@ void > >>>>>> SanitizerArgs::addArgs(const ToolChain &TC, const > llvm::opt::ArgList &Args, > >>>>>> CmdArgs.push_back( > >>>>>> Args.MakeArgString("-fsanitize-trap=" + > >>>>>> toString(TrapSanitizers))); > >>>>>> > >>>>>> - for (const auto &BLPath : BlacklistFiles) { > >>>>>> + for (const auto &BLPath : UserBlacklistFiles) { > >>>>>> SmallString<64> BlacklistOpt("-fsanitize-blacklist="); > >>>>>> BlacklistOpt += BLPath; > >>>>>> CmdArgs.push_back(Args.MakeArgString(BlacklistOpt)); > >>>>>> } > >>>>>> - for (const auto &Dep : ExtraDeps) { > >>>>>> - SmallString<64> ExtraDepOpt("-fdepfile-entry="); > >>>>>> - ExtraDepOpt += Dep; > >>>>>> - CmdArgs.push_back(Args.MakeArgString(ExtraDepOpt)); > >>>>>> + for (const auto &BLPath : SystemBlacklistFiles) { > >>>>>> + SmallString<64> BlacklistOpt("-fsanitize-system-blacklist="); > >>>>>> + BlacklistOpt += BLPath; > >>>>>> + CmdArgs.push_back(Args.MakeArgString(BlacklistOpt)); > >>>>>> } > >>>>>> > >>>>>> if (MsanTrackOrigins) > >>>>>> > >>>>>> diff --git a/clang/lib/Frontend/CompilerInvocation.cpp > >>>>>> b/clang/lib/Frontend/CompilerInvocation.cpp > >>>>>> index 195a29d71187..17fd4ce7752b 100644 > >>>>>> --- a/clang/lib/Frontend/CompilerInvocation.cpp > >>>>>> +++ b/clang/lib/Frontend/CompilerInvocation.cpp > >>>>>> @@ -1447,7 +1447,26 @@ static void > >>>>>> ParseDependencyOutputArgs(DependencyOutputOptions &Opts, // Add > >>>>>> sanitizer blacklists as extra dependencies. > >>>>>> // They won't be discovered by the regular preprocessor, so // > >>>>>> we let make / ninja to know about this implicit dependency. > >>>>>> - Opts.ExtraDeps = Args.getAllArgValues(OPT_fdepfile_entry); > >>>>>> + if (!Args.hasArg(OPT_fno_sanitize_blacklist)) { > >>>>>> + for (const auto *A : Args.filtered(OPT_fsanitize_blacklist)) { > >>>>>> + StringRef Val = A->getValue(); > >>>>>> + if (Val.find('=') == StringRef::npos) > >>>>>> + Opts.ExtraDeps.push_back(Val); > >>>>>> + } > >>>>>> + if (Opts.IncludeSystemHeaders) { > >>>>>> + for (const auto *A : > >>>>>> + Args.filtered(OPT_fsanitize_system_blacklist)) > >>>>>> { > >>>>>> + StringRef Val = A->getValue(); > >>>>>> + if (Val.find('=') == StringRef::npos) > >>>>>> + Opts.ExtraDeps.push_back(Val); > >>>>>> + } > >>>>>> + } > >>>>>> + } > >>>>>> + > >>>>>> + // Propagate the extra dependencies. > >>>>>> + for (const auto *A : Args.filtered(OPT_fdepfile_entry)) { > >>>>>> + Opts.ExtraDeps.push_back(A->getValue()); > >>>>>> + } > >>>>>> + > >>>>>> // Only the -fmodule-file=<file> form. > >>>>>> for (const auto *A : Args.filtered(OPT_fmodule_file)) { > >>>>>> StringRef Val = A->getValue(); > >>>>>> > >>>>>> diff --git a/clang/test/Driver/fsanitize-blacklist.c > >>>>>> b/clang/test/Driver/fsanitize-blacklist.c > >>>>>> index e08905c94eda..6878298e6752 100644 > >>>>>> --- a/clang/test/Driver/fsanitize-blacklist.c > >>>>>> +++ b/clang/test/Driver/fsanitize-blacklist.c > >>>>>> @@ -16,22 +16,18 @@ > >>>>>> // RUN: %clang -target aarch64-linux-gnu -fsanitize=hwaddress > >>>>>> -fsanitize- blacklist=%t.good -fsanitize-blacklist=%t.second %s > >>>>>> -### > >>>>>> 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST // > >>>>>> CHECK-BLACKLIST: -fsanitize- blacklist={{.*}}.good" > >>>>>> "-fsanitize-blacklist={{.*}}.second > >>>>>> > >>>>>> -// Now, check for -fdepfile-entry flags. > >>>>>> -// RUN: %clang -target x86_64-linux-gnu -fsanitize=address > >>>>>> -fsanitize- blacklist=%t.good -fsanitize-blacklist=%t.second %s > >>>>>> -### > >>>>>> 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST2 -// > >>>>>> CHECK-BLACKLIST2: -fdepfile- entry={{.*}}.good" > >>>>>> "-fdepfile-entry={{.*}}.second > >>>>>> - > >>>>>> // Check that the default blacklist is not added as an extra > >>>> dependency. > >>>>>> // RUN: %clang -target x86_64-linux-gnu -fsanitize=address > >>>>>> -resource- dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s > >>>>>> --check- prefix=CHECK-DEFAULT-BLACKLIST-ASAN > >>>>>> --implicit-check-not=fdepfile-entry -- > >>>>>> implicit-check-not=-fsanitize-blacklist= > >>>>>> -// CHECK-DEFAULT-BLACKLIST-ASAN: -fsanitize- > >>>>>> blacklist={{.*[^w]}}asan_blacklist.txt > >>>>>> +// CHECK-DEFAULT-BLACKLIST-ASAN: > >>>>>> +-fsanitize-system-blacklist={{.*[^w]}}asan_blacklist.txt > >>>>>> // RUN: %clang -target x86_64-linux-gnu -fsanitize=hwaddress > >>>>>> -resource- dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s > >>>>>> --check- prefix=CHECK-DEFAULT-BLACKLIST-HWASAN > >>>>>> --implicit-check-not=fdepfile-entry > >>>>>> --implicit-check-not=-fsanitize-blacklist= > >>>>>> -// CHECK-DEFAULT-BLACKLIST-HWASAN: -fsanitize- > >>>>>> blacklist={{.*}}hwasan_blacklist.txt > >>>>>> +// CHECK-DEFAULT-BLACKLIST-HWASAN: > >>>>>> +-fsanitize-system-blacklist={{.*}}hwasan_blacklist.txt > >>>>>> > >>>>>> // RUN: %clang -target x86_64-linux-gnu -fsanitize=integer > >>>>>> -resource- dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s > >>>>>> --check- prefix=CHECK-DEFAULT-UBSAN-BLACKLIST > >>>>>> --implicit-check-not=fdepfile-entry - > >>>>>> -implicit-check-not=-fsanitize-blacklist= > >>>>>> // RUN: %clang -target x86_64-linux-gnu -fsanitize=nullability > >>>>>> -resource- dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s > >>>>>> --check- prefix=CHECK-DEFAULT-UBSAN-BLACKLIST > >>>>>> --implicit-check-not=fdepfile-entry - > >>>>>> -implicit-check-not=-fsanitize-blacklist= > >>>>>> // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined > >>>>>> -resource- dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s > >>>>>> --check- prefix=CHECK-DEFAULT-UBSAN-BLACKLIST > >>>>>> --implicit-check-not=fdepfile-entry - > >>>>>> -implicit-check-not=-fsanitize-blacklist= > >>>>>> // RUN: %clang -target x86_64-linux-gnu -fsanitize=alignment > >>>>>> -resource- dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s > >>>>>> --check- prefix=CHECK-DEFAULT-UBSAN-BLACKLIST > >>>>>> --implicit-check-not=fdepfile-entry - > >>>>>> -implicit-check-not=-fsanitize-blacklist= > >>>>>> // RUN: %clang -target %itanium_abi_triple > >>>>>> -fsanitize=float-divide-by- zero > >>>>>> -resource-dir=%S/Inputs/resource_dir > >>>>>> %s -### 2>&1 | FileCheck %s -- > >>>>>> check-prefix=CHECK-DEFAULT-UBSAN-BLACKLIST > >>>>>> --implicit-check-not=fdepfile- entry > >>>>>> --implicit-check-not=-fsanitize-blacklist= > >>>>>> -// CHECK-DEFAULT-UBSAN-BLACKLIST: -fsanitize- > >>>>>> blacklist={{.*}}ubsan_blacklist.txt > >>>>>> +// CHECK-DEFAULT-UBSAN-BLACKLIST: > >>>>>> +-fsanitize-system-blacklist={{.*}}ubsan_blacklist.txt > >>>>>> > >>>>>> // Check that combining ubsan and another sanitizer results in > >>>>>> both blacklists being used. > >>>>>> // RUN: %clang -target x86_64-linux-gnu > >>>>>> -fsanitize=undefined,address > >>>>>> - resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s > >>>>>> --check- prefix=CHECK-DEFAULT-UBSAN-BLACKLIST > >>>>>> --check-prefix=CHECK-DEFAULT-ASAN- > >>>>>> BLACKLIST --implicit-check-not=fdepfile-entry > >>>>>> --implicit-check-not=- fsanitize-blacklist= > >>>>>> > >>>>>> diff --git a/clang/test/Frontend/dependency-gen.c > >>>>>> b/clang/test/Frontend/dependency-gen.c > >>>>>> index 963419cb1188..1db9b04c1d9f 100644 > >>>>>> --- a/clang/test/Frontend/dependency-gen.c > >>>>>> +++ b/clang/test/Frontend/dependency-gen.c > >>>>>> @@ -27,3 +27,20 @@ > >>>>>> #ifndef INCLUDE_FLAG_TEST > >>>>>> #include <x.h> > >>>>>> #endif > >>>>>> + > >>>>>> +// RUN: echo "fun:foo" > %t.blacklist1 // RUN: echo "fun:foo" > > >>>>>> +%t.blacklist2 // RUN: %clang -MD -MF - %s -fsyntax-only > >>>>>> +-resource-dir=%S/Inputs/resource_dir_with_cfi_blacklist > >>>>>> +-fsanitize=cfi- > >>>>>> vcall -flto -fvisibility=hidden > >>>>>> -fsanitize-blacklist=%t.blacklist1 > >>>>>> - > >>>>>> fsanitize-blacklist=%t.blacklist2 -I ./ | FileCheck > >>>>>> -check-prefix=TWO- BLACK-LISTS %s // TWO-BLACK-LISTS: dependency- > >> gen.o: > >>>>>> +// TWO-BLACK-LISTS-DAG: blacklist1 // TWO-BLACK-LISTS-DAG: > >>>>>> +blacklist2 // TWO-BLACK-LISTS-DAG: x.h // TWO-BLACK-LISTS-DAG: > >>>>>> +dependency-gen.c > >>>>>> + > >>>>>> +// RUN: %clang -MD -MF - %s -fsyntax-only > >>>>>> +-resource-dir=%S/Inputs/resource_dir_with_cfi_blacklist > >>>>>> +-fsanitize=cfi- > >>>>>> vcall -flto -fvisibility=hidden -I ./ | FileCheck > >>>>>> -check-prefix=USER-AND- SYS-DEPS %s // USER-AND-SYS-DEPS: > >>>>>> dependency- > >>>> gen.o: > >>>>>> +// USER-AND-SYS-DEPS-DAG: cfi_blacklist.txt > >>>>>> + > >>>>>> +// RUN: %clang -MMD -MF - %s -fsyntax-only > >>>>>> +-resource-dir=%S/Inputs/resource_dir_with_cfi_blacklist > >>>>>> +-fsanitize=cfi- > >>>>>> vcall -flto -fvisibility=hidden -I ./ | FileCheck > >>>>>> -check-prefix=ONLY-USER- DEPS %s // ONLY-USER-DEPS: dependency- > gen.o: > >>>>>> +// NOT-ONLY-USER-DEPS: cfi_blacklist.txt > >>>>>> \ No newline at end of file > >>>>>> > >>>>>> > >>>>>> > >>>>>> _______________________________________________ > >>>>>> cfe-commits mailing list > >>>>>> cfe-commits@lists.llvm.org > >>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > >>> > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits