[PATCH] DataFlowSanitizer: greylist is now ABI list.

Evgeniy Stepanov eugenis at google.com
Mon Aug 12 02:24:29 PDT 2013



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

================
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);
----------------
Can this description be changed to refer to the new capabilities of the abi list?

================
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) {}
 
----------------
This way a clang setting for abilistfile will override the llvm flag. Is this intentional? IMO, it should be the other way around.

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

================
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()),
----------------
What about the rest of llvm:: usage in this file?

================
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);
----------------
Would it make sense to add a check that the opposite never happens?


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



More information about the llvm-commits mailing list