[Issue 17461] New: Bad codegen: compiler emit's call to destructor for uninitialised temporary

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=17461

          Issue ID: 17461
           Summary: Bad codegen: compiler emit's call to destructor for
                    uninitialised temporary
           Product: D
           Version: D2
          Hardware: x86_64
                OS: Windows
            Status: NEW
          Severity: blocker
          Priority: P1
         Component: dmd
          Assignee: nobody at puremagic.com
          Reporter: turkeyman at gmail.com

DMD32 D Compiler v2.073.0-master-e29f8e7
x86_64 (Win64), standard debug build, no optimisations


This is as simple as I could get it:

----------------------------------------------------

// this function generates bad code; the `?:` introduces some weird locals
void t()
{
  A(u1 ? u1 : u2);
}

//------ context ------

B u1() { return B(); }
__gshared B u2;

struct A
{
  void *p;
  alias get this;

  void* get() pure nothrow @nogc @trusted { return p; }

  this(void* s)
  {
    p = s;
  }
}

struct B
{
  alias a this;
  A a;

  this(void* s)
  {
    a = A(s);
  }
  ~this()
  {
    if (p)
      *cast(int*)p = 10; // <- crash if p is uninitialised!
  }
}


int main()
{
  u2 = B(cast(void*)0x12345678);
  t();
  return 0;
}

-----------------------------------------------

The disassembly for the t() function which emits the bad code.
Symbol names from debuginfo are visible:

void t()
    push        rbp  
    mov         rbp,rsp  
    sub         rsp,38h  
    push        rbx  
    push        rsi  
    push        rdi  
    push        r12  
    push        r13  
    push        r14  
    push        r15  
{
  A(u1 ? u1 : u2);
    mov         qword ptr [rbp-28h],0  
    lea         rax,[rbp-28h]  
    mov         qword ptr [rbp-20h],rax  

; from here we call u1() to get the value to test in the `?:`...
; note, the result is stored in '__tmpfordtor1804'
    lea         rcx,[__tmpfordtor1804]  
    sub         rsp,20h  
    call        dplug.dplug.u1 (07FFB076611F0h)  
    add         rsp,20h  
    lea         rcx,[__tmpfordtor1804]  
    sub         rsp,20h  
    call        dplug.dplug.A.get (07FFB07661000h)  
    add         rsp,20h  
; test if u1() was null (it is!) and jump to the 'else' case
    test        rax,rax  
    je          dplug.dplug.t+71h (07FFB07661291h)  ; does branch!

; if u1() was not null, call u1() again to get the value for the 'if' case
; the result is stored in `__tmpfordtor1805`, which is _INITIALISED HERE_
    lea         rcx,[__tmpfordtor1805]  
    sub         rsp,20h  
    call        dplug.dplug.u1 (07FFB076611F0h)  
    add         rsp,20h  
    lea         rcx,[__tmpfordtor1805]  
    sub         rsp,20h  
    call        dplug.dplug.A.get (07FFB07661000h)  
    add         rsp,20h  
    mov         rdx,rax  
    jmp         dplug.dplug.t+88h (07FFB076612A8h)  

; this is the 'else' branch, which loads u2
; note; we SKIPPED INITIALISATION of `__tmpfordtor1805`
    lea         rcx,[dplug.dplug.u2 (07FFB07725660h)]  
    sub         rsp,20h  
    call        dplug.dplug.A.get (07FFB07661000h)  
    add         rsp,20h  
    mov         rdx,rax  
    mov         rcx,qword ptr [rbp-20h]  
    sub         rsp,20h  

; here we construct A() from the result of the `?:` statement
; note; we didn't actually do anything with A, so there's nothing here
    call        dplug.dplug.A.this (07FFB07661060h)  
    add         rsp,20h  
    mov         rax,qword ptr [rax]  
    mov         qword ptr [rbp-8],rax  

; from here we destruct the temporaries...

; call a fragment of code that destruct's '__tmpfordtor1805'
; note; this is NOT guarded, and '__tmpfordtor1805' was never initialised
above!
    call        dplug.dplug.t+0A7h (07FFB076612C7h)  
    jmp         dplug.dplug.t+0B9h (07FFB076612D9h)  

; destruct '__tmpfordtor1805' sub-function thing
07FFB076612C7h:
    lea         rcx,[__tmpfordtor1805]  
    sub         rsp,20h  
    call        dplug.dplug.B.~this (07FFB076610C0h)  
    add         rsp,20h  
    ret  

; call a fragment of code that destruct's '__tmpfordtor1804'
07FFB076612D9h:
    call        dplug.dplug.t+0C0h (07FFB076612E0h)  
    jmp         dplug.dplug.t+0D2h (07FFB076612F2h)  

; destruct '__tmpfordtor1804' sub-function thing
07FFB076612E0h:
    lea         rcx,[__tmpfordtor1804]  
    sub         rsp,20h  
    call        dplug.dplug.B.~this (07FFB076610C0h)  
    add         rsp,20h  
    ret  

07FFB076612F2h:
}
    pop         r15  
    pop         r14  
    pop         r13  
    pop         r12  
    pop         rdi  
    pop         rsi  
    pop         rbx  
    mov         rsp,rbp  
    pop         rbp  
    ret


I hope I've made that disassembly easy enough to follow.

The problem is, in the `a() ? a() : b` expression, the first call to a() and
the second call to a() have different result temporaries.
The second call only happens if the first call to a() evaluates 'true', which
in this test, it doesn't! This means that the second temporary is never
initialised.
When the function concludes, it calls destructor for both temporaries, even
though the second one was never initiailised, and destructing the uninitialised
object lead to me weird behaviours and crashes.

--