[PATCH] D20541: [esan|cfrag] Create the cfrag variable for the runtime

[Qin Zhao via llvm-commits]

zhaoqin added a comment.

In http://reviews.llvm.org/D20541#439070, @filcab wrote:

> Please add a test for the cache fragmentation tool's global variable.


http://reviews.llvm.org/D20542 is the test for the arg passed to runtime.
Are you thinking of a different test in llvm?


================
Comment at: lib/Transforms/Instrumentation/EfficiencySanitizer.cpp:207
@@ +206,3 @@
+  // struct CacheFragTy {
+  //   int         ToolType;
+  //   const char *UnitName;
----------------
filcab wrote:
> I don't really see a reason to include the tool type here. We're already emitting it in the call to `__esan_init`.
One of the reason is to potentially support multiple tools in the same build/run, where each tool may need separate information.
It is not in any near future plan, so no code refactoring for it, i.e., remove ToolType from __esan_init.
But adding code with that in mind would help.


================
Comment at: lib/Transforms/Instrumentation/EfficiencySanitizer.cpp:226
@@ +225,3 @@
+                          nullptr));
+  return CacheFragGV;
+}
----------------
filcab wrote:
> I'd rather have a more abstract version of this that just creates an anonymous structure, and then each tool knows what to do with it.
> A bit like what UBSan does in clang.
> 
> Check out clang's `lib/CodeGen/CGExpr.cpp`
> Create the array and then call EmitCheck (line 585):
> ```
>       llvm::Constant *StaticData[] = {
>         EmitCheckSourceLocation(E->getLocStart()),
>         EmitCheckTypeDescriptor(CalleeType)
>       };
>       EmitCheck(Checks, "type_mismatch", StaticData, Ptr);
> ```
> In EmitCheck, we have a common way to emit the static args:
> 
> ```
>   // Emit handler arguments and create handler function type.
>   if (!StaticArgs.empty()) {
>     llvm::Constant *Info = llvm::ConstantStruct::getAnon(StaticArgs);
>     auto *InfoPtr =
>         new llvm::GlobalVariable(CGM.getModule(), Info->getType(), false,
>                                  llvm::GlobalVariable::PrivateLinkage, Info);
>     InfoPtr->setUnnamedAddr(true);
>     CGM.getSanitizerMetadata()->disableSanitizerForGlobal(InfoPtr);
>     Args.push_back(Builder.CreateBitCast(InfoPtr, Int8PtrTy));
>     ArgTypes.push_back(Int8PtrTy);
>   }
> ```
> 
> That way, each tool can just create the ArrayRef and pass it to a general `createToolGV` or something.
> 
> `__esan_init` would get the two args: `ToolType` and a `void*` which is tool specific.
Thanks for the pointer.
It is nice to know there are flexible ways to create structs. Working on it.


http://reviews.llvm.org/D20541