[llvm-commits] [llvm] r157505 - /llvm/trunk/lib/Transforms/Instrumentation/GCOVProfiling.cpp

[Nick Lewycky]

Bill Wendling wrote:
> Author: void
> Date: Fri May 25 18:55:00 2012
> New Revision: 157505
>
> URL: http://llvm.org/viewvc/llvm-project?rev=157505&view=rev
> Log:
> The llvm_gcda_increment_indirect_counter function writes to the arguments that
> are passed in. However, those arguments may be in a write-protected area, as far
> as the runtime library is concerned.

I don't understand how this can happen. The arguments are chosen by the 
instrumentation pass and they point to writable globals.

  For instance, the data could be placed into
> a 'linkedit' section, which isn't writable.

I'm not familiar with 'linkedit'. Is the problem here an issue of the 
linkage type? Or is the global getting marked isConstant=true somehow?

  Emit the code from
> llvm_gcda_increment_indirect_counter directly into the function instead.

That can't be right, you're just inlining the function. The arguments 
would still be in a read-only area. What's going on?

Nick

>
> Note: The code for this is ugly, and can lead to bloat. We should look into
> simplifying this code instead of having all of these branches.
>
> <rdar://problem/11181370>
>
> Modified:
>      llvm/trunk/lib/Transforms/Instrumentation/GCOVProfiling.cpp
>
> Modified: llvm/trunk/lib/Transforms/Instrumentation/GCOVProfiling.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/GCOVProfiling.cpp?rev=157505&r1=157504&r2=157505&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Instrumentation/GCOVProfiling.cpp (original)
> +++ llvm/trunk/lib/Transforms/Instrumentation/GCOVProfiling.cpp Fri May 25 18:55:00 2012
> @@ -70,7 +70,9 @@
>
>       // Get pointers to the functions in the runtime library.
>       Constant *getStartFileFunc();
> -    Constant *getIncrementIndirectCounterFunc();
> +    void incrementIndirectCounter(IRBuilder<>  &Builder, BasicBlock *Exit,
> +                                  GlobalVariable *EdgeState,
> +                                  Value *CounterPtrArray);
>       Constant *getEmitFunctionFunc();
>       Constant *getEmitArcsFunc();
>       Constant *getEndFileFunc();
> @@ -501,16 +503,17 @@
>           }
>           for (int i = 0, e = ComplexEdgeSuccs.size(); i != e; ++i) {
>             // call runtime to perform increment
> -          BasicBlock::iterator InsertPt =
> -            ComplexEdgeSuccs[i+1]->getFirstInsertionPt();
> +          BasicBlock *BB = ComplexEdgeSuccs[i+1];
> +          BasicBlock::iterator InsertPt = BB->getFirstInsertionPt();
> +          BasicBlock *Split = BB->splitBasicBlock(InsertPt);
> +          InsertPt = BB->getFirstInsertionPt();
>             IRBuilder<>  Builder(InsertPt);
>             Value *CounterPtrArray =
>               Builder.CreateConstInBoundsGEP2_64(EdgeTable, 0,
>                                                  i * ComplexEdgePreds.size());
> -          Builder.CreateCall2(getIncrementIndirectCounterFunc(),
> -                              EdgeState, CounterPtrArray);
> -          // clear the predecessor number
> -          Builder.CreateStore(ConstantInt::get(Int32Ty, 0xffffffff), EdgeState);
> +
> +          // Build code to increment the counter.
> +          incrementIndirectCounter(Builder, Split, EdgeState, CounterPtrArray);
>           }
>         }
>       }
> @@ -573,14 +576,52 @@
>     return M->getOrInsertFunction("llvm_gcda_start_file", FTy);
>   }
>
> -Constant *GCOVProfiler::getIncrementIndirectCounterFunc() {
> -  Type *Args[] = {
> -    Type::getInt32PtrTy(*Ctx),                  // uint32_t *predecessor
> -    Type::getInt64PtrTy(*Ctx)->getPointerTo(),  // uint64_t **state_table_row
> -  };
> -  FunctionType *FTy = FunctionType::get(Type::getVoidTy(*Ctx),
> -                                              Args, false);
> -  return M->getOrInsertFunction("llvm_gcda_increment_indirect_counter", FTy);
> +/// incrementIndirectCounter - Emit code that increments the indirect
> +/// counter. The code is meant to copy the llvm_gcda_increment_indirect_counter
> +/// function, but because it's inlined into the function, we don't have to worry
> +/// about the runtime library possibly writing into a protected area.
> +void GCOVProfiler::incrementIndirectCounter(IRBuilder<>  &Builder,
> +                                            BasicBlock *Exit,
> +                                            GlobalVariable *EdgeState,
> +                                            Value *CounterPtrArray) {
> +  Type *Int64Ty = Type::getInt64Ty(*Ctx);
> +  ConstantInt *NegOne = ConstantInt::get(Type::getInt32Ty(*Ctx), 0xffffffff);
> +
> +  // Create exiting blocks.
> +  BasicBlock *InsBB = Builder.GetInsertBlock();
> +  Function *Fn = InsBB->getParent();
> +  BasicBlock *PredNotNegOne = BasicBlock::Create(*Ctx, "", Fn, Exit);
> +  BasicBlock *CounterEnd = BasicBlock::Create(*Ctx, "", Fn, Exit);
> +
> +  // uint32_t pred = *EdgeState;
> +  // if (pred == 0xffffffff) return;
> +  Value *Pred = Builder.CreateLoad(EdgeState, "predecessor");
> +  Value *Cond = Builder.CreateICmpEQ(Pred, NegOne);
> +  InsBB->getTerminator()->eraseFromParent();
> +  BranchInst::Create(Exit, PredNotNegOne, Cond, InsBB);
> +
> +  Builder.SetInsertPoint(PredNotNegOne);
> +
> +  // uint64_t *counter = CounterPtrArray[pred];
> +  // if (!counter) return;
> +  Value *ZExtPred = Builder.CreateZExt(Pred, Int64Ty);
> +  Value *GEP = Builder.CreateGEP(CounterPtrArray, ZExtPred);
> +  Value *Counter = Builder.CreateLoad(GEP, "counter");
> +  Cond = Builder.CreateICmpEQ(Counter,
> +                              Constant::getNullValue(Type::getInt64PtrTy(*Ctx, 0)));
> +  Builder.CreateCondBr(Cond, Exit, CounterEnd);
> +
> +  Builder.SetInsertPoint(CounterEnd);
> +
> +  // ++*counter;
> +  Value *Add = Builder.CreateAdd(Builder.CreateLoad(Counter),
> +                                 ConstantInt::get(Int64Ty, 1));
> +  Builder.CreateStore(Add, Counter);
> +  Builder.CreateBr(Exit);
> +
> +  // Clear the predecessor number
> +  Builder.SetInsertPoint(Exit->getFirstInsertionPt());
> +  Builder.CreateStore(NegOne, EdgeState);
>   }
>
>   Constant *GCOVProfiler::getEmitFunctionFunc() {
> @@ -588,8 +629,7 @@
>       Type::getInt32Ty(*Ctx),    // uint32_t ident
>       Type::getInt8PtrTy(*Ctx),  // const char *function_name
>     };
> -  FunctionType *FTy = FunctionType::get(Type::getVoidTy(*Ctx),
> -                                              Args, false);
> +  FunctionType *FTy = FunctionType::get(Type::getVoidTy(*Ctx), Args, false);
>     return M->getOrInsertFunction("llvm_gcda_emit_function", FTy);
>   }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>