This is an archive of the discontinued LLVM Phabricator instance.

[DFSan][NewPM] Port DataFlowSanitizer to NewPM
ClosedPublic

Authored by aeubanks on Jul 27 2020, 3:30 PM.

Diff Detail

Event Timeline

aeubanks created this revision.Jul 27 2020, 3:30 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 27 2020, 3:30 PM
ychen accepted this revision.Jul 27 2020, 5:54 PM

LGTM with two nits. Please wait one day or two in case other reviewers want to have a look.

llvm/include/llvm/Transforms/Instrumentation/DataFlowSanitizer.h
8

LLVM_TRANSFORMS_INSTRUMENTATION_DATAFLOWSANITIZER_H

11

Is Function.h needed?

This revision is now accepted and ready to land.Jul 27 2020, 5:54 PM
aeubanks updated this revision to Diff 281109.Jul 27 2020, 9:55 PM

Address review comments

aeubanks marked an inline comment as done.Jul 27 2020, 9:55 PM
aeubanks added inline comments.
llvm/include/llvm/Transforms/Instrumentation/DataFlowSanitizer.h
11

Nope, removed.

morehouse added inline comments.Jul 29 2020, 9:06 AM
llvm/lib/Passes/PassRegistry.def
92

Nit: maybe "dfsan-module" for consistency?

llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
778

Do we need a bool to avoid calling init more than once?

aeubanks added inline comments.Jul 29 2020, 9:14 AM
llvm/lib/Passes/PassRegistry.def
92

Almost all of the others have both a module and function pass, so the "-module" is to distinguish between those. I think sancov-module is the only one that doesn't fit that, and I actually did want to rename it to sancov.

I'd like to keep the name the same as the legacy pass for migration reasons (basically so I don't have to touch every dfsan test).

llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
778

A new DataFlowSanitizer is constructed for each run/runOnModule so that shouldn't be necessary.

This revision was landed with ongoing or failed builds.Jul 29 2020, 10:19 AM
This revision was automatically updated to reflect the committed changes.