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

[Duncan Sands]

Hi Alexey,

On 25/09/13 13:32, Alexey Samsonov 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.

having magic semantics for globals with a certain type seems like a bad idea
to me.  Here the global in effect has a secret external use, but that isn't
clear from the IR.  In my opinion it's only asking for trouble and it would be
better to make it explicit by requiring all GC roots to be used by (eg) a new
llvm.gc_root global, analogous to llvm.used.

Ciao, Duncan.

>
> 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
>