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

Make munmap() actually work #198

Merged
merged 1 commit into from
May 26, 2020
Merged

Make munmap() actually work #198

merged 1 commit into from
May 26, 2020

Conversation

whitequark
Copy link
Contributor

@whitequark whitequark commented May 23, 2020

Before this commit, he header of a mapped area, struct map, was defined as follows:

struct map {
    int prot;
    int flags;
    off_t offset;
    size_t length;
    char body[];
};

Because the size and alignment of an off_t is 8 bytes, the entire structure was padded to 24 bytes. However, the offset of body into struct map was only 20 bytes. Therefore the code in mmap() and munmap() did not agree on the offset from header to body.

This commit changes mmap() to skip the entire header, which is what munmap() expects and what the size calculation uses.

For the curious, the code in mmap() that calculates the offset looks like:

(local.set $6
 (i32.add
  (local.get $6)
  (i32.const 20)))

and the code in munmap() that cancels it out for freeing looks like:

(call $fimport$5
 (i32.add
  (local.get $0)
  (i32.const -24)))

Before this commit, he header of a mapped area, `struct map`, was
defined as follows:

    struct map {
        int prot;
        int flags;
        off_t offset;
        size_t length;
        char body[];
    };

Because the size and alignment of an `off_t` is 8 bytes, the entire
structure was padded to 24 bytes. However, the offset of `body` into
`struct map` was only 20 bytes. Therefore the code in mmap() and
munmap() did not agree on the offset from header to body.

This commit changes mmap() to skip the entire header, which is what
munmap() expects and what the size calculation uses.
@sunfishcode
Copy link
Member

Wow, thanks for tracking down this super subtle bug!

@sunfishcode sunfishcode merged commit 9b9d243 into WebAssembly:master May 26, 2020
@whitequark whitequark deleted the patch-2 branch May 26, 2020 19:16
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

Successfully merging this pull request may close these issues.

2 participants