From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x62e.google.com (mail-pl1-x62e.google.com [IPv6:2607:f8b0:4864:20::62e]) by sourceware.org (Postfix) with ESMTPS id 7EFB83858C1F for ; Wed, 16 Feb 2022 00:44:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7EFB83858C1F Received: by mail-pl1-x62e.google.com with SMTP id x11so707873pll.10 for ; Tue, 15 Feb 2022 16:44:30 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=I+IkLGt0uwVN6vH3dadJWfE9+1Jsi7rZY/SW/Nwy57Q=; b=OAdJQO0awCBhw8ZzEhYAD4o3xONvMtb56CsY79kmxAhpBa238vJNcJnmzLh+XmnQtj DktkTUzcTZUuRqZJTvNSS3MLh5a4+SiRg2TdAJ/YsWTkLSYLLZD2ql/49w0MgNJT5GH9 Y3pzbOP1W44vqoflm1cathciELISgWmyTNmHWbUCCKHBzJR9aJd+AUZrkTTiO6/t8TEs oAKg1J8QzFmcTDDnk1BVhBzQuI9zeHq7RV4IPPrMNZqc+i2hKHl7NkZX8KsKybTq1yMp MG+iRxh7SjECQwM49wjZJbkkVD/p+ljorfGaFH8yl5nuBOGMWZu8CgMcH6C2QxPcS3Rn f8Zw== X-Gm-Message-State: AOAM530MWxR3OSHb0TvxDHa5vtfpmJoBJVLgzu0ca9V57X/ax1ALwrmV tHfIouI04fZMklZS8/W8n420vT7csUhtPlM7+P4= X-Google-Smtp-Source: ABdhPJzAHqi3EQKIzg6kGApNGjq8TMhbtVRwtipAwkhyZXweCJuXLseHfe1l3F0fSGED8RuQuhhyZB5l5dqX7SmhMoU= X-Received: by 2002:a17:902:b410:b0:14b:e53:7aa0 with SMTP id x16-20020a170902b41000b0014b0e537aa0mr114463plr.101.1644972269564; Tue, 15 Feb 2022 16:44:29 -0800 (PST) MIME-Version: 1.0 References: <20220210160823.35a8508e@f35-zws-1> In-Reply-To: <20220210160823.35a8508e@f35-zws-1> From: "H.J. Lu" Date: Tue, 15 Feb 2022 16:43:53 -0800 Message-ID: Subject: Re: [PATCH v3] Support glibc multiple namespace extension To: Kevin Buettner Cc: "H.J. Lu via Gdb-patches" , Markus Metzger Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3026.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Feb 2022 00:44:33 -0000 On Thu, Feb 10, 2022 at 3:08 PM Kevin Buettner wrote: > > Hi H.J., > > This work looks pretty good to me. I found a couple of coding style nits > and have a question/request; see below. > > On Sat, 2 Oct 2021 13:43:40 -0700 > "H.J. Lu via Gdb-patches" wrote: > > [...] > > diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c > > index 3de1bb9c7f7..9d851ba8930 100644 > > --- a/gdb/solib-svr4.c > > +++ b/gdb/solib-svr4.c > [...] > > @@ -1335,6 +1366,18 @@ svr4_current_sos_direct (struct svr4_info *info) > > if (lm) > > svr4_read_so_list (info, lm, 0, &link_ptr, 0); > > > > + /* Get the next namespace from the r_next field. */ > > + lm = solib_svr4_r_next (info->debug_base); > > + while (lm) > > Due to the GDB coding standard, this test needs to be turned into an > explicit comparison against 0. See: > > https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Comparison_With_nullptr_And_Zero > > (FWIW, I don't really like this part of our standard; I personally > find it more readable to code it the way that you did. Also, I do > realize that there are existing implicit tests against 0 in this file > and elsewhere and that they do not conform to GDB's current coding > standard. I'm in no hurry to fix them - but, obviously, I won't > object if someone does.) > > > + { > > + /* Get the link map in this namespace. */ > > + CORE_ADDR link_map = solib_svr4_r_map (lm); > > + if (link_map) > > Likewise, for the check above. > > > + svr4_read_so_list (info, link_map, 0, &link_ptr, 0); > > + /* Go to the next namespace. */ > > + lm = solib_svr4_r_next (lm); > > + } > > + > > cleanup.release (); > > > > if (head == NULL) > > @@ -1706,7 +1749,8 @@ solist_update_full (struct svr4_info *info) > > failure. */ > > > > static int > > -solist_update_incremental (struct svr4_info *info, CORE_ADDR lm) > > +solist_update_incremental (struct svr4_info *info, CORE_ADDR debug_base, > > + CORE_ADDR lm) > > { > > struct so_list *tail; > > CORE_ADDR prev_lm; > > @@ -1727,8 +1771,15 @@ solist_update_incremental (struct svr4_info *info, CORE_ADDR lm) > > for (tail = info->solib_list; tail->next != NULL; tail = tail->next) > > /* Nothing. */; > > > > - lm_info_svr4 *li = (lm_info_svr4 *) tail->lm_info; > > - prev_lm = li->lm_addr; > > + /* Don't check shared libraries in other namespaces when updating > > + shared libraries in a new namespace. */ > > Shared libraries in other namespaces aren't being neglected though, > right? > > Assuming that's true, could you add a sentence or two to your comment > addressing this concern? > > Kevin > Hi Kevin, Markus has a much better patch to support multiple namespaces. -- H.J.