www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 2224] New: Temporary assignment circumvents bug

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2224

           Summary: Temporary assignment circumvents bug
           Product: D
           Version: 2.012
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Keywords: wrong-code
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: bugzilla digitalmars.com
        ReportedBy: fasching logic.at


Hi!

I encountered the following situation. If, in the 
code below, the assignment
   mem.data[d+i]=r(a+i);
is changed into
   uint rr=r(a+i); mem.data[d+i]=rr;
or
   int rr=r(a+i); mem.data[d+i]=rr;
the code works as it should.

Sorry for the strange code, I stripped my original
example down to the minimum size where this strange
behaviour is shown. (It also works with a mixin
instead of a stack class.) The .dups allow for
using dmd2, gdc-4.1 and gdc-4.2. No warnings are issued.
It does not depend on the optimisation switches
as far as I found out.

The example can be reproduced with:
dmd v2.012, gdc-4.1 (GCC) 4.1.3 20070831 and
gdc-4.2 (GCC) 4.2.3 20080225 (prerelease gdc 0.25 20071215, using dmd 1.022) 

Is this a bug or am I doing something wrong?
Any ideas?

Cheers
Oliver


import std.stdio;

class Stack(T)
{
        private uint len;
        private T[] dat;

        T[] data() { return dat[0..len]; }

        int length() { return len; }

        void push(T t) {
                if(dat.length<=len) dat.length=dat.length+32;
                dat[len]=t;
                len++;
        }

        void pushn(uint n) {
                len+=n;
                if(dat.length<len) dat.length=len+32;
        }
}

class termmem
{
        Stack!(uint) mem;
        this(){mem=new Stack!(uint);}

        uint cell(uint addr) { return mem.data[addr]; }
        uint head(uint addr) { return cell(deref(addr)); }

        uint deref(uint addr) {
                for(;;) {
                        uint c=mem.data[addr];
                        if(0!=(c & 0xf000_0000)) break;
                        if(c==addr) break;
                        addr=c;
                }
                return addr;
        }

