[PATCH] DataFlowSanitizer: greylist is now ABI list.

[Peter Collingbourne]

================
Comment at: lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:255
@@ -213,3 +254,3 @@
     : ModulePass(ID), GetArgTLSPtr(getArgTLS), GetRetvalTLSPtr(getRetValTLS),
-      Greylist(ClGreylistFile) {}
+      ABIList(ABIListFile.empty() ? ClABIListFile : ABIListFile) {}
 
----------------
Evgeniy Stepanov wrote:
> This way a clang setting for abilistfile will override the llvm flag. Is this intentional? IMO, it should be the other way around.
This is what all the other sanitizers do.  It also makes more sense to me -- the local setting (what clang provides through the ABIListFile argument) should override the global setting (the command line parameter).

================
Comment at: lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:88
@@ -87,2 +87,3 @@
+    "dfsan-abilist",
     cl::desc("File containing the list of functions with a native ABI"),
     cl::Hidden);
----------------
Evgeniy Stepanov wrote:
> Can this description be changed to refer to the new capabilities of the abi list?
Done.

================
Comment at: lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:91
@@ -89,3 +90,3 @@
 
 static cl::opt<bool> ClArgsABI(
     "dfsan-args-abi",
----------------
Evgeniy Stepanov wrote:
> Please add a comment for this flag.
Done.

================
Comment at: lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:510
@@ -411,3 +509,3 @@
     // Build a copy of the list before iterating over it.
-    llvm::SmallVector<BasicBlock *, 4> BBList;
+    SmallVector<BasicBlock *, 4> BBList;
     std::copy(df_begin(&(*i)->getEntryBlock()), df_end(&(*i)->getEntryBlock()),
----------------
Evgeniy Stepanov wrote:
> What about the rest of llvm:: usage in this file?
OK, I'll deal with that separately.

================
Comment at: lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:1032
@@ +1031,3 @@
+            DFSF.DFS.Mod->getOrInsertFunction(CustomFName, CustomFT);
+        if (Function *CustomFn = dyn_cast<Function>(CustomF)) {
+          CustomFn->copyAttributesFrom(F);
----------------
Evgeniy Stepanov wrote:
> Would it make sense to add a check that the opposite never happens?
It might happen -- e.g. if the translation unit uses or defines a __dfsw_ function with the wrong type (which would be wrong in any case because __dfsw_ functions should only be used directly or defined in a non-DFSan TU, but there's nothing stopping it from happening).  In that case we should probably do the least-worst thing we can which is to preserve the attributes on the original function.

================
Comment at: lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:376
@@ -311,1 +375,3 @@
+  DFSanUnimplementedFn =
+      Mod->getOrInsertFunction("__dfsan_unimplemented", DFSanUnimplementedFnTy);
 
----------------
Evgeniy Stepanov wrote:
> 80 chars
This is exactly 80 chars.


http://llvm-reviews.chandlerc.com/D1345