Page MenuHomePhabricator

[clang] Report sanitizer blacklist as a dependency in cc1 instead of driver
ClosedPublic

Authored by jkorous on Oct 21 2019, 5:39 PM.

Details

Summary

Report dependencies from cc1 instead of driver.

  • This treats the default implicit blacklist added by driver just like any other blacklist dependency.
  • This also makes clang-scan-deps report the blacklist dependencies.

Essentially reverts this commit:
Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output.
4c3f237edb3e
https://github.com/llvm/llvm-project/commit/4c3f237edb3ed7f0fe1b21708ad02f0ce0b6dac2
https://reviews.llvm.org/D12544

Diff Detail

Event Timeline

jkorous created this revision.Oct 21 2019, 5:39 PM

@pcc since you reviewed the above-mentioned patch - could you please take a look at this one too?

pcc requested changes to this revision.Oct 21 2019, 5:48 PM

The current behaviour is deliberate, see crbug.com/527080 for more context. If you're trying to solve a problem with clang-scan-deps then it seems like it should be solved at a different layer.

This revision now requires changes to proceed.Oct 21 2019, 5:48 PM

I am trying to solve two different things here.

  1. The default sanitizer blacklist is a dependency about which clang-scan-deps needs to know. IIUC the driver just adds the option without reporting it dependency.
  2. I need to get blacklist dependencies in clang-scan-deps which uses cc1 command as input.

How about ignoring the default blacklist in cc1 run with -MMD but reporting it with -MD? Would that work for you?

-MD                     Write a depfile containing user and system headers
-MMD                    Write a depfile containing user headers

How about ignoring the default blacklist in cc1 run with -MMD but reporting it with -MD? Would that work for you?

Yes, that looks like it would work. It appears that goma has since been fixed so that it passes a relative resource directory path to clang, so it should no longer be emitting the absolute paths with -MD that were causing problems before. Moreover, Chromium passes -MMD, so this change would not affect it anyway.

+ @thakis as heads up and @eugenis since he was asking about this.

FTR, I've had to workaround this with -fdepfile-entry in https://reviews.llvm.org/D69587

jkorous updated this revision to Diff 227368.Oct 31 2019, 4:01 PM

Added a cc1 command line option -fsanitize-system-blacklist so driver can tell cc1 which blacklists are user-specified and which are system ones.

@eugenis Does this make your workaround obsolete?

@eugenis Does this make your workaround obsolete?

Yes, looks like it should.

@pcc Could you please take a look?

pcc accepted this revision.Nov 7 2019, 11:34 AM

LGTM

This revision is now accepted and ready to land.Nov 7 2019, 11:34 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptNov 7 2019, 2:08 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript