Thread overview
[Bug 283] Wrong code with -O, possible cleanup_point_expr bug
Jan 25, 2018
Iain Buclaw
Jan 25, 2018
Iain Buclaw
Jan 25, 2018
Iain Buclaw
Jan 28, 2018
Iain Buclaw
January 25, 2018
https://bugzilla.gdcproject.org/show_bug.cgi?id=283

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |critical
           Priority|Normal                      |High

--- Comment #1 from Iain Buclaw <ibuclaw@gdcproject.org> ---
The above example

-- 
You are receiving this mail because:
You are watching all bug changes.
January 25, 2018
https://bugzilla.gdcproject.org/show_bug.cgi?id=283

--- Comment #2 from Iain Buclaw <ibuclaw@gdcproject.org> ---
The above reduced test fails when compiled with: 'gdc -frelease -O'

It runs just fine when compiling with: 'gdc -frelease'

An earlier, more complex version of this test passed when built with -O2, but failed with -O3.

When compiling the original source module (sdtor.d), it always succeeds regardless of what optimization flags you pass.

So, the same program ran in isolation can do something different than when compiled with a larger codebase.  Making it a rather peculiar optimization bug.

-- 
You are receiving this mail because:
You are watching all bug changes.
January 25, 2018
https://bugzilla.gdcproject.org/show_bug.cgi?id=283

--- Comment #3 from Iain Buclaw <ibuclaw@gdcproject.org> ---
Yes I think the CLEANUP_POINT_EXPR is the problem.

In briefest way of describing, the following is generated by codegen:

---
<<cleanup_point
    __gate68 = 0;
    TARGET_EXPR <__pfx69, __pfx69 = *this, __fieldPostblit (&__pfx69)>
    __gate68 = 1;>>;
__slPathE67 = {};
return <retval> = *__ctor (&__slPathE67, &__pfx69, __pfy70);
---

Gimplified into try/finally as:

---
try
  {
    __gate68 = 0;
    __pfx69 = *this;
    __fieldPostblit (&__pfx69);
    try
      {
        try
          {
            __gate68 = 1;
          }
        finally
          {
            __gate68.0_1 = __gate68;
            _2 = ~__gate68.0_1;
            if (_2 != 0) goto <D.4232>; else goto <D.4233>;
            <D.4232>:
            __fieldDtor (&__pfx69);
            <D.4233>:
          }
      }
    finally
      {
        __pfx69 = {CLOBBER};    // ***
      }
    __slPathE67 = {}
    _4 = __ctor (&__slPathE67, &__pfx69, __pfy70);    // ***
    <retval> = *_4;
    return <retval>;
  }
---

Marked with *** are places of interest.  First, temporary var __pfx69 is
clobbered (it is invalidated for further reuse).  Second, its address is taken
(undefined behaviour at this point).

This is likely a problem with the rewrite done in d_build_call, the cleanup_point expression should encapsulate both parameter and call.

-- 
You are receiving this mail because:
You are watching all bug changes.
January 28, 2018
https://bugzilla.gdcproject.org/show_bug.cgi?id=283

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Iain Buclaw <ibuclaw@gdcproject.org> ---
https://github.com/D-Programming-GDC/GDC/pull/606

-- 
You are receiving this mail because:
You are watching all bug changes.