Skip to content

There can be only one Linux MIPS module. #368

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

Closed
wants to merge 0 commits into from
Closed

There can be only one Linux MIPS module. #368

wants to merge 0 commits into from

Conversation

GuillaumeLestringant
Copy link
Contributor

@GuillaumeLestringant GuillaumeLestringant commented Aug 28, 2016

There were two MIPS modules inside the Linux module: one at the root, the other one inside musl/b32/. This should not happen.

So I transfered everything from the musl submodule that was missing in the root submodule to the latter, performing some checks in the Linux kernel source code by the way, just to be sure.

The only problem is with the structures: it is a more general problem of Linux that the structure definitions are different from one submodule to another, and also different from the definitions in the kernel. But here is not the place to discuss it, I shall open a specific issue on the matter.

For the time waiting, I did not delete the musl submodule file, in which only remain the said structures, although it cannot be accessed anymore. But the alternative structure definitions are still there, until we are sure we can delete them.

PS: @japaric

@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@japaric
Copy link
Member

japaric commented Aug 28, 2016

There were two MIPS modules inside the Linux module: one at the root, the other one inside musl/b32/.

The one at the root is for the mips-gnu (glibc) target and the musl/b32 one is for the mips-musl target. Because these targets use different C libraries, they have (slightly) different struct definitions (see the stat struct for example) and perhaps even different values for some constants.

I'm sure the modules could be reorganized to share more code between them but I wouldn't land any such refactor without having testing infrastructure for the mips-musl targets in place first. Also note that we want to support the new mips-uclibc targets, whose bindings will also be slightly different from the other two variants, that means the module hierarchy will have to change again to retrofit that target.

@alexcrichton
Copy link
Member

Thanks for the PR @GuillaumeLestringant! As @japaric mentioned though I'd be wary of doing this before we've got verification that the mips-musl bindings didn't break.

@japaric do you know if there's an easy compiler we can download somewhere to run these tests for mips-musl? We could use the ones in the linux-cross image but I'd prefer to avoid using that if possible and stick to a system download if we can.

@GuillaumeLestringant
Copy link
Contributor Author

GuillaumeLestringant commented Aug 28, 2016

@japaric @alexcrichton
I do understand that you’d rather not make any major change to the organization of the modules before you have an appropriate testing infrastructure, and it is OK for me to wait before we do anything.

This being said, it happens that I have been working a lot on reorganizing the submodules of Linux (MIPS being held apart) : my connexion was down the whole afternoon, I had plenty of time for it… The main idea was to create a common module containing everything that is common to glibc and musl, and imported in both cases.

And the fact is that they have much more in common than what they have different. Almost all macros are the same, and much of the simple type definitions too. If you wish, what I have done yet is on a branch of my fork. And there is more that could be done.

What I have not touched yet are the structure definitions that are different from one lib to the other. But I am actually confident that, in most case, it is possible to find a definition which fits all the libc. For instance, you take the example of stat. This is how it is defined in musl.

struct stat {
    dev_t st_dev;
    long __st_padding1[2];
    ino_t st_ino;
    mode_t st_mode;
    nlink_t st_nlink;
    uid_t st_uid;
    gid_t st_gid;
    dev_t st_rdev;
    long __st_padding2[2];
    off_t st_size;
    struct timespec st_atim;
    struct timespec st_mtim;
    struct timespec st_ctim;
    blksize_t st_blksize;
    long __st_padding3;
    blkcnt_t st_blocks;
    long __st_padding4[14];
};

This is the definition in glibc.

struct stat
  {
    unsigned long int st_dev;
    long int st_pad1[3];
#ifndef __USE_FILE_OFFSET64
    __ino_t st_ino;             /* File serial number.          */
#else
    __ino64_t st_ino;           /* File serial number.          */
#endif
    __mode_t st_mode;           /* File mode.  */
    __nlink_t st_nlink;         /* Link count.  */
    __uid_t st_uid;             /* User ID of the file's owner. */
    __gid_t st_gid;             /* Group ID of the file's group.*/
    unsigned long int st_rdev;  /* Device number, if device.  */
#ifndef __USE_FILE_OFFSET64
    long int st_pad2[2];
    __off_t st_size;            /* Size of file, in bytes.  */
    /* SVR4 added this extra long to allow for expansion of off_t.  */
    long int st_pad3;
#else
    long int st_pad2[3];
    __off64_t st_size;          /* Size of file, in bytes.  */
#endif
#ifdef __USE_XOPEN2K8
    /* Nanosecond resolution timestamps are stored in a format
       equivalent to 'struct timespec'.  This is the type used
       whenever possible but the Unix namespace rules do not allow the
       identifier 'timespec' to appear in the <sys/stat.h> header.
       Therefore we have to handle the use of this header in strictly
       standard-compliant sources special.  */
    struct timespec st_atim;            /* Time of last access.  */
    struct timespec st_mtim;            /* Time of last modification.  */
    struct timespec st_ctim;            /* Time of last status change.  */
# define st_atime st_atim.tv_sec        /* Backward compatibility.  */
# define st_mtime st_mtim.tv_sec
# define st_ctime st_ctim.tv_sec
#else
    __time_t st_atime;                  /* Time of last access.  */
    unsigned long int st_atimensec;     /* Nscecs of last access.  */
    __time_t st_mtime;                  /* Time of last modification.  */
    unsigned long int st_mtimensec;     /* Nsecs of last modification.  */
    __time_t st_ctime;                  /* Time of last status change.  */
    unsigned long int st_ctimensec;     /* Nsecs of last status change.  */
#endif
    __blksize_t st_blksize;     /* Optimal block size for I/O.  */
#ifndef __USE_FILE_OFFSET64
    __blkcnt_t st_blocks;       /* Number of 512-byte blocks allocated.  */
#else
    long int st_pad4;
    __blkcnt64_t st_blocks;     /* Number of 512-byte blocks allocated.  */
#endif
    long int st_pad5[14];
  };

