Follow @Openwall on Twitter for new release announcements and other news
[<prev] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20260320200300.GW1827@brightrain.aerifal.cx>
Date: Fri, 20 Mar 2026 16:03:00 -0400
From: Rich Felker <dalias@...c.org>
To: Fabian Rast <fabian.rast@....de>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] ldso: skip repeated symbol lookups for sorted
 relocations

On Tue, Dec 09, 2025 at 02:17:41AM +0100, Fabian Rast wrote:
> when relocations are sorted by symbol index (-z combreloc),
> we can remember the previous relocations symbol and skip doing the
> lookup again for the next relocation on the same symbol.
> 
> an exception to this are copy relocations that need to resolve to
> a different definition for the same symbol than regular relocations.
> ---
>  ldso/dynlink.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/ldso/dynlink.c b/ldso/dynlink.c
> index 715948f4..0c9b739a 100644
> --- a/ldso/dynlink.c
> +++ b/ldso/dynlink.c
> @@ -385,7 +385,7 @@ static void do_relocs(struct dso *dso, size_t *rel, size_t rel_size, size_t stri
>  	const char *name;
>  	void *ctx;
>  	int type;
> -	int sym_index;
> +	int sym_index, prev_sym_index = 0;
>  	struct symdef def;
>  	size_t *reloc_addr;
>  	size_t sym_val;
> @@ -423,13 +423,19 @@ static void do_relocs(struct dso *dso, size_t *rel, size_t rel_size, size_t stri
>  
>  		sym_index = R_SYM(rel[1]);
>  		if (sym_index) {
> -			sym = syms + sym_index;
> -			name = strings + sym->st_name;
> -			ctx = type==REL_COPY ? head->syms_next : head;
> -			def = (sym->st_info>>4) == STB_LOCAL
> -				? (struct symdef){ .dso = dso, .sym = sym }
> -				: find_sym(ctx, name, type==REL_PLT);
> -			if (!def.sym) def = get_lfs64(name);
> +			if (sym_index != prev_sym_index || type == REL_COPY) {
> +				sym = syms + sym_index;
> +				name = strings + sym->st_name;
> +				if (type == REL_COPY)
> +					ctx = head->syms_next, prev_sym_index = 0;
> +				else
> +					ctx = head, prev_sym_index = sym_index;
> +				def = (sym->st_info>>4) == STB_LOCAL
> +					? (struct symdef){ .dso = dso, .sym = sym }
> +					: find_sym(ctx, name, type==REL_PLT);
> +				if (!def.sym) def = get_lfs64(name);
> +			}
> +
>  			if (!def.sym && (sym->st_shndx != SHN_UNDEF
>  			    || sym->st_info>>4 != STB_WEAK)) {
>  				if (dso->lazy && (type==REL_PLT || type==REL_GOT)) {

I want to move forward with this one but I'm a bit concerned about
whether the logic is correct. There's a type==REL_PLT condition buried
in the find_sym invocation, which in theory could differ between
subsequent calls. This shouldn't happen in real world usage, but I
don't like that it's still a theoretically wrong behavior.

Always clearing prev_sym_index before and after the above code if type
is REL_COPY or REL_PLT seems like it would be safer and more obviously
correct. There should never be repeats of either of these types anyway
so we wouldn't be throwing anything away in terms of performance.

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.