Skip to content

randomize auto-layout field order in debug mode #4336

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

Open
fengb opened this issue Jan 30, 2020 · 4 comments
Open

randomize auto-layout field order in debug mode #4336

fengb opened this issue Jan 30, 2020 · 4 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@fengb
Copy link
Contributor

fengb commented Jan 30, 2020

Idea mostly stolen inspired from Go where iterating map is explicitly randomized by the compiler. This was introduced to prevent people from accidentally depending on order in testing which causes hard-to-reproduce bugs in prod.

Can we apply the same idea for things like struct memory layout? It is documented to be explicitly undefined but the memory is actually static and manually doing byte casting probably will consistently work right now, but it's almost guaranteed to break in the near future.

If we force randomization, it'll expose these bugs earlier and hopefully point towards the right direction.

@daurnimator daurnimator added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Jan 30, 2020
@daurnimator
Copy link
Contributor

daurnimator commented Jan 30, 2020

I like this; with the condition that the seed for randomisation is printed at the start of compilation.
Without knowing the seed, people will just end up blaming "flaky tests" and run their test suite 3 times until it passes....

May want to way until stage 2/3 though.

@andrewrk andrewrk added this to the 0.7.0 milestone Feb 10, 2020
@andrewrk
Copy link
Member

Related #168

@SpexGuy
Copy link
Contributor

SpexGuy commented Jun 12, 2020

#168 talks about using reversed field order, which is good for some cases but not all of them. I've recently seen some people trying to do stuff like this:

fn GenericType(comptime T: type) type {
    return struct {
        capacity: usize,
        data: []T,
    };
}
const ContainsAnyGeneric = struct {
    pointer: usize,

    // param is pointer to GenericType(?)
    pub fn init(ptr: var) @This() {
        return .{ .pointer = @ptrToInt(ptr) };
    }

    pub fn getCapacity(self: @This()) usize {
        // use u1 as a dummy type, capacity doesn't depend on T
        const dummyGeneric = @intToPtr(*const GenericType(u1), self.pointer);
        return dummyGeneric.capacity;
    }
};

This is subtle UB, because the release-mode compiler may decide to order GenericType(u1) as { capacity, data } and GenericType(SomeOtherT) as { data, capacity }. (it's unlikely that this will happen based on the goals of the reordering, but no guarantee is actually made by the language that this should work). Randomizing field order would catch this, but using reverse fields would not (unless the user tried to use a zero-sized T, which would change the size of the slice).

@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 27, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 May 19, 2021
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 Nov 23, 2021
@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Apr 16, 2022
@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 Apr 9, 2023
@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Jul 9, 2023
@andrewrk andrewrk changed the title Proposal: randomize "undefined" behaviors in debug mode randomize auto-layout field order in debug mode Feb 9, 2025
@andrewrk
Copy link
Member

andrewrk commented Feb 9, 2025

Focusing the scope of the proposal so as to avoid being rejected by conflicting with #63

@andrewrk andrewrk modified the milestones: 0.14.0, 0.15.0 Feb 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

4 participants