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

mmap's problem #658

Closed
mzzzoozz opened this Issue Jul 15, 2016 · 17 comments

Comments

Projects
None yet
10 participants
@mzzzoozz

mzzzoozz commented Jul 15, 2016

WSL's mmap behavior different from native Linux.
Native Linux work, but WSL(14388) segmentation fault.
Here is sample code:

#include <sys/types.h>
#include <sys/stat.h>
#include <sys/mman.h>
#include <fcntl.h>
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
#include <stdio.h>

int main(int argc, char *argv[])
{
    char *path = "./test.db";

    unlink(path);

    int fd = open(path, O_RDWR|O_CREAT, 0644);
    void *m_ptr = mmap(NULL, 16 * 1024, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
    lseek(fd, 8 * 1024 - 1, SEEK_SET);
    write(fd, "\0", 1);;

    memset(m_ptr, 0, 8 * 1024);

    printf("ok!!\n");

    return 0;
}
@stehufntdev

This comment has been minimized.

Collaborator

stehufntdev commented Jul 28, 2016

Thanks for providing the detailed repro. If you add error checking to the sample above, you will see the mmap call is failing on WSL. This is a known issue where NT has different semantics around mapping a file of size 0.

So we can prioritize appropriately, is this impacting a specific application or is it just something you noticed in the test above?

@therealkenc

This comment has been minimized.

Collaborator

therealkenc commented Jul 29, 2016

FWIW, Cygwin has the same behavior as WSL here, so there's that defense.

@mzzzoozz

This comment has been minimized.

mzzzoozz commented Jul 29, 2016

I did not notice that NT has different semantics around mapping a file of size 0. Mmm, That is another problem.
My sample code wants to indicate that when mmap's size larger that file's actual size, memory operation will fail although extend the file size before action. Here is new example code:

#include <sys/types.h>
#include <sys/stat.h>
#include <sys/mman.h>
#include <fcntl.h>
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
#include <stdio.h>

int main(int argc, char *argv[])
{
    char *path = "./test.db";
    int fd;

    unlink(path);

    fd = open(path, O_RDWR|O_CREAT, 0644);
    lseek(fd, 4 * 1024 - 1, SEEK_SET);
    write(fd, "\0", 1);;
    close(fd);

    fd = open(path, O_RDWR|O_CREAT, 0644);
    void *m_ptr = mmap(NULL, 16 * 1024, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
    printf("m_ptr %p\n", m_ptr);
    lseek(fd, 8 * 1024 - 1, SEEK_SET);
    write(fd, "\0", 1);;

    memset(m_ptr, 0, 4 * 1024);
    printf("first memset ok!!\n");

    memset(m_ptr + 4 * 1024, 0, 4 * 1024);
    printf("last memset ok!!\n");

    printf("ok!!\n");

    return 0;
}

Native linux work, but WSL segmentation fault.
libdb (Berkeley DB) based on the mechanism.

@therealkenc

This comment has been minimized.

Collaborator

therealkenc commented Jul 29, 2016

The feature test in the berkeleydb source is here.

@stehufntdev

This comment has been minimized.

Collaborator

stehufntdev commented Jul 29, 2016

Yes, there are additional different semantics between NT memory mappings. For example, today truncating a file with an outstanding memory mapping will not behave the same.

Thanks for the pointer to the source, we can take a look. We are tracking these issues but to help us prioritize, please give feedback on the uservoice page - https://wpdev.uservoice.com/forums/266908-command-prompt-console-bash-on-ubuntu-on-windo.

@whizzter

This comment has been minimized.

whizzter commented Aug 6, 2016

I was looking forward to working "natively" on windows/wsl with my mmap based code without the horrible remapping cludges for windows file mappings but after testing it the argument could be made that the wsl function syscalls doesn't even follow specs (or atleast common conventions of the function) since the mapping is truncated to file size rather than growing as being able to access more of the file as the file becomes longer.

This code works on linux and freebsd but fails when mmaping(0 sized file) or even worse segfaults once past the silently truncated mapping on wsl even if the file has been enlarged later.

#include <sys/mman.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

int main(int argc,char **argv) {
    int i;
    char * fp;  // file data pointer
    char * buf; // tmp buffer for writes
    int psz;    // page size
    int fsz=0;  // remembered file size
    int fh=open("mmaptest.db",O_CREAT|O_RDWR,S_IRUSR|S_IWUSR); // file handle
    if (fh==-1) {
        printf("could not open\n");
        return -1;
    }
    psz=getpagesize();  // properly the system should work in multiples of pagesizes
    buf=malloc(psz);
    fp=mmap(NULL,16*1024,PROT_READ|PROT_WRITE,MAP_SHARED,fh,0);
    if (fp==MAP_FAILED) {
        printf("could not mmap");
        close(fh);
        return -1;
    }
    printf("file handle:%d filedata pointer:%p page sz:%d\n",fh,fp,psz);
    for (i=0;i<16*1024;i+=512) {
        printf("Writing at %d\n",i);
        if (i>=fsz) {
            write(fh,buf,psz); // enlarge file
            fsz+=psz;
        }
        fp[i]=i;
        getchar();
    }
    munmap(fp,16*1024);
    close(fh);
    return 0;
}
@bitcrazed

This comment has been minimized.

Collaborator

bitcrazed commented Oct 7, 2016

Looking into this a little, running strace on @whizzter's code reveals the following:

mmap(NULL, 16384, PROT_READ|PROT_WRITE, MAP_SHARED, 3, 0) = -1 ENOEXEC (Exec format error)

Filed Bug 9251741:[GH #658] mmap() call results in ENOEXEC

@bitcrazed bitcrazed self-assigned this Oct 7, 2016

@bitcrazed bitcrazed added the bug label Oct 7, 2016

@whizzter

This comment has been minimized.

whizzter commented Oct 7, 2016

Remember that it's a 2 part bug (possibly connected though)

1: the failure to map a nil sized file
2: the failure to do the mapping of a requested size where the pages becomes valid as the file becomes larger. Try creating a 4096 byte file, the accesses to the first page will succeed but because of the silent truncation the writes beyond 4096 bytes will fail, restarting the program after writing another 4kb will create a 8kb mapping but again the memory accesses will fail after the initial 8kb despite the file being 12kb at the time of access.

For correct behaviour the mapping should be of the requested size (16kb) directly and the memory pages should become available as the file is enlarged.

@stehufntdev

This comment has been minimized.

Collaborator

stehufntdev commented Oct 12, 2016

Thanks @whizzter, yes those are both known issues that we are tracking. To help us prioritize, please give feedback on the uservoice page - https://wpdev.uservoice.com/forums/266908-command-prompt-console-bash-on-ubuntu-on-windo.

@benhillis

This comment has been minimized.

Member

benhillis commented Mar 30, 2017

This has been fixed but is not in Creator's update. Look for this to be fixed in the first post-Creator's Update Insider build.

@therealkenc

This comment has been minimized.

Collaborator

therealkenc commented Apr 20, 2017

Maybe update the tag on this one if it is not in Creators to avoid confusion.

@imReker

This comment has been minimized.

imReker commented May 22, 2017

Any update? I've read the lastest release notes of wsl, this issue is not mentioned.

@whizzter

This comment has been minimized.

whizzter commented Oct 18, 2017

Happy to see the mmap being atleast partially fixed in the Fall Creator Update. (MMap'ing a 0 sized file still doesn't seem to work!!! :( ) , but atleast the silent failure is fixed.

@Uldiniad

This comment has been minimized.

Uldiniad commented Mar 3, 2018

@imReker on 16299 I still can't run android builds without your ijar modification so I'm guessing it wasn't fully fixed

@therealkenc

This comment has been minimized.

Collaborator

therealkenc commented Mar 3, 2018

on 16299 I still can't run android builds without your ijar modification so I'm guessing it wasn't fully fixed

I've re-opened #902 which if I'm reading correctly has (had) a test case.

@whizzter if mmaping 0 length files still doesn't work (I haven't checked) then it probably deserves new issue.

@whizzter

This comment has been minimized.

whizzter commented Mar 4, 2018

@therealkenc Maybe, it doesn't affect me personally too much (just caught by running my small test code above the first time). @stehufntdev said they already track it (internally?) and i guess he and @bitcrazed knows how much of it is related to or a separate issue from this one and #902

@hickeng

This comment has been minimized.

hickeng commented Aug 8, 2018

@yorickdowne The WSL maintainers may be particularly good and see updates on closed issues, but experience in various repos taught me that opening a new issue and referencing the closed one is more likely to get attention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment