mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] [PATCH] ldso: notify the debugger when we're doing a dlopen
@ 2020-10-24  5:31 rcombs
  2020-10-24 15:41 ` Rich Felker
  0 siblings, 1 reply; 5+ messages in thread
From: rcombs @ 2020-10-24  5:31 UTC (permalink / raw)
  To: musl

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] [PATCH] ldso: notify the debugger when we're doing a dlopen
  2020-10-24  5:31 [musl] [PATCH] ldso: notify the debugger when we're doing a dlopen rcombs
@ 2020-10-24 15:41 ` Rich Felker
  2020-10-24 19:28   ` Ridley Combs
  0 siblings, 1 reply; 5+ messages in thread
From: Rich Felker @ 2020-10-24 15:41 UTC (permalink / raw)
  To: musl

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.

Rich

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] [PATCH] ldso: notify the debugger when we're doing a dlopen
  2020-10-24 15:41 ` Rich Felker
@ 2020-10-24 19:28   ` Ridley Combs
  2020-10-24 19:39     ` Rich Felker
  0 siblings, 1 reply; 5+ messages in thread
From: Ridley Combs @ 2020-10-24 19:28 UTC (permalink / raw)
  To: musl



> 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.
> 
> Rich

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [musl] [PATCH] ldso: notify the debugger when we're doing a dlopen
  2020-10-24 19:28   ` Ridley Combs
@ 2020-10-24 19:39     ` Rich Felker
  0 siblings, 0 replies; 5+ messages in thread
From: Rich Felker @ 2020-10-24 19:39 UTC (permalink / raw)
  To: musl

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [musl] [PATCH] ldso: notify the debugger when we're doing a dlopen
@ 2020-10-24 19:31 rcombs
  0 siblings, 0 replies; 5+ messages in thread
From: rcombs @ 2020-10-24 19:31 UTC (permalink / raw)
  To: musl

Otherwise lldb doesn't notice the new library and stack traces containing it
get cut off unhelpfully.
---
 ldso/dynlink.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/ldso/dynlink.c b/ldso/dynlink.c
index fcc8f6a6..7df92892 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,9 @@ void *dlopen(const char *file, int mode)
 	pthread_rwlock_wrlock(&lock);
 	__inhibit_ptc();
 
+	debug.state = RT_ADD;
+	_dl_debug_state();
+
 	p = 0;
 	if (shutting_down) {
 		error("Cannot dlopen while program is exiting.");
@@ -2104,9 +2107,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 = RT_CONSISTENT;
+	_dl_debug_state();
 	__release_ptc();
 	if (p) gencnt++;
 	pthread_rwlock_unlock(&lock);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-10-24 19:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-24  5:31 [musl] [PATCH] ldso: notify the debugger when we're doing a dlopen rcombs
2020-10-24 15:41 ` Rich Felker
2020-10-24 19:28   ` Ridley Combs
2020-10-24 19:39     ` Rich Felker
2020-10-24 19:31 rcombs

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).