[clang] 03b84e4 - [clang] Report sanitizer blacklist as a dependency in cc1

Jan Korous via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 8 14:01:11 PST 2019


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.Voss at 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: jkorous at apple.com <jkorous at apple.com>
>> Sent: Friday, November 8, 2019 10:55 AM
>> To: Voss, Matthew <Matthew.Voss at sony.com>
>> Cc: Jan Korous <llvmlistbot at llvm.org>; cfe-commits at lists.llvm.org;
>> jeremy.morse.llvm at 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.Voss at 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/24312/
>>> steps/64-bit%20check-dfsan/logs/stdio
>>> 
>>> Thanks,
>>> Matthew
>>> 
>>>> -----Original Message-----
>>>> From: cfe-commits <cfe-commits-bounces at lists.llvm.org> On Behalf Of
>>>> Jan Korous via cfe-commits
>>>> Sent: Thursday, November 7, 2019 2:07 PM
>>>> To: cfe-commits at 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 at lists.llvm.org
>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> 



More information about the cfe-commits mailing list