returning a pointer to a local should be a compile error

[ghost]

Maybe duplicate, but I couldn't find it anywhere.

Consider the following code:

const Point = struct {
    x: f32,
    y: f32,
};
// Using #1717 syntax -- this proposal does not depend on #1717
const init = fn (_x: f32, _y: f32) Point {
    var inst = Point { .x = _x, .y = _y };
    return inst;
};

No problems here, copy elision (should) put inst in the right place. Now consider this alternative:

const init = fn (_x: f32, _y: f32) *Point {
    var inst = Point { .x = _x, .y = _y };
    return &inst;
};

Someone who doesn't know all the details of RLS might do this. Problem is, frame is now explicitly stack-allocated, in a frame which is invalidated as soon as init returns. This is comptime-detectable undefined behaviour. Zig should not compile this code.

(There are reasons not to allow this at comptime either -- just look at the headache that is #5718 (comment). Also, breaking from blocks permits a similar problem.)

[Vexu]

There has already been some work done on this (see ir_analyze_instruction_return in src/ir.cpp).

[ghost]

Ah, I thought there might have been. I'll keep this open for explication's sake, though. Is the plan to disallow this at comptime as well?

[mikdusan]

from zig weekly 2020/12/16

Martin: I think there’s another issue that we should consider that might help us decide this.
Compiler intelligence: Returning a pointer to a local should be a compile error (#5725)

This is a case where there is a large spectrum of how smart the compiler could be about detection.  But the problem is that the definition of “what compiles” comes from the language spec, and we’ve said in the past that keeping the spec simple is an important goal.  This is one of the major use cases for warnings in C/C++ - the spec defines what compiles, and warnings do the “smarter” checks that are too complicated to standardize.

Andrew: Idea, what about the following rule:  If a compiler can statically detect illegal behavior, it is allowed to be a compile error.  By definition, illegal behavior represents an invalid Zig program, so there is no valid zig program that this would fail to compile.

Josh: This is not actually very useful.  In the case of returning a stack local, the compiler would need to prove:
- the function is called in all cases
- the function returns a stack local
- the returned pointer is dereferenced

This needs to happen in all cases of running the program in order to statically prove UB.  The `--help` flag in the Zig compiler codebase would effectively prevent this analysis from finding any problems in it, because it can’t prove that most functions are actually executed.

It can’t be done at function level, because this is a totally valid callback:
fn callback() void {
  // this should not be called on my inputs
  unreachable;
}


But maybe things like this should be warnings.

Andrew:  This is a good realization, but I want to think more about it.  If we did include warnings, we should at least have the compiler default treat warnings as errors.  But I’m not prepared to decide on this immediately.  Let’s come back to it in the future.

If the compiler can prove something statically that is an error, let's emit an error. For unused variables, let's reduce the surface:

• leave globals out of it
• identify unused locals only if there is no comptime control flow in or of any nested block

How effectual will this be? If it's not a compile-speed killer and relatively straight forward, why not? Whether this is a warning or error, is a different question.

On the subject of returning local pointers:

Josh: This is not actually very useful.  In the case of returning a stack local, the compiler would need to prove:
- the function is called in all cases
- the function returns a stack local
- the returned pointer is dereferenced

Against the first point, it doesn't matter if the function is called in all cases. We already allow bogus things in lazy functions that would blow up if ever compiled. This should not be a prerequisite for detecting errors.

Against the third point, we could knowingly let local pointers escape with attribution:

fn getEndOfStackPtr() *noderef const u8 { ... }

which lets zig know and enforce that pointer can never be dereferenced. Thus it is possible to return but never deref. In the other case, if the compiler can prove escape, that needs to be an error. And again here, it doesn't matter if the compiler can detect 100% escapes. Any % is valuable.

[renatoathaydes]

I made a mistake due to this and the program worked for many cases, until a test finally observed a "corrupted" value and it took me several hours to find out this was the problem.

In my case the pointer was a little bit hidden behind a call like this:

var buffer: [3]u8{0,0,0};
/// do stuff
return std.fmt.bufPrint(&buffer, "{d}", .{code});

This made me sad... a language that allows this without warnings cannot be used to write reliable code. Hope Zig can fix this.

[matu3ba]

At least for C its not possible in a portable way: https://stackoverflow.com/a/16361335

You cannot do what you want in a portable way, because the C language standard does not specify the stack, 
program area,  and heap as distinct areas. Their location can depend on the processor architecture, the 
operating system, the loader, the linker, and the compiler. Trying to guess where a pointer is pointing is 
breaking the abstraction provided by C, so you probably you shouldn't be doing that.

This makes me suspect, that this should be part of tooling https://en.wikipedia.org/wiki/Dangling_pointer#Dangling_pointer_detection or static analysis with the ability to annotate intended program semantics (without static analysis can only help for simple cases or has significant runtime cost).

[gooncreeper]

I don't see why we cannot make returning a pointer to a local variable undefined behavior. If this behavior is desired then it can be made explicit with @intFromPtr. Additionally, the undefined behavior can be caught at runtime by checking if the returned pointer is within the function's stack frame.

[randomcoder67]

Has this been implemented?

const std = @import("std");

fn get() *[10]u8 {
    var data : [10]u8 = undefined;
    var j: u8 = 0;
    for (0..10) |i| {
        data[i] = j + 65;
        j += 1;
    }
    return &data;
}

pub fn main() !void {
    const data = get();
    std.debug.print("{s}\n", .{data});
}

Compiles perfectly fine, but it of course broken.

Comparable c code:

#include <stdio.h>

int* get() {
	int thing[10];
	for (int i=0; i<10; i++) {
		thing[i] = i + 65;
	}
	return thing;
}

int main() {
	int* data = get();
	printf("%s\n", data);
}

Gives a warning:
10:16: warning: function returns address of local variable [-Wreturn-local-addr]

[mlugg]

Has this been implemented?

No, that's why the issue remains open.

[andrewrk]

I don't think it's possible to catch at compile-time. Please see this issue for catching it at runtime: #23528