Re: [musl] [PATCH] ldso: notify the debugger when we're doing a dlopen

[Rich Felker <dalias@libc.org>]

On Sat, Oct 24, 2020 at 02:28:59PM -0500, Ridley Combs wrote:
> 
> 
> > On Oct 24, 2020, at 10:41, Rich Felker <dalias@libc.org> wrote:
> > 
> > On Sat, Oct 24, 2020 at 12:31:56AM -0500, rcombs wrote:
> >> Otherwise lldb doesn't notice the new library and stack traces containing it
> >> get cut off unhelpfully.
> >> ---
> >> ldso/dynlink.c | 9 +++++++--
> >> 1 file changed, 7 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/ldso/dynlink.c b/ldso/dynlink.c
> >> index fcc8f6a6..1ab9bf5b 100644
> >> --- a/ldso/dynlink.c
> >> +++ b/ldso/dynlink.c
> >> @@ -1963,7 +1963,7 @@ void __dls3(size_t *sp, size_t *auxv)
> >> 	debug.bp = dl_debug_state;
> >> 	debug.head = head;
> >> 	debug.base = ldso.base;
> >> -	debug.state = 0;
> >> +	debug.state = RT_CONSISTENT;
> >> 	_dl_debug_state();
> >> 
> >> 	if (replace_argv0) argv[0] = replace_argv0;
> >> @@ -2012,6 +2012,10 @@ void *dlopen(const char *file, int mode)
> >> 	pthread_rwlock_wrlock(&lock);
> >> 	__inhibit_ptc();
> >> 
> >> +	int oldstate = debug.state;
> >> +	debug.state = RT_ADD;
> >> +	_dl_debug_state();
> >> +
> >> 	p = 0;
> >> 	if (shutting_down) {
> >> 		error("Cannot dlopen while program is exiting.");
> >> @@ -2104,9 +2108,10 @@ void *dlopen(const char *file, int mode)
> >> 	update_tls_size();
> >> 	if (tls_cnt != orig_tls_cnt)
> >> 		install_new_tls();
> >> -	_dl_debug_state();
> >> 	orig_tail = tail;
> >> end:
> >> +	debug.state = oldstate;
> >> +	_dl_debug_state();
> >> 	__release_ptc();
> >> 	if (p) gencnt++;
> >> 	pthread_rwlock_unlock(&lock);
> >> -- 
> >> 2.17.1
> > 
> > This looks good, but saving/restoring oldstate isn't necessary and
> > doesn't make sense logically. The code here is taking place under an
> > exclusive lock on the state. The initial state being consistent is a
> > logical precondition to it, and a requirement for releasing the lock
> > again when finished.
> 
> Ah, I'd been thinking that dlopen might get called from within a
> ctor, but now that I look closer I see those are run after we set
> the state back anyway, so yeah this isn't needed.

Yep, addition of new libraries has to be committed before they can be
made visible to the application in any way, which includes via
execution of their ctors. This is what the top half of dlopen is doing
with the lock, saves of old state, and longjmp target, allowing
backout or atomic commit of the entire (recursive dependency loading)
operation.

Rich