[PATCH] Don't localize globals that may serve as leak checker roots.

[Reid Kleckner]

I missed any earlier discussion about this.  Don't we want to keep this
optimization, even if it's a real corner case that fires once in a blue
moon?  isLeakCheckerRoot() doesn't check any flags to see if any of the
sanitizers are on.


On Wed, Sep 25, 2013 at 4:32 AM, Alexey Samsonov <samsonov at google.com>wrote:

> Hi nicholas,
>
> This change prevents GlobalOpt from turning globals into
> stack-allocated locals, if these variables may serve as a leak checker
> root. For instance, transforming
>   Foo *x;
>   int main() { x = new Foo(); }
> into
>   int main() { Foo *x = new Foo(); }
> will make LeakSanitizer treat "x" as definitely leaked, which is
> undesirable.
>
> http://llvm-reviews.chandlerc.com/D1754
>
> Files:
>   test/Transforms/GlobalOpt/metadata.ll
>   test/Transforms/GlobalOpt/dont-localize-pointer.ll
>   lib/Transforms/IPO/GlobalOpt.cpp
>
> Index: test/Transforms/GlobalOpt/metadata.ll
> ===================================================================
> --- test/Transforms/GlobalOpt/metadata.ll
> +++ test/Transforms/GlobalOpt/metadata.ll
> @@ -3,24 +3,24 @@
>  ; PR6112 - When globalopt does RAUW(@G, %G), the metadata reference
> should drop
>  ; to null.  Function local metadata that references @G from a different
> function
>  ; to that containing %G should likewise drop to null.
> - at G = internal global i8** null
> + at G = internal global i32 0
>
>  define i32 @main(i32 %argc, i8** %argv) {
>  ; CHECK-LABEL: @main(
>  ; CHECK: %G = alloca
> -  store i8** %argv, i8*** @G
> +  store i32 %argc, i32* @G
>    ret i32 0
>  }
>
>  define void @foo(i32 %x) {
> -  call void @llvm.foo(metadata !{i8*** @G, i32 %x})
> +  call void @llvm.foo(metadata !{i32* @G, i32 %x})
>  ; CHECK: call void @llvm.foo(metadata !{null, i32 %x})
>    ret void
>  }
>
>  declare void @llvm.foo(metadata) nounwind readnone
>
>  !named = !{!0}
>
> -!0 = metadata !{i8*** @G}
> +!0 = metadata !{i32* @G}
>  ; CHECK: !0 = metadata !{null}
> Index: test/Transforms/GlobalOpt/dont-localize-pointer.ll
> ===================================================================
> --- /dev/null
> +++ test/Transforms/GlobalOpt/dont-localize-pointer.ll
> @@ -0,0 +1,10 @@
> +; RUN: opt -S -globalopt < %s | FileCheck %s
> +
> + at G = internal global i8** null
> +
> +define i32 @main(i32 %argc, i8** %argv) {
> +; CHECK-LABEL: @main(
> +; CHECK-NOT: alloca
> +  store i8** %argv, i8*** @G
> +  ret i32 0
> +}
> Index: lib/Transforms/IPO/GlobalOpt.cpp
> ===================================================================
> --- lib/Transforms/IPO/GlobalOpt.cpp
> +++ lib/Transforms/IPO/GlobalOpt.cpp
> @@ -1951,7 +1951,8 @@
>        GV->getType()->getElementType()->isSingleValueType() &&
>        GS.AccessingFunction->getName() == "main" &&
>        GS.AccessingFunction->hasExternalLinkage() &&
> -      GV->getType()->getAddressSpace() == 0) {
> +      GV->getType()->getAddressSpace() == 0 &&
> +      !isLeakCheckerRoot(GV)) {
>      DEBUG(dbgs() << "LOCALIZING GLOBAL: " << *GV);
>      Instruction &FirstI = const_cast<Instruction&>(*GS.AccessingFunction
>
> ->getEntryBlock().begin());
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130925/bc81e883/attachment.html>