Asan Initialization Order Checking - Clang

Need clang to emit metadata identifying dynamically initialized globals.

[kcc1, 2012-07-27 07:20:55]

This patch looks good to me.
I don't think my review is enough outside of asan sources, so please send
this patch to cfe-commits

--kcc

On Fri, Jul 27, 2012 at 3:43 AM, <reidw@google.com> wrote:

> Reviewers: samsonov, kcc1,
>
> Description:
> Need clang to emit metadata identifying dynamically initialized globals.
>
> Please review this at
http://codereview.appspot.com/**6440051/<http://codereview.appspot.com/6440051/>
>
> Affected files:
>   lib/CodeGen/CGDeclCXX.cpp
>
>
> Index: lib/CodeGen/CGDeclCXX.cpp
> ==============================**==============================**=======
> --- lib/CodeGen/CGDeclCXX.cpp   (revision 160832)
> +++ lib/CodeGen/CGDeclCXX.cpp   (working copy)
> @@ -244,6 +244,17 @@
>    llvm::Function *Fn =
>      CreateGlobalInitOrDestructFunc**tion(*this, FTy,
> "__cxx_global_var_init");
>
> +  // If we are compiling with ASan, add metadata indicating dynamically
> +  // initialized globals.
> +  if (LangOpts.AddressSanitizer) {
> +    llvm::Module *M = Addr->getParent();
> +    llvm::NamedMDNode *DynamicInitializers
> +      = M->getOrInsertNamedMetadata("**dynamically_initialized_**
> globals");
> +    llvm::Value *GlobalsToAdd[1] = {Addr};
> +    llvm::MDNode *ThisGlobal = llvm::MDNode::get(M->**getContext(),
> GlobalsToAdd);
> +    DynamicInitializers->**addOperand(ThisGlobal);
> +  }
> +
>    CodeGenFunction(*this).**GenerateCXXGlobalVarDeclInitFu**nc(Fn, D,
> Addr,
>                                                            PerformInit);
>
>
>
>

[glider1, 2012-08-16 09:00:25]

Two minor comments. My review isn't sufficient for Clang code either.

https://codereview.appspot.com/6440051/diff/2001/lib/CodeGen/CodeGenModule.cpp
File lib/CodeGen/CodeGenModule.cpp (right):

https://codereview.appspot.com/6440051/diff/2001/lib/CodeGen/CodeGenModule.cp...
lib/CodeGen/CodeGenModule.cpp:1690: =
M->getOrInsertNamedMetadata("dynamically_initialized_globals");
Please put the "=" sign on the previous line.

https://codereview.appspot.com/6440051/diff/2001/lib/CodeGen/CodeGenModule.cp...
lib/CodeGen/CodeGenModule.cpp:1691: llvm::Value *GlobalToAdd[] = {GV};
I'd put spaces around GV, but this is up to you.

[Richard Smith, 2012-08-16 20:03:31]

LGTM, with a couple more trivial things.

http://codereview.appspot.com/6440051/diff/2001/lib/CodeGen/CodeGenModule.cpp
File lib/CodeGen/CodeGenModule.cpp (right):

http://codereview.appspot.com/6440051/diff/2001/lib/CodeGen/CodeGenModule.cpp...
lib/CodeGen/CodeGenModule.cpp:1688: llvm::Module *M = GV->getParent();
You can use getModule() for this.

http://codereview.appspot.com/6440051/diff/2001/lib/CodeGen/CodeGenModule.cpp...
lib/CodeGen/CodeGenModule.cpp:1692: llvm::MDNode *ThisGlobal =
llvm::MDNode::get(M->getContext(), GlobalToAdd);
CodeGen tends to use VMContext or getLLVMContext() instead of M->getContext().