|
Message-ID: <20130226233342.GF20323@brightrain.aerifal.cx> Date: Tue, 26 Feb 2013 18:33:42 -0500 From: Rich Felker <dalias@...ifal.cx> To: musl@...ts.openwall.com Subject: Re: Fix for fields in utmp On Tue, Feb 26, 2013 at 09:21:12PM +0100, Szabolcs Nagy wrote: > see inline comments > > * Ivan Kanakarakis <ivan.kanak@...il.com> [2013-02-26 12:34:43 +0200]: > > Linux suicidemachine 3.7.5-1-ARCH #1 SMP PREEMPT Mon Jan 28 10:03:32 > > CET 2013 x86_64 GNU/Linux > > ---------------------------------- > > --- data/glibc.sizeof 2013-02-26 12:27:48.112569344 +0200 > > +++ data/musl.sizeof 2013-02-26 12:27:48.119236080 +0200 > > @@ -90,3 +90,3 @@ > > float 4 > > -float_t 4 > > +float_t 8 > > looks like musl does not respect FLT_EVAL_METHOD on x86_64 > this should be fixed I just changed this to 4, as it should be. Or does x86_64 support changing the FLT_EVAL_METHOD, in which case we'd need to treat it like on i386.. > > fpos_t 16 > > @@ -112,4 +112,4 @@ > > int8_t 1 > > -int_fast16_t 8 > > -int_fast32_t 8 > > +int_fast16_t 4 > > +int_fast32_t 4 > > int_fast64_t 8 > > these diffs may cause problems These are actually omitted from the ABI documents if I remember correctly, with the idea that different compilers might define them differently and that they shouldn't be used for interfaces between functions, only internal use. There may be compelling reasons we should match anyway, but there's one compelling reason NOT to match: the glibc values are WRONG. 64-bit integers are much slower than 32-bit integers if you just need 16 or 32 bits of data for at least two operations: division and modulo. And they're at best the same speed (but larger and thus slower in a cache pressure sense) for other arithmetic operations. > > @@ -122,3 +122,3 @@ > > intptr_t 8 > > -jmp_buf 200 > > +jmp_buf 64 This is because our jmp_buf does not contain sigset_t. However due to ABI issues for C++ (glibc's jmp_buf and sigjmp_buf sharing the same struct tag) we might want to make them the same anyway. There's an existing thread somewhere on the topic. > > key_t 4 > > @@ -181,6 +181,6 @@ > > quad_t 8 > > -regex_t 64 > > +regex_t 56 I think this should be fixed to match. > > register_t 8 > > -regmatch_t 8 > > -regoff_t 4 > > +regmatch_t 16 > > +regoff_t 8 These are a bug in glibc; their types do not conform to the requirements imposed on regex.h and regexec(). > > res_state 8 > > @@ -196,3 +196,3 @@ > > sighandler_t 8 > > -siginfo_t 128 > > +siginfo_t 136 This is probably a mistake, maybe in the padding..? > > sigjmp_buf 200 > > @@ -215,3 +215,3 @@ > > struct ar_hdr 60 > > -struct arpd_request 40 > > +struct arpd_request 28 No idea. > > struct arphdr 8 > > @@ -222,3 +222,3 @@ > > struct cmsghdr 16 > > -struct crypt_data 131232 > > +struct crypt_data 260 This is intentional. glibc's crypt_data is uselessly large and does not fit on thread stacks. The only useful part of crypt_data is a buffer for storing the resulting hash. > > struct dirent 280 > > @@ -282,3 +282,3 @@ > > struct itimerval 32 > > -struct lastlog 292 > > +struct lastlog 296 Probably needs to be checked. > > struct lconv 96 > > @@ -312,3 +312,3 @@ > > struct ns_tsig_key 2072 > > -struct ntptimeval 72 > > +struct ntptimeval 32 Should we change this? Probably. > > struct option 32 > > @@ -326,4 +326,4 @@ > > struct rtentry 120 > > -struct rusage 144 > > -struct sched_param 4 > > +struct rusage 272 > > +struct sched_param 48 Not sure about rusage. For sched_param, we support the extra fields for the sporadic server option even though we don't support it (and Linux probably never will). > > struct sembuf 6 > > @@ -348,3 +348,3 @@ > > struct sockaddr_ll 20 > > -struct sockaddr_storage 128 > > +struct sockaddr_storage 136 Probably wrong. > > struct sockaddr_un 110 > > @@ -361,3 +361,3 @@ > > struct strrecvfd 20 > > -struct sysinfo 112 > > +struct sysinfo 368 Just extra padding for growth. Maybe it should be changed. > > struct termios 60 > > @@ -376,3 +376,3 @@ > > struct utimbuf 16 > > -struct utmpx 384 > > +struct utmpx 400 > > so we need the glibc hacks to be abi compatible: > > struct utmpx > { > short ut_type; > pid_t ut_pid; > char ut_line[UT_LINESIZE]; > char ut_id[4]; > char ut_user[32]; > char ut_host[256]; > struct { > short e_termination; > short e_exit; > } ut_exit; > #if 64 bit abi > __int32_t ut_session; > struct {__int32_t tv_sec; __int32_t tv_usec;} ut_tv; > #else > long ut_session; > struct timeval ut_tv; > #endif > unsigned ut_addr_v6[4]; > char __unused[20]; > }; > > the comment in glibc sysdeps/gnu/bits/utmpx.h is > > /* The fields ut_session and ut_tv must be the same size when compiled > 32- and 64-bit. This allows files and shared memory to be shared > between 32- and 64-bit applications. */ This is idiotic and breaks correct applications that access the ut_tv member as ut_tv rather than just accessing its members, since ut_tv now has the wrong type and cannot be assigned to/from timeval structs and cannot have its address passed to functions that expect timeval structs. Since utmp is not supported anyway, I think it's better to have the right types to avoid breaking correct code; the data will never be written or read anyway; it just goes to the bitbucket so layout does not matter so much. > > struct utsname 390 > > @@ -399,4 +399,4 @@ > > uint8_t 1 > > -uint_fast16_t 8 > > -uint_fast32_t 8 > > +uint_fast16_t 4 > > +uint_fast32_t 4 > > uint_fast64_t 8 See above. > > @@ -416,4 +416,4 @@ > > wchar_t 4 > > -wctrans_t 8 > > -wctype_t 8 > > +wctrans_t 4 > > +wctype_t 4 > > wint_t 4 > > > > it seems wctype_t is unconditionally long in glibc As it should be. This is a bug in musl. Being an opaque type that could (and often should) be implemented as a pointer, wctype_t should always be pointer-sized. Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.