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

codegen incorrect for some kinds of aliasing #103

Closed
andrewrk opened this issue Jan 31, 2016 · 5 comments
Closed

codegen incorrect for some kinds of aliasing #103

andrewrk opened this issue Jan 31, 2016 · 5 comments
Labels
bug Observed behavior contradicts documented or intended behavior
Milestone

Comments

@andrewrk
Copy link
Member

var global_array : [1024]i32 = undefined;

fn dont_modify_param(param: [1024]i32) -> i32 {
    const y = param[0];
    global_array[0] = 0;
    const x = param[0];
    return x + y;
}

fn nothing() {
    dont_modify_param(global_array);
}

In Zig, parameters are supposed to be immutable. But when dont_modify_param sets global_array[0] = 0, it succeeds in modifying the bytes of param.

To fix this, zig must copy bytes before sending a byvalue parameter under the following condition:

  • when non-const bytes which are not contained in the caller's stack are passed byvalue to a non-noalias parameter.
@andrewrk andrewrk added the bug Observed behavior contradicts documented or intended behavior label Jan 31, 2016
@andrewrk andrewrk added this to the debut milestone Jan 31, 2016
@andrewrk
Copy link
Member Author

andrewrk commented Feb 2, 2016

We can perhaps salvage the ability to make functions accept byvalue structs by implicitly adding noalias to byval parameters if the function body does not modify memory outside the stack.

This is a little cumbersome to constantly reason about though, so perhaps it may be common practice to accept &const Foo args. Hm.

@andrewrk
Copy link
Member Author

Another option: make byval parameters illegal, if the handle of the type is a pointer. In other words, any type which isn't a simple scalar would be an illegal argument type.

For extern/export functions which interact with C ABI, we can allow byval parameters, and keep the C byval semantics.

@andrewrk
Copy link
Member Author

See also #83

@andrewrk
Copy link
Member Author

If we follow this idea of making byval parameters illegal when the handle of the type is a pointer, this would make it illegal to pass type ?i32 as a parameter. This seems problematic. You'd have to instead pass &const ?i32, which makes it inconvenient to do all the nullable type stuff.

@thejoshwolfe
Copy link
Contributor

what's the plan for allowing user types to be passable byval? #83 mentions a copyable attribute. wouldn't that obviate this entire issue?

running with the copyable concept, we should probably make ?T inherit the copyability of T. at most, the ? adds a single bit of size, rounded up to whatever alignment is appropriate. this is a bounded size increase, so it makes sense to me to make it leave the copyability unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior
Projects
None yet
Development

No branches or pull requests

2 participants