[dfsan] Fix clang-tidy warnings

[gbalats]

Summary

This addresses ~50 clang-tidy warnings on dfsan instrumentation pass.
It also contains some refactoring (all non-functional changes) to eliminate some variables and simplify code.

[stephan.yichao.zhao]

Thank you!

[gbalats]

Fix llvm-dfsan test failure

llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp

 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/DepthFirstIterator.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/ADT/iterator.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/Argument.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DerivedTypes.h"

 #include <set>
 #include <string>
 #include <utility>
 #include <vector>
 
 using namespace llvm;
 
 // This must be consistent with ShadowWidthBits.
-static const Align kShadowTLSAlignment = Align(2);
+static const Align ShadowTLSAlignment = Align(2);
 
-static const Align kMinOriginAlignment = Align(4);
+static const Align MinOriginAlignment = Align(4);
 
 // The size of TLS variables. These constants must be kept in sync with the ones
 // in dfsan.cpp.
-static const unsigned kArgTLSSize = 800;
-static const unsigned kRetvalTLSSize = 800;
+static const unsigned ArgTLSSize = 800;
+static const unsigned RetvalTLSSize = 800;
 
 // External symbol to be used when generating the shadow address for
 // architectures with multiple VMAs. Instead of using a constant integer
 // the runtime will set the external mask based on the VMA range.
