* [PATCH] reduce size of struct __libc by using narrower integers
@ 2018-03-06 16:55 Denys Vlasenko
2018-03-06 17:21 ` Rich Felker
0 siblings, 1 reply; 2+ messages in thread
From: Denys Vlasenko @ 2018-03-06 16:55 UTC (permalink / raw)
To: Rich Felker; +Cc: Denys Vlasenko, musl
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@googlemail.com>
CC: musl <musl@lists.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;
};
--
2.16.2
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] reduce size of struct __libc by using narrower integers
2018-03-06 16:55 [PATCH] reduce size of struct __libc by using narrower integers Denys Vlasenko
@ 2018-03-06 17:21 ` Rich Felker
0 siblings, 0 replies; 2+ messages in thread
From: Rich Felker @ 2018-03-06 17:21 UTC (permalink / raw)
To: musl
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@googlemail.com>
> CC: musl <musl@lists.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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-03-06 17:21 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-06 16:55 [PATCH] reduce size of struct __libc by using narrower integers Denys Vlasenko
2018-03-06 17:21 ` Rich Felker
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).