petelomax/Phix

weird memory violation

Closed this issue · 3 comments

loo66 commented

Hi,

I'm porting some old software and stumbled over a weird crash:

global function func1(atom man, atom ecc)
constant P2 = 2*PI;

atom m=man/360.0

m = P2*(m-trunc(m));

return m
end function

?func1(50.0, 0.9672725)

message is

...\test_bug.ex:12
(warning: lineno of -1 for era of #00685CEC)
fatal exception [MEMORY VIOLATION] at #00685CEC

it seems to crash on the m=P2* ... line ...

Version is 1.0.0 32 Bit on Windows

Greetings
Otto

loo66 commented

if you split it up in

m = m-trunc(m)
m *= P2

it works

Good catch. When the compiler finds m = func(m) it sets (the local) m undefined over the call as part of automatic pass by reference handling, however of course it should not be doing that for cases such as m = m-func(m). There is some logic in the compiler that is supposed to do all that, and in fact as you have already found that actually works, but it looks like the P2* part confuses it somehow. Before reading your split comment I was about to suggest "atom res = P2*(m-trunc(m))" as a temp fix, while I get to work on a proper one.

A secondary issue is that %opSub is not reporting undefined-function result very elegantly, although that might be caused by it (semi-correctly) assuming m cannot possibly be undefined at that point.

The specific fix for this was in pmain.e/StoreVar() under the "elsif opTopIsOp=MathOp then" case:

    p3 = opstack[opsidxm1]
    p2 = opstack[opsidxm2]
    --added 24/7/21(!!)
    if p2=lhsvar
    or p3=lhsvar then
        lhsvar = 0
    end if

In practice I found another two dozen places where similar code was warranted. The code in pmain.e/Assignment() that zeroes lhspos when it finds lhsvar is still in the stack coped and is still needed when StoreVar() is not invoked mid-expression, which is why splitting the line worked. It is now clear that %opSub was not actually at fault, as suspected, so no change there.

Thanks again. Marked as closed on the assumption that I don't need to rush out 1.0.1 for this.