-const char kDFSanExternShadowPtrMask[] = "__dfsan_shadow_ptr_mask";
+const char DFSanExternShadowPtrMask[] = "__dfsan_shadow_ptr_mask";
 
 // The -dfsan-preserve-alignment flag controls whether this pass assumes that
 // alignment requirements provided by the input IR are correct.  For example,
 // if the input IR contains a load with alignment 8, this flag will cause
 // the shadow load to have alignment 16.  This flag is disabled by default as
 // we have unfortunately encountered too much code (including Clang itself;
 // see PR14291) which performs misaligned access.
 static cl::opt<bool> ClPreserveAlignment(

 // which are described below under DataFlowSanitizer::WrapperKind.
 static cl::list<std::string> ClABIListFiles(
     "dfsan-abilist",
     cl::desc("File listing native ABI functions and how the pass treats them"),
     cl::Hidden);
 
 // Controls whether the pass uses IA_Args or IA_TLS as the ABI for instrumented
 // functions (see DataFlowSanitizer::InstrumentedABI below).
-static cl::opt<bool> ClArgsABI(
-    "dfsan-args-abi",
+static cl::opt<bool>
+    ClArgsABI("dfsan-args-abi",
               cl::desc("Use the argument ABI rather than the TLS ABI"),
               cl::Hidden);
 
 // Controls whether the pass includes or ignores the labels of pointers in load
 // instructions.
 static cl::opt<bool> ClCombinePointerLabelsOnLoad(
     "dfsan-combine-pointer-labels-on-load",
     cl::desc("Combine the label of the pointer with the label of the data when "
              "loading from memory."),
     cl::Hidden, cl::init(true));

 // Controls how to track origins.
 // * 0: do not track origins.
 // * 1: track origins at memory store operations.
 // * 2: TODO: track origins at memory store operations and callsites.
 static cl::opt<int> ClTrackOrigins("dfsan-track-origins",
                                    cl::desc("Track origins of labels"),
                                    cl::Hidden, cl::init(0));
 
-static StringRef GetGlobalTypeString(const GlobalValue &G) {
+static StringRef getGlobalTypeString(const GlobalValue &G) {
   // Types of GlobalVariables are always pointer types.
   Type *GType = G.getValueType();
   // For now we support excluding struct types only.
   if (StructType *SGType = dyn_cast<StructType>(GType)) {
     if (!SGType->isLiteral())
       return SGType->getName();
   }
   return "<unknown type>";
 }
 
 namespace {
 
 class DFSanABIList {
   std::unique_ptr<SpecialCaseList> SCL;
 
- public:
   DFSanABIList() = default;
 
   void set(std::unique_ptr<SpecialCaseList> List) { SCL = std::move(List); }
 
   /// Returns whether either this function or its source file are listed in the
   /// given category.
   bool isIn(const Function &F, StringRef Category) const {
     return isIn(*F.getParent(), Category) ||
            SCL->inSection("dataflow", "fun", F.getName(), Category);
   }
 
   /// Returns whether this global alias is listed in the given category.
   ///
   /// If GA aliases a function, the alias's name is matched as a function name
   /// would be.  Similarly, aliases of globals are matched like globals.
   bool isIn(const GlobalAlias &GA, StringRef Category) const {
     if (isIn(*GA.getParent(), Category))
       return true;
 
     if (isa<FunctionType>(GA.getValueType()))
       return SCL->inSection("dataflow", "fun", GA.getName(), Category);
 
     return SCL->inSection("dataflow", "global", GA.getName(), Category) ||
-           SCL->inSection("dataflow", "type", GetGlobalTypeString(GA),
+           SCL->inSection("dataflow", "type", getGlobalTypeString(GA),
                           Category);
   }
 
   /// Returns whether this module is listed in the given category.
   bool isIn(const Module &M, StringRef Category) const {
     return SCL->inSection("dataflow", "src", M.getModuleIdentifier(), Category);
   }
 };
 
 /// TransformedFunction is used to express the result of transforming one
 /// function type into another.  This struct is immutable.  It holds metadata
 /// useful for updating calls of the old function to the new type.
 struct TransformedFunction {
-  TransformedFunction(FunctionType* OriginalType,
-                      FunctionType* TransformedType,
+  TransformedFunction(FunctionType *OriginalType, FunctionType *TransformedType,
                       std::vector<unsigned> ArgumentIndexMapping)
-      : OriginalType(OriginalType),
-        TransformedType(TransformedType),
+      : OriginalType(OriginalType), TransformedType(TransformedType),
         ArgumentIndexMapping(ArgumentIndexMapping) {}
 
   // Disallow copies.
-  TransformedFunction(const TransformedFunction&) = delete;
-  TransformedFunction& operator=(const TransformedFunction&) = delete;
+  TransformedFunction(const TransformedFunction &) = delete;
+  TransformedFunction &operator=(const TransformedFunction &) = delete;
 
   // Allow moves.
-  TransformedFunction(TransformedFunction&&) = default;
-  TransformedFunction& operator=(TransformedFunction&&) = default;
+  TransformedFunction(TransformedFunction &&) = default;
+  TransformedFunction &operator=(TransformedFunction &&) = default;
 
   /// Type of the function before the transformation.
   FunctionType *OriginalType;
 
   /// Type of the function after the transformation.
   FunctionType *TransformedType;
 
   /// Transforming a function may change the position of arguments.  This
   /// member records the mapping from each argument's old position to its new
   /// position.  Argument positions are zero-indexed.  If the transformation
   /// from F to F' made the first argument of F into the third argument of F',
   /// then ArgumentIndexMapping[0] will equal 2.
   std::vector<unsigned> ArgumentIndexMapping;
 };
 
 /// Given function attributes from a call site for the original function,
 /// return function attributes appropriate for a call to the transformed
 /// function.
-AttributeList TransformFunctionAttributes(
-    const TransformedFunction& TransformedFunction,
-    LLVMContext& Ctx, AttributeList CallSiteAttrs) {
+AttributeList
+transformFunctionAttributes(const TransformedFunction &TransformedFunction,
+                            LLVMContext &Ctx, AttributeList CallSiteAttrs) {
 
   // Construct a vector of AttributeSet for each function argument.
   std::vector<llvm::AttributeSet> ArgumentAttributes(
       TransformedFunction.TransformedType->getNumParams());
 
   // Copy attributes from the parameter of the original function to the
   // transformed version.  'ArgumentIndexMapping' holds the mapping from
   // old argument position to new.
-  for (unsigned i=0, ie = TransformedFunction.ArgumentIndexMapping.size();
-       i < ie; ++i) {
-    unsigned TransformedIndex = TransformedFunction.ArgumentIndexMapping[i];
-    ArgumentAttributes[TransformedIndex] = CallSiteAttrs.getParamAttributes(i);
+  for (unsigned I = 0, IE = TransformedFunction.ArgumentIndexMapping.size();
+       I < IE; ++I) {
+    unsigned TransformedIndex = TransformedFunction.ArgumentIndexMapping[I];
+    ArgumentAttributes[TransformedIndex] = CallSiteAttrs.getParamAttributes(I);
   }
 
   // Copy annotations on varargs arguments.
-  for (unsigned i = TransformedFunction.OriginalType->getNumParams(),
-       ie = CallSiteAttrs.getNumAttrSets(); i<ie; ++i) {
-    ArgumentAttributes.push_back(CallSiteAttrs.getParamAttributes(i));
+  for (unsigned I = TransformedFunction.OriginalType->getNumParams(),
+                IE = CallSiteAttrs.getNumAttrSets();
+       I < IE; ++I) {
+    ArgumentAttributes.push_back(CallSiteAttrs.getParamAttributes(I));
   }
 
-  return AttributeList::get(
-      Ctx,
-      CallSiteAttrs.getFnAttributes(),
+  return AttributeList::get(Ctx, CallSiteAttrs.getFnAttributes(),
                             CallSiteAttrs.getRetAttributes(),
                             llvm::makeArrayRef(ArgumentAttributes));
 }
 
 class DataFlowSanitizer {
   friend struct DFSanFunction;
   friend class DFSanVisitor;
 
   enum {
     ShadowWidthBits = 16,
class DataFlowSanitizer {
   /// getShadowTy(other type) = i16
   ///
   /// Note that a shadow type is always i16 when shouldTrackFieldsAndIndices
   /// returns false.
   Type *getShadowTy(Type *OrigTy);
   /// Returns the shadow type of of V's type.
   Type *getShadowTy(Value *V);
 
-  const uint64_t kNumOfElementsInArgOrgTLS = kArgTLSSize / OriginWidthBytes;
+  const uint64_t NumOfElementsInArgOrgTLS = ArgTLSSize / OriginWidthBytes;
 
 public:
   DataFlowSanitizer(const std::vector<std::string> &ABIListFiles);
 
   bool runImpl(Module &M);
 };
 
 struct DFSanFunction {
TransformedFunction DataFlowSanitizer::getCustomFunctionType(FunctionType *T) {
   SmallVector<Type *, 4> ArgTypes;
 
   // Some parameters of the custom function being constructed are
   // parameters of T.  Record the mapping from parameters of T to
   // parameters of the custom function, so that parameter attributes
   // at call sites can be updated.
   std::vector<unsigned> ArgumentIndexMapping;
   for (unsigned I = 0, E = T->getNumParams(); I != E; ++I) {
-    Type *Param_type = T->getParamType(I);
+    Type *ParamType = T->getParamType(I);
     FunctionType *FT;
-    if (isa<PointerType>(Param_type) &&
-        (FT = dyn_cast<FunctionType>(
-             cast<PointerType>(Param_type)->getElementType()))) {
+    if (isa<PointerType>(ParamType) &&
+        (FT = dyn_cast<FunctionType>(ParamType->getPointerElementType()))) {
       ArgumentIndexMapping.push_back(ArgTypes.size());
       ArgTypes.push_back(getTrampolineFunctionType(FT)->getPointerTo());
       ArgTypes.push_back(Type::getInt8PtrTy(*Ctx));
     } else {
       ArgumentIndexMapping.push_back(ArgTypes.size());
-      ArgTypes.push_back(Param_type);
+      ArgTypes.push_back(ParamType);
     }
   }
   for (unsigned I = 0, E = T->getNumParams(); I != E; ++I)
     ArgTypes.push_back(PrimitiveShadowTy);
   if (T->isVarArg())
     ArgTypes.push_back(PrimitiveShadowPtrTy);
   Type *RetType = T->getReturnType();
   if (!RetType->isVoidTy())
if (const ConstantInt *CI = dyn_cast<ConstantInt>(V))
       return CI->isZero();
     return false;
   }
 
   return isa<ConstantAggregateZero>(V);
 }
 
 bool DataFlowSanitizer::shouldTrackOrigins() {
-  static const bool kShouldTrackOrigins =
+  static const bool ShouldTrackOrigins =
       ClTrackOrigins && getInstrumentedABI() == DataFlowSanitizer::IA_TLS &&
       ClFast16Labels;
-  return kShouldTrackOrigins;
+  return ShouldTrackOrigins;
 }
 
 bool DataFlowSanitizer::shouldTrackFieldsAndIndices() {
   return getInstrumentedABI() == DataFlowSanitizer::IA_TLS && ClFast16Labels;
 }
 
 Constant *DataFlowSanitizer::getZeroShadow(Type *OrigTy) {
   if (!shouldTrackFieldsAndIndices())

 }
 
 Type *DataFlowSanitizer::getShadowTy(Value *V) {
   return getShadowTy(V->getType());
 }
 
 bool DataFlowSanitizer::init(Module &M) {
   Triple TargetTriple(M.getTargetTriple());
-  bool IsX86_64 = TargetTriple.getArch() == Triple::x86_64;
-  bool IsMIPS64 = TargetTriple.isMIPS64();
-  bool IsAArch64 = TargetTriple.getArch() == Triple::aarch64 ||
-                   TargetTriple.getArch() == Triple::aarch64_be;
-
   const DataLayout &DL = M.getDataLayout();
 
   Mod = &M;
   Ctx = &M.getContext();
   Int8Ptr = Type::getInt8PtrTy(*Ctx);
   OriginTy = IntegerType::get(*Ctx, OriginWidthBits);
   OriginPtrTy = PointerType::getUnqual(OriginTy);
   PrimitiveShadowTy = IntegerType::get(*Ctx, ShadowWidthBits);
   PrimitiveShadowPtrTy = PointerType::getUnqual(PrimitiveShadowTy);
   IntptrTy = DL.getIntPtrType(*Ctx);
   ZeroPrimitiveShadow = ConstantInt::getSigned(PrimitiveShadowTy, 0);
   ShadowPtrMul = ConstantInt::getSigned(IntptrTy, ShadowWidthBytes);
   OriginBase = ConstantInt::get(IntptrTy, 0x200000000000LL);
   ZeroOrigin = ConstantInt::getSigned(OriginTy, 0);
-  if (IsX86_64)
+
+  switch (TargetTriple.getArch()) {
+  case Triple::x86_64:
     ShadowPtrMask = ConstantInt::getSigned(IntptrTy, ~0x700000000000LL);
-  else if (IsMIPS64)
+    break;
+  case Triple::mips64:
+  case Triple::mips64el:
     ShadowPtrMask = ConstantInt::getSigned(IntptrTy, ~0xF000000000LL);
+    break;
+  case Triple::aarch64:
+  case Triple::aarch64_be:
     // AArch64 supports multiple VMAs and the shadow mask is set at runtime.
-  else if (IsAArch64)
     DFSanRuntimeShadowMask = true;
-  else
+    break;
+  default:
     report_fatal_error("unsupported triple");
+  }
 
   Type *DFSanUnionArgs[2] = {PrimitiveShadowTy, PrimitiveShadowTy};
   DFSanUnionFnTy =
       FunctionType::get(PrimitiveShadowTy, DFSanUnionArgs, /*isVarArg=*/false);
   Type *DFSanUnionLoadArgs[2] = {PrimitiveShadowPtrTy, IntptrTy};
   DFSanUnionLoadFnTy = FunctionType::get(PrimitiveShadowTy, DFSanUnionLoadArgs,
                                          /*isVarArg=*/false);
   Type *DFSanLoadLabelAndOriginArgs[2] = {Int8Ptr, IntptrTy};
DataFlowSanitizer::buildWrapperFunction(Function *F, StringRef NewFName,
   if (F->isVarArg()) {
     NewF->removeAttributes(AttributeList::FunctionIndex,
                            AttrBuilder().addAttribute("split-stack"));
     CallInst::Create(DFSanVarargWrapperFn,
                      IRBuilder<>(BB).CreateGlobalStringPtr(F->getName()), "",
                      BB);
     new UnreachableInst(*Ctx, BB);
   } else {
-    std::vector<Value *> Args;
-    unsigned n = FT->getNumParams();
-    for (Function::arg_iterator ai = NewF->arg_begin(); n != 0; ++ai, --n)
-      Args.push_back(&*ai);
+    auto ArgIt = pointer_iterator<Argument *>(NewF->arg_begin());
+    std::vector<Value *> Args(ArgIt, ArgIt + FT->getNumParams());
+
     CallInst *CI = CallInst::Create(F, Args, "", BB);
     if (FT->getReturnType()->isVoidTy())
       ReturnInst::Create(*Ctx, BB);
     else
       ReturnInst::Create(*Ctx, CI, BB);
   }
 
   return NewF;
 }
 
 Constant *DataFlowSanitizer::getOrBuildTrampolineFunction(FunctionType *FT,
                                                           StringRef FName) {
   FunctionType *FTT = getTrampolineFunctionType(FT);
   FunctionCallee C = Mod->getOrInsertFunction(FName, FTT);
   Function *F = dyn_cast<Function>(C.getCallee());
   if (F && F->isDeclaration()) {
     F->setLinkage(GlobalValue::LinkOnceODRLinkage);
     BasicBlock *BB = BasicBlock::Create(*Ctx, "entry", F);
     std::vector<Value *> Args;
-    Function::arg_iterator AI = F->arg_begin(); ++AI;
+    Function::arg_iterator AI = F->arg_begin() + 1;
     for (unsigned N = FT->getNumParams(); N != 0; ++AI, --N)
       Args.push_back(&*AI);
     CallInst *CI = CallInst::Create(FT, &*F->arg_begin(), Args, "", BB);
-    ReturnInst *RI;
     Type *RetType = FT->getReturnType();
-    if (RetType->isVoidTy())
-      RI = ReturnInst::Create(*Ctx, BB);
-    else
-      RI = ReturnInst::Create(*Ctx, CI, BB);
+    ReturnInst *RI = RetType->isVoidTy() ? ReturnInst::Create(*Ctx, BB)
+                                         : ReturnInst::Create(*Ctx, CI, BB);
 
     // F is called by a wrapped custom function with primitive shadows. So
     // its arguments and return value need conversion.
     DFSanFunction DFSF(*this, F, /*IsNativeABI=*/true);
     Function::arg_iterator ValAI = F->arg_begin(), ShadowAI = AI;
     ++ValAI;
     for (unsigned N = FT->getNumParams(); N != 0; ++ValAI, ++ShadowAI, --N) {
       Value *Shadow =
bool DataFlowSanitizer::runImpl(Module &M) {
   if (ABIList.isIn(M, "skip"))
     return false;
 
   const unsigned InitialGlobalSize = M.global_size();
   const unsigned InitialModuleSize = M.size();
 
   bool Changed = false;
 
-  auto getOrInsertGlobal = [this, &Changed](StringRef Name,
+  auto GetOrInsertGlobal = [this, &Changed](StringRef Name,
                                             Type *Ty) -> Constant * {
     Constant *C = Mod->getOrInsertGlobal(Name, Ty);
     if (GlobalVariable *G = dyn_cast<GlobalVariable>(C)) {
       Changed |= G->getThreadLocalMode() != GlobalVariable::InitialExecTLSModel;
       G->setThreadLocalMode(GlobalVariable::InitialExecTLSModel);
     }
     return C;
   };
 
   // These globals must be kept in sync with the ones in dfsan.cpp.
-  ArgTLS = getOrInsertGlobal(
-      "__dfsan_arg_tls",
-      ArrayType::get(Type::getInt64Ty(*Ctx), kArgTLSSize / 8));
-  RetvalTLS = getOrInsertGlobal(
+  ArgTLS =
+      GetOrInsertGlobal("__dfsan_arg_tls",
+                        ArrayType::get(Type::getInt64Ty(*Ctx), ArgTLSSize / 8));
+  RetvalTLS = GetOrInsertGlobal(
       "__dfsan_retval_tls",
-      ArrayType::get(Type::getInt64Ty(*Ctx), kRetvalTLSSize / 8));
-  ArgOriginTLSTy = ArrayType::get(OriginTy, kNumOfElementsInArgOrgTLS);
-  ArgOriginTLS = getOrInsertGlobal("__dfsan_arg_origin_tls", ArgOriginTLSTy);
-  RetvalOriginTLS = getOrInsertGlobal("__dfsan_retval_origin_tls", OriginTy);
+      ArrayType::get(Type::getInt64Ty(*Ctx), RetvalTLSSize / 8));
+  ArgOriginTLSTy = ArrayType::get(OriginTy, NumOfElementsInArgOrgTLS);
+  ArgOriginTLS = GetOrInsertGlobal("__dfsan_arg_origin_tls", ArgOriginTLSTy);
+  RetvalOriginTLS = GetOrInsertGlobal("__dfsan_retval_origin_tls", OriginTy);
 
   (void)Mod->getOrInsertGlobal("__dfsan_track_origins", OriginTy, [&] {
     Changed = true;
     return new GlobalVariable(
         M, OriginTy, true, GlobalValue::WeakODRLinkage,
         ConstantInt::getSigned(OriginTy, shouldTrackOrigins()),
         "__dfsan_track_origins");
   });
 
   injectMetadataGlobals(M);
 
   ExternalShadowMask =
-      Mod->getOrInsertGlobal(kDFSanExternShadowPtrMask, IntptrTy);
+      Mod->getOrInsertGlobal(DFSanExternShadowPtrMask, IntptrTy);
 
   initializeCallbackFunctions(M);
   initializeRuntimeFunctions(M);
 
   std::vector<Function *> FnsToInstrument;
   SmallPtrSet<Function *, 2> FnsWithNativeABI;
-  for (Function &i : M)
-    if (!i.isIntrinsic() && !DFSanRuntimeFunctions.contains(&i))
-      FnsToInstrument.push_back(&i);
+  for (Function &F : M)
+    if (!F.isIntrinsic() && !DFSanRuntimeFunctions.contains(&F))
+      FnsToInstrument.push_back(&F);
 
   // Give function aliases prefixes when necessary, and build wrappers where the
   // instrumentedness is inconsistent.
-  for (Module::alias_iterator i = M.alias_begin(), e = M.alias_end(); i != e;) {
-    GlobalAlias *GA = &*i;
-    ++i;
+  for (Module::alias_iterator AI = M.alias_begin(), AE = M.alias_end();
+       AI != AE;) {
+    GlobalAlias *GA = &*AI;
+    ++AI;
     // Don't stop on weak.  We assume people aren't playing games with the
     // instrumentedness of overridden weak aliases.
-    if (auto F = dyn_cast<Function>(GA->getBaseObject())) {
-      bool GAInst = isInstrumented(GA), FInst = isInstrumented(F);
-      if (GAInst && FInst) {
-        addGlobalNamePrefix(GA);
-      } else if (GAInst != FInst) {
-        // Non-instrumented alias of an instrumented function, or vice versa.
-        // Replace the alias with a native-ABI wrapper of the aliasee.  The pass
-        // below will take care of instrumenting it.
-        Function *NewF =
-            buildWrapperFunction(F, "", GA->getLinkage(), F->getFunctionType());
-        GA->replaceAllUsesWith(ConstantExpr::getBitCast(NewF, GA->getType()));
-        NewF->takeName(GA);
-        GA->eraseFromParent();
-        FnsToInstrument.push_back(NewF);
-      }
-    }
-  }
+    auto *F = dyn_cast<Function>(GA->getBaseObject());
+    if (!F)
+      continue;
+
 
   ReadOnlyNoneAttrs.addAttribute(Attribute::ReadOnly)
       .addAttribute(Attribute::ReadNone);
 
   // First, change the ABI of every function in the module.  ABI-listed
   // functions keep their original ABI and get a wrapper function.
-  for (std::vector<Function *>::iterator i = FnsToInstrument.begin(),
-                                         e = FnsToInstrument.end();
-       i != e; ++i) {
-    Function &F = **i;
+  for (std::vector<Function *>::iterator FI = FnsToInstrument.begin(),
+                                         FE = FnsToInstrument.end();
+       FI != FE; ++FI) {
+    Function &F = **FI;
     FunctionType *FT = F.getFunctionType();
 
     bool IsZeroArgsVoidRet = (FT->getNumParams() == 0 && !FT->isVarArg() &&
                               FT->getReturnType()->isVoidTy());
 
     if (isInstrumented(&F)) {
       // Instrumented functions get a 'dfs$' prefix.  This allows us to more
       // easily identify cases of mismatching ABIs.
if (isInstrumented(&F)) {
                 BlockAddress::get(NewF, BA->getBasicBlock()));
             delete BA;
           }
         }
         F.replaceAllUsesWith(
             ConstantExpr::getBitCast(NewF, PointerType::getUnqual(FT)));
         NewF->takeName(&F);
         F.eraseFromParent();
-        *i = NewF;
+        *FI = NewF;
         addGlobalNamePrefix(NewF);
       } else {
         addGlobalNamePrefix(&F);
       }
     } else if (!IsZeroArgsVoidRet || getWrapperKind(&F) == WK_Custom) {
       // Build a wrapper function for F.  The wrapper simply calls F, and is
       // added to FnsToInstrument so that any instrumentation according to its
       // WrapperKind is done in the second pass below.
-      FunctionType *NewFT = getInstrumentedABI() == IA_Args
-                                ? getArgsFunctionType(FT)
-                                : FT;
+      FunctionType *NewFT =
+          getInstrumentedABI() == IA_Args ? getArgsFunctionType(FT) : FT;
 
       // If the function being wrapped has local linkage, then preserve the
       // function's linkage in the wrapper function.
-      GlobalValue::LinkageTypes wrapperLinkage =
-          F.hasLocalLinkage()
-              ? F.getLinkage()
+      GlobalValue::LinkageTypes WrapperLinkage =
+          F.hasLocalLinkage() ? F.getLinkage()
                               : GlobalValue::LinkOnceODRLinkage;
 
       Function *NewF = buildWrapperFunction(
           &F,
           (shouldTrackOrigins() ? std::string("dfso$") : std::string("dfsw$")) +
               std::string(F.getName()),
-          wrapperLinkage, NewFT);
+          WrapperLinkage, NewFT);
       if (getInstrumentedABI() == IA_TLS)
         NewF->removeAttributes(AttributeList::FunctionIndex, ReadOnlyNoneAttrs);
 
       Value *WrappedFnCst =
           ConstantExpr::getBitCast(NewF, PointerType::getUnqual(FT));
       F.replaceAllUsesWith(WrappedFnCst);
 
       UnwrappedFnMap[WrappedFnCst] = &F;
-      *i = NewF;
+      *FI = NewF;
 
       if (!F.isDeclaration()) {
         // This function is probably defining an interposition of an
         // uninstrumented function and hence needs to keep the original ABI.
         // But any functions it may call need to use the instrumented ABI, so
         // we instrument it in a mode which preserves the original ABI.
         FnsWithNativeABI.insert(&F);
 
         // This code needs to rebuild the iterators, as they may be invalidated
         // by the push_back, taking care that the new range does not include
         // any functions added by this code.
-        size_t N = i - FnsToInstrument.begin(),
-               Count = e - FnsToInstrument.begin();
+        size_t N = FI - FnsToInstrument.begin(),
+               Count = FE - FnsToInstrument.begin();
         FnsToInstrument.push_back(&F);
-        i = FnsToInstrument.begin() + N;
-        e = FnsToInstrument.begin() + Count;
+        FI = FnsToInstrument.begin() + N;
+        FE = FnsToInstrument.begin() + Count;
       }
-               // Hopefully, nobody will try to indirectly call a vararg
-               // function... yet.
     } else if (FT->isVarArg()) {
       UnwrappedFnMap[&F] = &F;
-      *i = nullptr;
+      *FI = nullptr;
     }
   }
 
-  for (Function *i : FnsToInstrument) {
-    if (!i || i->isDeclaration())
+  for (Function *F : FnsToInstrument) {
+    if (!F || F->isDeclaration())
       continue;
 
-    removeUnreachableBlocks(*i);
+    removeUnreachableBlocks(*F);
 
-    DFSanFunction DFSF(*this, i, FnsWithNativeABI.count(i));
+    DFSanFunction DFSF(*this, F, FnsWithNativeABI.count(F));
 
     // DFSanVisitor may create new basic blocks, which confuses df_iterator.
     // Build a copy of the list before iterating over it.
-    SmallVector<BasicBlock *, 4> BBList(depth_first(&i->getEntryBlock()));
+    SmallVector<BasicBlock *, 4> BBList(depth_first(&F->getEntryBlock()));
 
-    for (BasicBlock *i : BBList) {
-      Instruction *Inst = &i->front();
+    for (BasicBlock *BB : BBList) {
+      Instruction *Inst = &BB->front();
       while (true) {
         // DFSanVisitor may split the current basic block, changing the current
         // instruction's next pointer and moving the next instruction to the
         // tail block from which we should continue.
         Instruction *Next = Inst->getNextNode();
         // DFSanVisitor may delete Inst, so keep track of whether it was a
         // terminator.
         bool IsTerminator = Inst->isTerminator();
         if (!DFSF.SkipInsts.count(Inst))
           DFSanVisitor(DFSF).visit(Inst);
         if (IsTerminator)
           break;
         Inst = Next;
       }
     }
 
     // We will not necessarily be able to compute the shadow for every phi node
     // until we have visited every block.  Therefore, the code that handles phi
     // nodes adds them to the PHIFixups list so that they can be properly
     // handled here.
-    for (std::vector<std::pair<PHINode *, PHINode *>>::iterator
-             i = DFSF.PHIFixups.begin(),
-             e = DFSF.PHIFixups.end();
-         i != e; ++i) {
-      for (unsigned val = 0, n = i->first->getNumIncomingValues(); val != n;
-           ++val) {
-        i->second->setIncomingValue(
-            val, DFSF.getShadow(i->first->getIncomingValue(val)));
+    for (auto PHIFixup : DFSF.PHIFixups) {
+      PHINode *PN, *ShadowPN;
+      std::tie(PN, ShadowPN) = PHIFixup;
+      for (unsigned Val = 0, N = PN->getNumIncomingValues(); Val < N; ++Val) {
+        ShadowPN->setIncomingValue(Val,
+                                   DFSF.getShadow(PN->getIncomingValue(Val)));
       }
     }
 
     // -dfsan-debug-nonzero-labels will split the CFG in all kinds of crazy
     // places (i.e. instructions in basic blocks we haven't even begun visiting
     // yet).  To make our life easier, do this work in a pass after the main
     // instrumentation.
     if (ClDebugNonzeroLabels) {
if (!isa<Argument>(V) && !isa<Instruction>(V))
     return DFS.ZeroOrigin;
   Value *&Origin = ValOriginMap[V];
   if (!Origin) {
     if (Argument *A = dyn_cast<Argument>(V)) {
       if (IsNativeABI)
         return DFS.ZeroOrigin;
       switch (IA) {
       case DataFlowSanitizer::IA_TLS: {
-        if (A->getArgNo() < DFS.kNumOfElementsInArgOrgTLS) {
+        if (A->getArgNo() < DFS.NumOfElementsInArgOrgTLS) {
           Instruction *ArgOriginTLSPos = &*F->getEntryBlock().begin();
           IRBuilder<> IRB(ArgOriginTLSPos);
           Value *ArgOriginPtr = getArgOriginTLS(A->getArgNo(), IRB);
           Origin = IRB.CreateLoad(DFS.OriginTy, ArgOriginPtr);
         } else {
           // Overflow
           Origin = DFS.ZeroOrigin;
         }
for (auto &FArg : F->args()) {
     if (!FArg.getType()->isSized()) {
       if (A == &FArg)
         break;
       continue;
     }
 
     unsigned Size = DL.getTypeAllocSize(DFS.getShadowTy(&FArg));
     if (A != &FArg) {
-      ArgOffset += alignTo(Size, kShadowTLSAlignment);
-      if (ArgOffset > kArgTLSSize)
+      ArgOffset += alignTo(Size, ShadowTLSAlignment);
+      if (ArgOffset > ArgTLSSize)
         break; // ArgTLS overflows, uses a zero shadow.
       continue;
     }
 
-    if (ArgOffset + Size > kArgTLSSize)
+    if (ArgOffset + Size > ArgTLSSize)
       break; // ArgTLS overflows, uses a zero shadow.
 
     Instruction *ArgTLSPos = &*F->getEntryBlock().begin();
     IRBuilder<> IRB(ArgTLSPos);
     Value *ArgShadowPtr = getArgTLS(FArg.getType(), ArgOffset, IRB);
     return IRB.CreateAlignedLoad(DFS.getShadowTy(&FArg), ArgShadowPtr,
-                                 kShadowTLSAlignment);
+                                 ShadowTLSAlignment);
   }
 
   return DFS.getZeroShadow(A);
 }
 
 Value *DFSanFunction::getShadow(Value *V) {
   if (!isa<Argument>(V) && !isa<Instruction>(V))
     return DFS.getZeroShadow(V);
   Value *&Shadow = ValShadowMap[V];
   if (!Shadow) {
     if (Argument *A = dyn_cast<Argument>(V)) {
       if (IsNativeABI)
         return DFS.getZeroShadow(V);
       switch (IA) {
       case DataFlowSanitizer::IA_TLS: {
         Shadow = getShadowForTLSArgument(A);
         break;
       }
       case DataFlowSanitizer::IA_Args: {
         unsigned ArgIdx = A->getArgNo() + F->arg_size() / 2;
-        Function::arg_iterator i = F->arg_begin();
-        while (ArgIdx--)
-          ++i;
-        Shadow = &*i;
+        Function::arg_iterator Arg = F->arg_begin();
+        std::advance(Arg, ArgIdx);
+        Shadow = &*Arg;
         assert(Shadow->getType() == DFS.PrimitiveShadowTy);
         break;
       }
       }
       NonZeroChecks.push_back(Shadow);
     } else {
       Shadow = DFS.getZeroShadow(V);
     }
DataFlowSanitizer::getShadowOriginAddress(Value *Addr, Align InstAlignment,
   Value *ShadowPtr = IRB.CreateIntToPtr(
       IRB.CreateMul(ShadowOffset, ShadowPtrMul), PrimitiveShadowPtrTy);
   Value *OriginPtr = nullptr;
   if (shouldTrackOrigins()) {
     Value *OriginLong = IRB.CreateAdd(ShadowOffset, OriginBase);
     const Align Alignment = llvm::assumeAligned(InstAlignment.value());
     // When alignment is >= 4, Addr must be aligned to 4, otherwise it is UB.
     // So Mask is unnecessary.
-    if (Alignment < kMinOriginAlignment) {
-      uint64_t Mask = kMinOriginAlignment.value() - 1;
+    if (Alignment < MinOriginAlignment) {
+      uint64_t Mask = MinOriginAlignment.value() - 1;
       OriginLong = IRB.CreateAnd(OriginLong, ConstantInt::get(IntptrTy, ~Mask));
     }
     OriginPtr = IRB.CreateIntToPtr(OriginLong, OriginPtrTy);
   }
   return {ShadowPtr, OriginPtr};
 }
 */
 Value *DataFlowSanitizer::getShadowAddress(Value *Addr, Instruction *Pos) {
if (V1 == V2)
     return collapseToPrimitiveShadow(V1, Pos);
 
   auto V1Elems = ShadowElements.find(V1);
   auto V2Elems = ShadowElements.find(V2);
   if (V1Elems != ShadowElements.end() && V2Elems != ShadowElements.end()) {
     if (std::includes(V1Elems->second.begin(), V1Elems->second.end(),
                       V2Elems->second.begin(), V2Elems->second.end())) {
       return collapseToPrimitiveShadow(V1, Pos);
-    } else if (std::includes(V2Elems->second.begin(), V2Elems->second.end(),
-                             V1Elems->second.begin(), V1Elems->second.end())) {
+    }
+    if (std::includes(V2Elems->second.begin(), V2Elems->second.end(),
       return collapseToPrimitiveShadow(V2, Pos);
     }
   } else if (V1Elems != ShadowElements.end()) {
     if (V1Elems->second.count(V2))
       return collapseToPrimitiveShadow(V1, Pos);
   } else if (V2Elems != ShadowElements.end()) {
     if (V2Elems->second.count(V1))
       return collapseToPrimitiveShadow(V2, Pos);

 // A convenience function which folds the shadows of each of the operands
 // of the provided instruction Inst, inserting the IR before Inst.  Returns
 // the computed union Value.
 Value *DFSanFunction::combineOperandShadows(Instruction *Inst) {
   if (Inst->getNumOperands() == 0)
     return DFS.getZeroShadow(Inst);
 
   Value *Shadow = getShadow(Inst->getOperand(0));
-  for (unsigned i = 1, n = Inst->getNumOperands(); i != n; ++i) {
-    Shadow = combineShadows(Shadow, getShadow(Inst->getOperand(i)), Inst);
-  }
+  for (unsigned I = 1, N = Inst->getNumOperands(); I < N; ++I)
+    Shadow = combineShadows(Shadow, getShadow(Inst->getOperand(I)), Inst);
+
   return expandFromPrimitiveShadow(Inst->getType(), Shadow, Inst);
 }
 
 void DFSanVisitor::visitInstOperands(Instruction &I) {
   Value *CombinedShadow = DFSF.combineOperandShadows(&I);
   DFSF.setShadow(&I, CombinedShadow);
   visitInstOperandOrigins(I);
 }

 }
 
 // Generates IR to load shadow corresponding to bytes [Addr, Addr+Size), where
 // Addr has alignment Align, and take the union of each of those shadows. The
 // returned shadow always has primitive type.
 Value *DFSanFunction::loadShadow(Value *Addr, uint64_t Size, uint64_t Align,
                                  Instruction *Pos) {
   if (AllocaInst *AI = dyn_cast<AllocaInst>(Addr)) {
-    const auto i = AllocaShadowMap.find(AI);
-    if (i != AllocaShadowMap.end()) {
+    const auto I = AllocaShadowMap.find(AI);
+    if (I != AllocaShadowMap.end()) {
       IRBuilder<> IRB(Pos);
-      return IRB.CreateLoad(DFS.PrimitiveShadowTy, i->second);
+      return IRB.CreateLoad(DFS.PrimitiveShadowTy, I->second);
     }
   }
 
   const llvm::Align ShadowAlign(Align * DFS.ShadowWidthBytes);
   SmallVector<const Value *, 2> Objs;
   getUnderlyingObjects(Addr, Objs);
   bool AllConstants = true;
   for (const Value *Obj : Objs) {
void DFSanFunction::storeZeroPrimitiveShadow(Value *Addr, uint64_t Size,
   // Do not write origins for 0 shadows because we do not trace origins for
   // untainted sinks.
 }
 void DFSanFunction::storePrimitiveShadow(Value *Addr, uint64_t Size,
                                          Align Alignment,
                                          Value *PrimitiveShadow,
                                          Instruction *Pos) {
   if (AllocaInst *AI = dyn_cast<AllocaInst>(Addr)) {
-    const auto i = AllocaShadowMap.find(AI);
-    if (i != AllocaShadowMap.end()) {
+    const auto I = AllocaShadowMap.find(AI);
+    if (I != AllocaShadowMap.end()) {
       IRBuilder<> IRB(Pos);
-      IRB.CreateStore(PrimitiveShadow, i->second);
+      IRB.CreateStore(PrimitiveShadow, I->second);
       return;
     }
   }
 
   const Align ShadowAlign(Alignment.value() * DFS.ShadowWidthBytes);
   if (DFS.isZeroShadow(PrimitiveShadow)) {
     storeZeroPrimitiveShadow(Addr, Size, ShadowAlign, Pos);
     return;
   }
 
   IRBuilder<> IRB(Pos);
   Value *ShadowAddr = DFS.getShadowAddress(Addr, Pos);
   const unsigned ShadowVecSize = 128 / DFS.ShadowWidthBits;
   uint64_t Offset = 0;
   if (Size >= ShadowVecSize) {
     auto *ShadowVecTy =
         FixedVectorType::get(DFS.PrimitiveShadowTy, ShadowVecSize);
     Value *ShadowVec = UndefValue::get(ShadowVecTy);
-    for (unsigned i = 0; i != ShadowVecSize; ++i) {
+    for (unsigned I = 0; I != ShadowVecSize; ++I) {
       ShadowVec = IRB.CreateInsertElement(
           ShadowVec, PrimitiveShadow,
-          ConstantInt::get(Type::getInt32Ty(*DFS.Ctx), i));
+          ConstantInt::get(Type::getInt32Ty(*DFS.Ctx), I));
     }
     Value *ShadowVecAddr =
         IRB.CreateBitCast(ShadowAddr, PointerType::getUnqual(ShadowVecTy));
     do {
       Value *CurShadowVecAddr =
           IRB.CreateConstGEP1_32(ShadowVecTy, ShadowVecAddr, Offset);
       IRB.CreateAlignedStore(ShadowVec, CurShadowVecAddr, ShadowAlign);
       Size -= ShadowVecSize;
void DFSanVisitor::visitReturnInst(ReturnInst &RI) {
   if (!DFSF.IsNativeABI && RI.getReturnValue()) {
     switch (DFSF.IA) {
     case DataFlowSanitizer::IA_TLS: {
       Value *S = DFSF.getShadow(RI.getReturnValue());
       IRBuilder<> IRB(&RI);
       Type *RT = DFSF.F->getFunctionType()->getReturnType();
       unsigned Size =
           getDataLayout().getTypeAllocSize(DFSF.DFS.getShadowTy(RT));
-      if (Size <= kRetvalTLSSize) {
+      if (Size <= RetvalTLSSize) {
         // If the size overflows, stores nothing. At callsite, oversized return
         // shadows are set to zero.
         IRB.CreateAlignedStore(S, DFSF.getRetvalTLS(RT, IRB),
-                               kShadowTLSAlignment);
+                               ShadowTLSAlignment);
       }
       if (DFSF.DFS.shouldTrackOrigins()) {
         Value *O = DFSF.getOrigin(RI.getReturnValue());
         IRB.CreateStore(O, DFSF.getRetvalOriginTLS());
       }
       break;
     }
     case DataFlowSanitizer::IA_Args: {
if (Function *CustomFn = dyn_cast<Function>(CustomF.getCallee())) {
                                    DFSF.DFS.ReadOnlyNoneAttrs);
       }
     }
 
     std::vector<Value *> Args;
 
     // Adds non-variable arguments.
     auto *I = CB.arg_begin();
-    for (unsigned n = FT->getNumParams(); n != 0; ++I, --n) {
+    for (unsigned N = FT->getNumParams(); N != 0; ++I, --N) {
       Type *T = (*I)->getType();
       FunctionType *ParamFT;
       if (isa<PointerType>(T) &&
-          (ParamFT = dyn_cast<FunctionType>(
-               cast<PointerType>(T)->getElementType()))) {
+          (ParamFT = dyn_cast<FunctionType>(T->getPointerElementType()))) {
         std::string TName = "dfst";
-        TName += utostr(FT->getNumParams() - n);
+        TName += utostr(FT->getNumParams() - N);
         TName += "$";
         TName += F.getName();
         Constant *T = DFSF.DFS.getOrBuildTrampolineFunction(ParamFT, TName);
         Args.push_back(T);
         Args.push_back(
             IRB.CreateBitCast(*I, Type::getInt8PtrTy(*DFSF.DFS.Ctx)));
       } else {
         Args.push_back(*I);
case DataFlowSanitizer::WK_Custom:
     if (ShouldTrackOrigins)
       addOriginArguments(F, CB, Args, IRB);
 
     // Adds variable arguments.
     append_range(Args, drop_begin(CB.args(), FT->getNumParams()));
 
     CallInst *CustomCI = IRB.CreateCall(CustomF, Args);
     CustomCI->setCallingConv(CI->getCallingConv());
-    CustomCI->setAttributes(TransformFunctionAttributes(
+    CustomCI->setAttributes(transformFunctionAttributes(
         CustomFn, CI->getContext(), CI->getAttributes()));
 
     // Update the parameter attributes of the custom call instruction to
     // zero extend the shadow parameters. This is required for targets
     // which consider PrimitiveShadowTy an illegal type.
     for (unsigned N = 0; N < FT->getNumParams(); N++) {
       const unsigned ArgNo = ShadowArgStart + N;
       if (CustomCI->getArgOperand(ArgNo)->getType() ==
if ((F && F->isIntrinsic()) || CB.isInlineAsm()) {
     return;
   }
 
   // Calls to this function are synthesized in wrappers, and we shouldn't
   // instrument them.
   if (F == DFSF.DFS.DFSanVarargWrapperFn.getCallee()->stripPointerCasts())
     return;
 
-  DenseMap<Value *, Function *>::iterator i =
+  DenseMap<Value *, Function *>::iterator UnwrappedFnIt =
       DFSF.DFS.UnwrappedFnMap.find(CB.getCalledOperand());
-  if (i != DFSF.DFS.UnwrappedFnMap.end())
-    if (visitWrappedCallBase(*i->second, CB))
+  if (UnwrappedFnIt != DFSF.DFS.UnwrappedFnMap.end())
+    if (visitWrappedCallBase(*UnwrappedFnIt->second, CB))
       return;
 
   IRBuilder<> IRB(&CB);
 
   const bool ShouldTrackOrigins = DFSF.DFS.shouldTrackOrigins();
   FunctionType *FT = CB.getFunctionType();
   if (DFSF.DFS.getInstrumentedABI() == DataFlowSanitizer::IA_TLS) {
     // Stores argument shadows.
     unsigned ArgOffset = 0;
     const DataLayout &DL = getDataLayout();
     for (unsigned I = 0, N = FT->getNumParams(); I != N; ++I) {
       if (ShouldTrackOrigins) {
         // Ignore overflowed origins
         Value *ArgShadow = DFSF.getShadow(CB.getArgOperand(I));
-        if (I < DFSF.DFS.kNumOfElementsInArgOrgTLS &&
+        if (I < DFSF.DFS.NumOfElementsInArgOrgTLS &&
             !DFSF.DFS.isZeroShadow(ArgShadow))
           IRB.CreateStore(DFSF.getOrigin(CB.getArgOperand(I)),
                           DFSF.getArgOriginTLS(I, IRB));
       }
 
       unsigned Size =
           DL.getTypeAllocSize(DFSF.DFS.getShadowTy(FT->getParamType(I)));
       // Stop storing if arguments' size overflows. Inside a function, arguments
       // after overflow have zero shadow values.
-      if (ArgOffset + Size > kArgTLSSize)
+      if (ArgOffset + Size > ArgTLSSize)
         break;
       IRB.CreateAlignedStore(
           DFSF.getShadow(CB.getArgOperand(I)),
           DFSF.getArgTLS(FT->getParamType(I), ArgOffset, IRB),
-          kShadowTLSAlignment);
-      ArgOffset += alignTo(Size, kShadowTLSAlignment);
+          ShadowTLSAlignment);
+      ArgOffset += alignTo(Size, ShadowTLSAlignment);
     }
   }
 
   Instruction *Next = nullptr;
   if (!CB.getType()->isVoidTy()) {
     if (InvokeInst *II = dyn_cast<InvokeInst>(&CB)) {
       if (II->getNormalDest()->getSinglePredecessor()) {
         Next = &II->getNormalDest()->front();
       } else {
         BasicBlock *NewBB =
             SplitEdge(II->getParent(), II->getNormalDest(), &DFSF.DT);
         Next = &NewBB->front();
       }
     } else {
       assert(CB.getIterator() != CB.getParent()->end());
       Next = CB.getNextNode();
     }
 
     if (DFSF.DFS.getInstrumentedABI() == DataFlowSanitizer::IA_TLS) {
       // Loads the return value shadow.
       IRBuilder<> NextIRB(Next);
       const DataLayout &DL = getDataLayout();
       unsigned Size = DL.getTypeAllocSize(DFSF.DFS.getShadowTy(&CB));
-      if (Size > kRetvalTLSSize) {
+      if (Size > RetvalTLSSize) {
         // Set overflowed return shadow to be zero.
         DFSF.setShadow(&CB, DFSF.DFS.getZeroShadow(&CB));
       } else {
         LoadInst *LI = NextIRB.CreateAlignedLoad(
             DFSF.DFS.getShadowTy(&CB), DFSF.getRetvalTLS(CB.getType(), NextIRB),
-            kShadowTLSAlignment, "_dfsret");
+            ShadowTLSAlignment, "_dfsret");
         DFSF.SkipInsts.insert(LI);
         DFSF.setShadow(&CB, LI);
         DFSF.NonZeroChecks.push_back(LI);
       }
 
       if (ShouldTrackOrigins) {
         LoadInst *LI = NextIRB.CreateLoad(
             DFSF.DFS.OriginTy, DFSF.getRetvalOriginTLS(), "_dfsret_o");
         DFSF.SkipInsts.insert(LI);
         DFSF.setOrigin(&CB, LI);
       }
     }
   }
 
   // Do all instrumentation for IA_Args down here to defer tampering with the
   // CFG in a way that SplitEdge may be able to detect.
   if (DFSF.DFS.getInstrumentedABI() == DataFlowSanitizer::IA_Args) {
     FunctionType *NewFT = DFSF.DFS.getArgsFunctionType(FT);
     Value *Func =
         IRB.CreateBitCast(CB.getCalledOperand(), PointerType::getUnqual(NewFT));
-    std::vector<Value *> Args;
 
-    auto i = CB.arg_begin(), E = CB.arg_end();
-    for (unsigned n = FT->getNumParams(); n != 0; ++i, --n)
-      Args.push_back(*i);
-
-    i = CB.arg_begin();
-    for (unsigned n = FT->getNumParams(); n != 0; ++i, --n)
-      Args.push_back(DFSF.getShadow(*i));
+    const unsigned NumParams = FT->getNumParams();
+
+    // Copy original arguments.
+    auto *ArgIt = CB.arg_begin(), *ArgEnd = CB.arg_end();
+    std::vector<Value *> Args(NumParams);
+    std::copy_n(ArgIt, NumParams, Args.begin());
+
+    // Add shadow arguments by transforming original arguments.
+    std::generate_n(std::back_inserter(Args), NumParams,
+                    [&]() { return DFSF.getShadow(*ArgIt++); });
 
     if (FT->isVarArg()) {
-      unsigned VarArgSize = CB.arg_size() - FT->getNumParams();
+      unsigned VarArgSize = CB.arg_size() - NumParams;
       ArrayType *VarArgArrayTy =
           ArrayType::get(DFSF.DFS.PrimitiveShadowTy, VarArgSize);
       AllocaInst *VarArgShadow =
           new AllocaInst(VarArgArrayTy, getDataLayout().getAllocaAddrSpace(),
                          "", &DFSF.F->getEntryBlock().front());
       Args.push_back(IRB.CreateConstGEP2_32(VarArgArrayTy, VarArgShadow, 0, 0));
-      for (unsigned n = 0; i != E; ++i, ++n) {
+
+      // Copy remaining var args.
+      unsigned GepIndex = 0;
+      std::for_each(ArgIt, ArgEnd, [&](Value *Arg) {
         IRB.CreateStore(
-            DFSF.getShadow(*i),
-            IRB.CreateConstGEP2_32(VarArgArrayTy, VarArgShadow, 0, n));
-        Args.push_back(*i);
-      }
+            DFSF.getShadow(Arg),
+            IRB.CreateConstGEP2_32(VarArgArrayTy, VarArgShadow, 0, GepIndex++));
+        Args.push_back(Arg);
+      });
     }
 
     CallBase *NewCB;
     if (InvokeInst *II = dyn_cast<InvokeInst>(&CB)) {
       NewCB = IRB.CreateInvoke(NewFT, Func, II->getNormalDest(),
                                II->getUnwindDest(), Args);
     } else {
       NewCB = IRB.CreateCall(NewFT, Func, Args);

 
 void DFSanVisitor::visitPHINode(PHINode &PN) {
   Type *ShadowTy = DFSF.DFS.getShadowTy(&PN);
   PHINode *ShadowPN =
       PHINode::Create(ShadowTy, PN.getNumIncomingValues(), "", &PN);
 
   // Give the shadow phi node valid predecessors to fool SplitEdge into working.
   Value *UndefShadow = UndefValue::get(ShadowTy);
-  for (PHINode::block_iterator i = PN.block_begin(), e = PN.block_end(); i != e;
-       ++i) {
-    ShadowPN->addIncoming(UndefShadow, *i);
-  }
+  for (BasicBlock *BB : PN.blocks())
+    ShadowPN->addIncoming(UndefShadow, BB);
 
   DFSF.PHIFixups.push_back(std::make_pair(&PN, ShadowPN));
   DFSF.setShadow(&PN, ShadowPN);
 }
 
 namespace {
 class DataFlowSanitizerLegacyPass : public ModulePass {
 private: