[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20180306172155.GJ1436@brightrain.aerifal.cx>
Date: Tue, 6 Mar 2018 12:21:55 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] reduce size of struct __libc by using narrower
integers
On Tue, Mar 06, 2018 at 05:55:02PM +0100, Denys Vlasenko wrote:
> can_do_threads, threaded and secure are boolean flags, can use bytes
> instead of full word ints.
>
> tls_align and page_size are surely smaller than 4GB, no need to use
> potentially 64-bit size_t.
>
> Signed-off-by: Denys Vlasenko <vda.linux@...glemail.com>
> CC: musl <musl@...ts.openwall.com>
> ---
> ldso/dynlink.c | 3 ++-
> src/internal/libc.h | 11 ++++++-----
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/ldso/dynlink.c b/ldso/dynlink.c
> index 9bf6924b..67d36395 100644
> --- a/ldso/dynlink.c
> +++ b/ldso/dynlink.c
> @@ -124,8 +124,9 @@ static jmp_buf *rtld_fail;
> static pthread_rwlock_t lock;
> static struct debug debug;
> static struct tls_module *tls_tail;
> -static size_t tls_cnt, tls_offset, tls_align = MIN_TLS_ALIGN;
> +static size_t tls_cnt, tls_offset;
> static size_t static_tls_cnt;
> +static unsigned tls_align = MIN_TLS_ALIGN;
> static pthread_mutex_t init_fini_lock = { ._m_type = PTHREAD_MUTEX_RECURSIVE };
> static struct fdpic_loadmap *app_loadmap;
> static struct fdpic_dummy_loadmap app_dummy_loadmap;
> diff --git a/src/internal/libc.h b/src/internal/libc.h
> index 5e145183..3cd44e90 100644
> --- a/src/internal/libc.h
> +++ b/src/internal/libc.h
> @@ -18,14 +18,15 @@ struct tls_module {
> };
>
> struct __libc {
> - int can_do_threads;
> - int threaded;
> - int secure;
> + char can_do_threads;
> + char threaded;
> + char secure;
> volatile int threads_minus_1;
> size_t *auxv;
> struct tls_module *tls_head;
> - size_t tls_size, tls_align, tls_cnt;
> - size_t page_size;
> + size_t tls_size, tls_cnt;
> + unsigned tls_align;
> + unsigned page_size;
> struct __locale_struct global_locale;
> };
This might be okay but needs some care, as it introduces some subtle
changes wrt types. For instance, PAGE_SIZE is #defined (internally for
musl) using libc.page_size when it's not constant for the arch, and
unsigned int will promote & compare differently against signed longs
than size_t would. However, I think you've actually shed light on a
bigger bug here: PAGE_SIZE, when it is defined in the public headers,
has the wrong type (int rather than unsigned int or unsigned long). It
needs to be fixed to use U or UL suffix. I'll do this along with the
PAGESIZE/PAGE_SIZE swap logic (pending namespace issue) right away.
The int->char changes are only valid if we only store positive values,
since char has arch-specific signedness, but I think it's true that we
only store 0 or 1.
For tls_align it's true that >4GB would be ridiculous and breaking,
but there are no checks to see that a loaded module does not have
ridiculous alignment requirements except for failure of the allocation
requests. If we change to a type that can't represent all possible
inputs, there needs to be a check to reject large alignments anywhere
they could arise. I think that means __init_tls.c and dynlink.c's
load_library().
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.
