From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 18613 invoked from network); 24 Oct 2020 19:40:01 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 24 Oct 2020 19:40:01 -0000 Received: (qmail 25700 invoked by uid 550); 24 Oct 2020 19:39:59 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 25676 invoked from network); 24 Oct 2020 19:39:59 -0000 Date: Sat, 24 Oct 2020 15:39:46 -0400 From: Rich Felker To: musl@lists.openwall.com Message-ID: <20201024193946.GG534@brightrain.aerifal.cx> References: <20201024053156.32053-1-rcombs@rcombs.me> <20201024154128.GE534@brightrain.aerifal.cx> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [PATCH] ldso: notify the debugger when we're doing a dlopen On Sat, Oct 24, 2020 at 02:28:59PM -0500, Ridley Combs wrote: > > > > On Oct 24, 2020, at 10:41, Rich Felker 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