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

runtime should kill when assigning to an unallocated pointer (was: returning pointer to a local variable) #82

Open
norayr opened this issue Mar 12, 2020 · 21 comments

Comments

@norayr
Copy link
Member

norayr commented Mar 12, 2020

i think we have to add a warning (because as i understand it is not required to throw an error by oberon report) for this case, when returning a pointer to a local variable:

MODULE test;
IMPORT Out;
TYPE
  arr    = ARRAY 16 OF CHAR;
  string = POINTER TO arr;

PROCEDURE str(): string;
VAR
  a: arr;
  s: string;
BEGIN
  a := "aaa";
  s^ := a;
  RETURN s;
END str;

PROCEDURE main;
VAR st: string;
BEGIN
  st := str();
  Out.String(st^); Out.Ln;
END main;

BEGIN
  main
END test.
@diegosardina
Copy link

I don't understand the issue, except that s^ := a; tries to assign a value to an unallocated pointer and this is a runtime error.
Without that statement, str() would return NIL.

Am I missing something?

@norayr
Copy link
Member Author

norayr commented Mar 12, 2020

no, runtime error doesn't happen there.
it happens when main is called and Out.String is called with the s^ which points to the unexisting location in the stack segment. therefore - segmentation fault, it tried to access memory it should not.

@norayr
Copy link
Member Author

norayr commented Mar 12, 2020

what i thought is - I would like to make a warning at the line

RETURN s;

because s is a pointer to variable allocated on stack.
when we get out of that function stack pointer changes back. and the memory, pointer points to, is inaccessible.

@norayr
Copy link
Member Author

norayr commented Mar 12, 2020

omg, you are right.

@svorkoetter
Copy link

diegosardina's comment is correct. s doesn't point to anything, so assigning to s^ should give an error.

@norayr
Copy link
Member Author

norayr commented Mar 12, 2020

so Oberon pointer should only point to the area allocated on heap with NEW, and that kind of error I wanted to illustrate is not possible in Oberon.

on the other hand, should we make a warning when there is an assignment to an unallocated pointer?

@diegosardina
Copy link

The real issue is that the runtime doesn't halt the program in this statement:

s^ := a;

In Oberon (or any other strong typing language) segmentation faults must never exist.

Type safety implies memory safety.

@norayr
Copy link
Member Author

norayr commented Mar 12, 2020

you mean the runtime should've been killed the program at that point.

@diegosardina
Copy link

Of course.

@norayr
Copy link
Member Author

norayr commented Mar 12, 2020

aside from that, what do you think about compile time warning?
this can be prevented at compile time too.

@diegosardina
Copy link

For what I remember (I don't use an Oberon or Modula-2 compiler since long) the compiler would raise a warning that pointer variable s may not be initialised. However because nested procedures may access intermediate globals, this warning is likely to be false sometimes.

@svorkoetter
Copy link

I don't think it's feasible to reliably generate a compile time warning. Consider:

PROCEDURE foo( i: INTEGER ): string;
VAR
    s: string;
BEGIN
    IF i < 0 THEN
        NEW(s,10)
    ENDIF;
    RETURN s
END foo;

@norayr
Copy link
Member Author

norayr commented Mar 12, 2020

thank you all, I will try several compilers, and will think of solutions.

@norayr
Copy link
Member Author

norayr commented Mar 12, 2020

@svorkoetter I mean warning when parsing such an assignment. Now when I realized the problem is different.

@svorkoetter
Copy link

Unfortunately, the only 100% reliable solution is a run-time check.

At least in Oberon that is possible, since s is guaranteed to initialize to a null pointer.

The big problem with unsafe languages is not that they give a seg fault when assigning through an uninitialized pointer, but that they might not give a seg fault, but just quietly clobber some other data structure in your program.

@svorkoetter
Copy link

Same problem if parsing an assignment:

PROCEDURE foo( i: INTEGER; VAR a: arr );
VAR
    s: string;
BEGIN
    IF i < 0 THEN
        NEW(s,10)
    ENDIF;
    s^ := a;
END foo;

@norayr
Copy link
Member Author

norayr commented Mar 12, 2020

voc already has some runtime checks and kills the program in some cases.
i think it is possible to implement the same for this case as well.

@norayr
Copy link
Member Author

norayr commented Mar 12, 2020

yes, thank you for illustrations.

@norayr norayr changed the title returning pointer to a local variable runtime should kill when assigning to an unallocated pointer (was: returning pointer to a local variable) Mar 12, 2020
@diegosardina
Copy link

This is even more troublesome for a parser:

PROCEDURE Test;
   PROCEDURE Allocate(); BEGIN NEW(s) END Allocate;
VAR
  s: String;
BEGIN  
  Allocate();
  s^ := "aaa";
END Test;

In general this kind of warning to be useful requires global analysis.

@diegosardina
Copy link

I tested the above code with Oxford Oberon-2, indeed it raises a warning that is false in this case (it behaves like some old Modula-2 compilers).

It's my opinion that a warning that may be false sometimes is better than nothing (but fortunately this is the only case, allocating pointers globally via nested procedures is a bad practice. In Oberon-07 that code wouldn't compile).

Like some old Modula-2 compilers, the compiler should raise a warning if a pointer is not passed to NEW(), not assigned by a function procedure or not passed via a VAR parameter.

This doesn't guarantee that the pointer will be valid after, but... with that warning you wouldn't have opened this issue ;-)

@svorkoetter
Copy link

A warning that is sometimes false is useless, as the programmer becomes conditioned to ignoring it, and in this case, there's no way to get rid of the nuisance warning (compared to a warning that a variable is used before it is assigned a value in C; one can always just assign 0 to it, which is a cheap operation compared to allocating memory).

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

3 participants