Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug of memory access after disposing it #39

Closed
Oleg-N-Cher opened this issue Aug 1, 2016 · 4 comments
Closed

Bug of memory access after disposing it #39

Oleg-N-Cher opened this issue Aug 1, 2016 · 4 comments

Comments

@Oleg-N-Cher
Copy link

Oleg-N-Cher commented Aug 1, 2016

Dear David.

Once you've got rid of alloca in favor of an explicit allocation and deallocation, there was a problem that still doesn't solved, only masked.

    PROCEDURE Prefixed(x: OPT.ConstExt;  y: ARRAY OF CHAR): BOOLEAN;
        VAR i: INTEGER;
    BEGIN i := 0; 
        WHILE x[i+1] = y[i] DO INC(i) END ;
        RETURN y[i] = 0X 
    END Prefixed;
static BOOLEAN OfrontOPC_Prefixed (OfrontOPT_ConstExt x, CHAR *y, LONGINT y__len)
{
    INTEGER i;
    __DUP(y, y__len, CHAR);
    i = 0;
    while ((*x)[__X(i + 1, 256)] == y[__X(i, y__len)]) {
        i += 1;
    }
    __DEL(y);
    return y[__X(i, y__len)] == 0x00;
}

Look at the last line of the C function. Here is the access into the memory which is already disposed. The way of alloca meant empty definition of __DEL(), and everything worked properly. But now __DEL() is

#define __DEL(x) Platform_OSFree((LONGINT)(uintptr_t)x)

So we have to solve this problem somehow. Such a solution, as below, as you have now, I find unsatisfactory.

    PROCEDURE Prefixed(x: OPT.ConstExt;  y: ARRAY OF CHAR): BOOLEAN;
        VAR i: INTEGER;  r: BOOLEAN;
    BEGIN i := 0;
        WHILE x[i+1] = y[i] DO INC(i) END ;
        r := y[i] = 0X;
        RETURN r;
    END Prefixed;

If you are interested in how I'm doing. Now I will do:

    PROCEDURE Prefixed(x: OPT.ConstExt; y: ARRAY [1] OF CHAR): BOOLEAN;
        VAR i: INTEGER;
    BEGIN i := 0;
        WHILE x[i+1] = y[i] DO INC(i) END ;
        RETURN y[i] = 0X
    END Prefixed;

And probably I'll go back to the use of alloca. I do not like this, but I did not come up with a better solution.

@dcwbrown
Copy link
Contributor

dcwbrown commented Aug 1, 2016

I believe this is fundamentally a problem that should be fixed in the
compiler, so

in voc the compiler always generates a dedicated variable for the result
and

evaluates the expression on the return statement into it before calling
__DEL.

Arguably the compiler should be more intelligent and avoid generating a
dedicated

result variable if the return statement does not reference the deleted
parameter copy,

but that's quite a big compiler change, so I excused myself by arguing
that

the C compiler ought to optimise it to the same code anyway.

The change is here:

dcwbrown@e3ca6f6

-- Dave.

On 2016-08-01 13:33, Oleg N. Cher wrote:

Dear David.

Once you've got rid of alloca in favor of an explicit allocation and deallocation, there was a problem that still doesn't solved, only masked.

PROCEDURE Prefixed(x: OPT.ConstExt; y: ARRAY OF CHAR): BOOLEAN;
VAR i: INTEGER;
BEGIN i := 0;
WHILE x[i+1] = y[i] DO INC(i) END ;
RETURN y[i] = 0X
END Prefixed;

static BOOLEAN OfrontOPC_Prefixed (OfrontOPT_ConstExt x, CHAR _y, LONGINT y__len)
{
INTEGER i;
__DUP(y, y__len, CHAR);
i = 0;
while ((_x)[__X(i + 1, 256)] == y[__X(i, y__len)]) {
Console_Char(y[__X(i, y__len)]);
i += 1;
}
if (y[X(i, y__len)] == 0x00) {
Console_String((CHAR
)" YES! ", (LONGINT)7);
} else {
Console_String((CHAR
)" NO! ", (LONGINT)6);
}
Console_Ln();
__DEL(y);
return y[__X(i, y__len)] == 0x00;
}

Look at the last line of the C function. Here is the access into the memory which is already disposed. The way of alloca meant empty definition of __DEL(), and everything worked properly. But now __DEL() is

#define __DEL(x) Platform_OSFree((LONGINT)(uintptr_t)x)

So we have to solve this problem somehow. Such a solution, as below, as you have now, I find unsatisfactory.

PROCEDURE Prefixed(x: OPT.ConstExt; y: ARRAY OF CHAR): BOOLEAN;
VAR i: INTEGER; r: BOOLEAN;
BEGIN i := 0;
WHILE x[i+1] = y[i] DO INC(i) END ;
r := y[i] = 0X;
RETURN r;
END Prefixed;

If you are interested in how I'm doing. Now I will do:

PROCEDURE Prefixed(x: OPT.ConstExt; y: ARRAY [1] OF CHAR): BOOLEAN;
VAR i: INTEGER;
BEGIN i := 0;
WHILE x[i+1] = y[i] DO INC(i) END ;
RETURN y[i] = 0X
END Prefixed;

And probably I'll go back to the use of alloca. I do not like this, but I did not come up with a better solution.

You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub [1], or mute the thread [2].

Links:

[1] #39
[2]
https://github.com/notifications/unsubscribe-auth/ADChoEZa19J4SQULWKmjza9-QkPh46zaks5qbeehgaJpZM4JZiT_