        void dump() {
                char[] str(uint a) {
                        if(a==0x9000_0000) return "S".dup;
                        if(a==0xa000_0000) return "+".dup;
                        if(a==0xa000_0001) return "*".dup;
                        return " ".dup;
                }
                for(uint i=0; i<mem.length; i++) writefln("%+10x %+10x
%s",i,mem.data[i],str(mem.data[i]));
        }

        uint var() {
                uint d=mem.length;
                mem.push(d);
                return d;
        }

        uint f(uint fnId, uint[] arg...) { return f_(fnId,arg); }

        uint f_(uint fnId, uint[] arg) {
                uint a=fnId>>28;
                assert(a>0x8);
                a-=0x8;
                assert(a==arg.length);
                uint r=mem.length;
                mem.push(fnId);
                for(uint i=0; i<a; i++) {
                        uint w=arg[i];
                        uint t=w>>28;
                        assert(0==t || 0x8==t);
                        mem.push(w);
                }
                return r;
        }

        uint copyfskeleton(uint _addr)
        {
                uint r(uint a) {
                        a=deref(a);
                        uint c=head(a);
                        uint t=c>>28;
                        if(t==0 || t==0x8) return var();
                        assert(!(0<t && t<0x8));
                        t=t&7;
                        uint d=mem.length;
                        mem.push(c);
                        mem.pushn(t);
                        for(int i=1; i<=t; i++) {
                                mem.data[d+i]=r(a+i);
                                /+
                                // If code is changed to that it works as
expected.
                                uint rr=r(a+i);
                                mem.data[d+i]=rr;
                                +/
                        }
                        return d;
                }
                return r(_addr);
        }
}

int main(char[][])
{
        termmem T=new termmem;
        uint S(uint x) { return T.f(0x9000_0000,x); }
        uint P(uint x, uint y) { return T.f(0xa000_0000,x,y); }
        uint x=T.var;
        uint y=T.var;
        uint t=S(P(S(S(x)),x));
        T.dump();
        uint s=S(P(S(S(x)),S(x))); // if commented out, bad behaviour
disappears
        uint u=T.copyfskeleton(t);
        T.dump();
        return 0;
}


-- 
Jul 13 2008
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2224





------- Comment #1 from fasching logic.at  2008-07-13 15:06 -------
Forgotten to say:

The bug persists if
      T[] data() { return dat[0..len]; }
is replaced by
      T[] data() { return dat; }
but disappears if you delete this line and do
      alias dat data;
instead.


-- 
Jul 13 2008
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2224


Don <clugdbug yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |clugdbug yahoo.com.au
            Version|2.012                       |1.020
            Summary|Temporary assignment        |Bad codegen for array
                   |circumvents bug             |element assignment
         OS/Version|Linux                       |All


--- Comment #2 from Don <clugdbug yahoo.com.au> 2009-09-11 00:24:21 PDT ---
This is a TERRIBLE bug report! The test case is really complicated, and very
far from minimal. I've reduced it slightly so that it at least asserts when it
fails.
It also fails on D1, at least as far back as 1.020.

If you comment out the line marked 'FAILS' and replace it with the line marked
'WORKS' it will work correctly. I would appreciate if someone could cut this
test case down. I think it might be important. (OTOH it might just be luck that
it works at all, it might just be reading whatever's on the stack).

----
class Stack{
    private uint len;
    private uint[] dat;

    uint[] data() { return dat[0..len]; }

    int length() { return len; }

    void push(uint t) {
        if(dat.length<=len) dat.length=dat.length+32;
        dat[len]=t;
        len++;
    }
    void pushn(uint n) {
        len+=n;
        if(dat.length<len) dat.length=len+32;
    }
}

class termmem{
    Stack mem;
    this(){mem=new Stack;}

    uint cell(uint addr) { return mem.data[addr]; }
    uint head(uint addr) { return cell(deref(addr)); }

    uint deref(uint addr) {
        for(;;) {
            uint c=mem.data[addr];
            if(0!=(c & 0xf000_0000)) break;
            if(c==addr) break;
            addr=c;
        }
        return addr;
    }
    uint var() {
        uint d=mem.length;
        mem.push(d);
        return d;
    }
    uint f(uint fnId, uint[] arg...) { return f_(fnId,arg); }
    uint f_(uint fnId, uint[] arg) {
        uint a=fnId>>28;
        assert(a>0x8);
        a-=0x8;
        assert(a==arg.length);
        uint r=mem.length;
        mem.push(fnId);
        for(uint i=0; i<a; i++) {
            uint w=arg[i];
            uint t=w>>28;
            assert(0==t || 0x8==t);
            mem.push(w);
        }
        return r;
    }

    uint copyfskeleton(uint _addr)
    {
        uint r(uint a) {
            a=deref(a);
            uint c=head(a);
            uint t=c>>28;
            if(t==0 || t==0x8) return var();
            t=t&7;
            uint d=mem.length;
            mem.push(c);
            mem.pushn(t);
            for(int i=1; i<=t; i++) {
// FAILS:
      mem.data[d+i]=r(a+i);
// WORKS:
      //uint rr=r(a+i); mem.data[d+i]=rr;
            }
            return d;
        }
        return r(_addr);
    }
}

void main()
{
    termmem T=new termmem;
    uint S(uint x) { return T.f(0x9000_0000,x); }
    uint P(uint x, uint y) { return T.f(0xa000_0000,x,y); }
    uint x=T.var;
    uint y=T.var;
    uint t=S(P(S(S(x)),x));
    uint s=S(P(S(S(x)),S(x))); // if commented out, bad behaviour disappears
    uint u=T.copyfskeleton(t);
    assert(T.mem.data[0x17]==0x18);
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 11 2009
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2224


Matti Niemenmaa <matti.niemenmaa+dbugzilla iki.fi> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |matti.niemenmaa+dbugzilla i
                   |                            |ki.fi
         Resolution|                            |INVALID


--- Comment #3 from Matti Niemenmaa <matti.niemenmaa+dbugzilla iki.fi>
2009-09-12 00:20:04 PDT ---
This is not a bug.

There's no sequence point in "mem.data[d+i]=r(a+i);", so the order of
evaluation of mem.data[d+i] and r(a+i) is unspecified. r calls both mem.push
and mem.pushn, both of which may lead to reallocations of mem.data, possibly
moving the array to a completely different location. Thus the behaviour of the
code depends on whether r(a+i) or mem.data is evaluated first.

The "bug" is no doubt due to the mem.data getting evaluated first, since doing
the temporary assignment forces it to be the other way around.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 12 2009