And finally, this is for uclibc.

struct stat
  {
    __dev_t st_dev;
    long int st_pad1[2];
#ifndef __USE_FILE_OFFSET64
    __ino_t st_ino;     /* File serial number.      */
#else
    __ino64_t st_ino;       /* File serial number.      */
#endif
    __mode_t st_mode;       /* File mode.  */
    __nlink_t st_nlink;     /* Link count.  */
    __uid_t st_uid;     /* User ID of the file's owner. */
    __gid_t st_gid;     /* Group ID of the file's group.*/
    __dev_t st_rdev;    /* Device number, if device.  */
#ifndef __USE_FILE_OFFSET64
    long int st_pad2[1];
    __off_t st_size;        /* Size of file, in bytes.  */
    /* SVR4 added this extra long to allow for expansion of off_t.  */
    long int st_pad3;
#else
    long int st_pad2[2];
    __off64_t st_size;      /* Size of file, in bytes.  */
#endif
#ifdef __USE_MISC
    /* Nanosecond resolution timestamps are stored in a format
       equivalent to 'struct timespec'.  This is the type used
       whenever possible but the Unix namespace rules do not allow the
       identifier 'timespec' to appear in the <sys/stat.h> header.
       Therefore we have to handle the use of this header in strictly
       standard-compliant sources special.  */
    struct timespec st_atim;            /* Time of last access.  */
    struct timespec st_mtim;            /* Time of last modification.  */
    struct timespec st_ctim;            /* Time of last status change.  */
# define st_atime st_atim.tv_sec        /* Backward compatibility.  */
# define st_mtime st_mtim.tv_sec
# define st_ctime st_ctim.tv_sec
#else
    __time_t st_atime;      /* Time of last access.  */
    unsigned long int st_atimensec; /* Nscecs of last access.  */
    __time_t st_mtime;      /* Time of last modification.  */
    unsigned long int st_mtimensec; /* Nsecs of last modification.  */
    __time_t st_ctime;      /* Time of last status change.  */
    unsigned long int st_ctimensec; /* Nsecs of last status change.  */
#endif
    __blksize_t st_blksize; /* Optimal block size for I/O.  */
#ifndef __USE_FILE_OFFSET64
    __blkcnt_t st_blocks;   /* Number of 512-byte blocks allocated.  */
#else
    long int st_pad4;
    __blkcnt64_t st_blocks; /* Number of 512-byte blocks allocated.  */
#endif
    long int st_pad5[14];
  };

When you take a look at how the types are defined in the different libraries, and you take into account the specificities of each one, that is:

  • it is not used here, but glibc also has a dev_t type that is 64 bits long;
  • __USE_FILE_OFFSET64 is set by default in musl;
  • given that rust-libc seeks compatibility with recent APIs, struct timespec should always be defined,

then it appears that this whole mess can be reduced to two definitions, depending on whether __USE_FILE_OFFSET64 is set or not (which could be a cfg attribute).

// With __USE_FILE_OFFSET64.
struct stat {
    dev_t st_dev;
    long __st_padding1[2];
    ino_t st_ino;
    mode_t st_mode;
    nlink_t st_nlink;
    uid_t st_uid;
    gid_t st_gid;
    dev_t st_rdev;
    long __st_padding2[2];
    off_t st_size;
    struct timespec st_atim;
    struct timespec st_mtim;
    struct timespec st_ctim;
    blksize_t st_blksize;
    long __st_padding3;
    blkcnt_t st_blocks;
    long __st_padding4[14];
};

// Without __USE_FILE_OFFSET64.
struct stat {
    dev_t st_dev;
    long __st_padding1[2];
    ino_t st_ino;
    mode_t st_mode;
    nlink_t st_nlink;
    uid_t st_uid;
    gid_t st_gid;
    dev_t st_rdev;
    long __st_padding2;
    off_t st_size;
    long __st_padding3;
    struct timespec st_atim;
    struct timespec st_mtim;
    struct timespec st_ctim;
    blksize_t st_blksize;
    blkcnt_t st_blocks;
    long __st_padding4[14];
};

And that way, we would have the same definition for the three libraries, and could put it in a single common module.

EDIT: just to be sure… Do you intend to link against the old uclibc or against the one that is still being updated?

@bors
Copy link
Contributor

bors commented Aug 28, 2016

☔ The latest upstream changes (presumably #369) made this pull request unmergeable. Please resolve the merge conflicts.

@GuillaumeLestringant
Copy link
Contributor Author

Given that @japaric ’s module is not an all-libraries MIPS module, it seems easier to just close this PR and open a more general issue on the reorganizing of the Linux module.

@alexcrichton
Copy link
Member

@GuillaumeLestringant note that as of #369 we're testing both musl and glibc mips so we can try to unify the two as well as it'll ensure there's no regressions.

Susurrus pushed a commit to Susurrus/libc that referenced this pull request Mar 26, 2017
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.

5 participants