@dcwbrown
Copy link
Contributor

dcwbrown commented Aug 1, 2016

Oh, by the way, the commit I referenced below also includes a change to
OPC.Mod

procedure Prefixed which predates the real fix and is not required.

-- Dave.

On 2016-08-01 14:19, dave wrote:

I believe this is fundamentally a problem that should be fixed in the compiler, so

in voc the compiler always generates a dedicated variable for the result and

evaluates the expression on the return statement into it before calling __DEL.

Arguably the compiler should be more intelligent and avoid generating a dedicated

result variable if the return statement does not reference the deleted parameter copy,

but that's quite a big compiler change, so I excused myself by arguing that

the C compiler ought to optimise it to the same code anyway.

The change is here:

dcwbrown@e3ca6f6

-- Dave.

On 2016-08-01 13:33, Oleg N. Cher wrote:

Dear David.

Once you've got rid of alloca in favor of an explicit allocation and deallocation, there was a problem that still doesn't solved, only masked.

PROCEDURE Prefixed(x: OPT.ConstExt; y: ARRAY OF CHAR): BOOLEAN;
VAR i: INTEGER;
BEGIN i := 0;
WHILE x[i+1] = y[i] DO INC(i) END ;
RETURN y[i] = 0X
END Prefixed;

static BOOLEAN OfrontOPC_Prefixed (OfrontOPT_ConstExt x, CHAR _y, LONGINT y__len)
{
INTEGER i;
__DUP(y, y__len, CHAR);
i = 0;
while ((_x)[__X(i + 1, 256)] == y[__X(i, y__len)]) {
Console_Char(y[__X(i, y__len)]);
i += 1;
}
if (y[X(i, y__len)] == 0x00) {
Console_String((CHAR
)" YES! ", (LONGINT)7);
} else {
Console_String((CHAR
)" NO! ", (LONGINT)6);
}
Console_Ln();
__DEL(y);
return y[__X(i, y__len)] == 0x00;
}

Look at the last line of the C function. Here is the access into the memory which is already disposed. The way of alloca meant empty definition of __DEL(), and everything worked properly. But now __DEL() is

#define __DEL(x) Platform_OSFree((LONGINT)(uintptr_t)x)

So we have to solve this problem somehow. Such a solution, as below, as you have now, I find unsatisfactory.

PROCEDURE Prefixed(x: OPT.ConstExt; y: ARRAY OF CHAR): BOOLEAN;
VAR i: INTEGER; r: BOOLEAN;
BEGIN i := 0;
WHILE x[i+1] = y[i] DO INC(i) END ;
r := y[i] = 0X;
RETURN r;
END Prefixed;

If you are interested in how I'm doing. Now I will do:

PROCEDURE Prefixed(x: OPT.ConstExt; y: ARRAY [1] OF CHAR): BOOLEAN;
VAR i: INTEGER;
BEGIN i := 0;
WHILE x[i+1] = y[i] DO INC(i) END ;
RETURN y[i] = 0X
END Prefixed;

And probably I'll go back to the use of alloca. I do not like this, but I did not come up with a better solution.

You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub [1], or mute the thread [2].

Links:

[1] #39
[2]
https://github.com/notifications/unsubscribe-auth/ADChoEZa19J4SQULWKmjza9-QkPh46zaks5qbeehgaJpZM4JZiT_

@Oleg-N-Cher
Copy link
Author

Thank you for the answer, David.

Then you can rollback the procedure Prefixed to the previous state.

And how about the presence RETURN not only at the end, and in the body of a procedure? Like

    PROCEDURE Prefixed(x: OPT.ConstExt; y: ARRAY [1] OF CHAR): BOOLEAN;
        VAR i: INTEGER;
    BEGIN i := 0; IF i # 0 THEN RETURN y[i] = 0X END;
        WHILE x[i+1] = y[i] DO INC(i);
        IF i # 0 THEN RETURN y[i] = 0X END;
        END ;
        RETURN y[i] = 0X
    END Prefixed;

Everything is work properly too?

I'm sorry, but I have not tested on voc since I do not have a binary distribution of it.

@dcwbrown
Copy link
Contributor

dcwbrown commented Aug 1, 2016

Yes, it handles return correctly wherever you put it.

By the way you don't need a binary distribution of voc to build it - it
comes with pre-prepared bootstrap

C source that is ready to go. It's all handled automatically for you.

Just git clone https://github.com/vishaps/voc, cd into voc and run make
full.

-- Dave.

On 2016-08-01 16:05, Oleg N. Cher wrote:

Thank you for the answer, David.

Then you can rollback the procedure Prefixed to the previous state.

And how about the presence RETURN not only at the end, and in the body of a procedure? Like

PROCEDURE Prefixed(x: OPT.ConstExt; y: ARRAY [1] OF CHAR): BOOLEAN;
VAR i: INTEGER;
BEGIN i := 0; IF i # 0 THEN RETURN y[i] = 0X END;
WHILE x[i+1] = y[i] DO INC(i);
IF i # 0 THEN RETURN y[i] = 0X END;
END ;
RETURN y[i] = 0X
END Prefixed;

Everything is work properly too?

I'm sorry, but I have not tested on voc since I do not have a binary distribution of it.

You are receiving this because you commented.
Reply to this email directly, view it on GitHub [1], or mute the thread [2].

Links:

[1] #39 (comment)
[2]
https://github.com/notifications/unsubscribe-auth/ADChoDcpCN09QpPVOgGI40etRBHvsRq7ks5qbgtVgaJpZM4JZiT_

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants