From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x2b.google.com (mail-oa1-x2b.google.com [IPv6:2001:4860:4864:20::2b]) by sourceware.org (Postfix) with ESMTPS id B789E3858D32 for ; Mon, 19 Sep 2022 18:31:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B789E3858D32 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-oa1-x2b.google.com with SMTP id 586e51a60fabf-12803ac8113so633252fac.8 for ; Mon, 19 Sep 2022 11:31:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date; bh=p+d+F5ko4p1cyLSE7VbVmJbfnWdxJDiOrT9mRfn5Tn4=; b=iGpwXJcqQ/TnElSIoWkmD8jyTiIa6eqe/s2SoAVbJ96V2waKKkWUeBTpV/3/rMk31B 2kY0EKgwxPNGO0VlukNY7ubCtzlcHphRbFU51qtPwRWq1UYu4WtnraqLULksNDGSgAj9 sR5pjFn1RSk8FXT3TNcufQj+pM4juP54TEEPFsm+CDCuWMSw7T/Si7TfTAEPYQEN4UT4 vapT+KXiKuxazFLt9sw/+mlBtY5g5cPu7fcynJu35GaKmXxrIqnYmfChUszbG87EMEgb OD1Gfc6HQ5/EC8u4kN5RWmzO1EMxySLlRA7iYhqe4JH3dOW0+3xMCfQIS98D2UJatT18 1/Wg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date; bh=p+d+F5ko4p1cyLSE7VbVmJbfnWdxJDiOrT9mRfn5Tn4=; b=hRYoFwKdgZzilJwiepHNW1CR/Zg8z6EkdS6xZFQcf9vkZ3C0z8bX81TCWhRFDM2PCM vEcsCcCXEzlGX/38VcpyxfEx9Twn2T2JRC88g6iMb3Y8ziQSz3CCuiB/NjetK9OP01Qd 22W/ttIxi6Wq7HcFBKw++PbBqVkAC6mdwFOjqEKYs+VD7C7iSxjtbI5QEwXLyJZhEUKH JZDT+iS02DT1exk2Y7HpDPC2fOL2jQFz6DqIO4soF/IHQA/XaHIN1nPcQc/26OVZG8hg CMtgAk7cgFdndGyBYzGfBwlMY0zrXujp1cY7nvO12eOrvN1MWz5yQVHYdTZhYjJPeklm tE9A== X-Gm-Message-State: ACgBeo1iaQG04KQguYBGl04PfFoeCEW/XFWUVc6dXdGRii83I0ALU3fp wCL0LGQUdBbfsxtCtuKoQu7gm8VhbvjZFO1P X-Google-Smtp-Source: AA6agR72aoj6shRMF6TfZDoJ5XcP07ewsR6JSlpevo7mp9JTunYbVrQ/zc/femf0UJHTeaxQPnZdpQ== X-Received: by 2002:a05:6870:3484:b0:12b:1a11:e96e with SMTP id n4-20020a056870348400b0012b1a11e96emr16089379oah.191.1663612295035; Mon, 19 Sep 2022 11:31:35 -0700 (PDT) Received: from ?IPV6:2804:1b3:a7c1:c266:6474:c804:752d:521c? ([2804:1b3:a7c1:c266:6474:c804:752d:521c]) by smtp.gmail.com with ESMTPSA id s10-20020acadb0a000000b0034d14c6ce3dsm13252644oig.16.2022.09.19.11.31.33 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 19 Sep 2022 11:31:34 -0700 (PDT) Message-ID: <8ec64f4e-81ff-5a95-6a50-7166e9f44d3b@linaro.org> Date: Mon, 19 Sep 2022 15:31:32 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH v2] elf: Implement force_first handling in _dl_sort_maps_dfs (bug 28937) Content-Language: en-US To: Florian Weimer , libc-alpha@sourceware.org References: <87bkrtasv6.fsf@oldenburg.str.redhat.com> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: <87bkrtasv6.fsf@oldenburg.str.redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 06/09/22 04:01, Florian Weimer via Libc-alpha wrote: > The implementation in _dl_close_worker requires that the first > element of l_initfini is always this very map (“We are always the > zeroth entry, and since we don't include ourselves in the > dependency analysis start at 1.”). Rather than fixing that > assumption, this commit adds an implementation of the force_first > argument to the new dependency sorting algorithm. This also means > that the directly dlopen'ed shared object is always initialized last, > which is the least surprising behavior in the presence of cycles. LGTM, thanks. Reviewed-by: Adhemerval Zanella > > --- > v2: Incorporate Adhemerval's review comments. Retested on i386-linux-gnu > and x86_64-linux-gnu. > elf/dl-sort-maps.c | 32 +++++++++++++++++++++++--------- > elf/dso-sort-tests-1.def | 7 +++++++ > 2 files changed, 30 insertions(+), 9 deletions(-) > > diff --git a/elf/dl-sort-maps.c b/elf/dl-sort-maps.c > index 5b550b1e94..3e2a6a584e 100644 > --- a/elf/dl-sort-maps.c > +++ b/elf/dl-sort-maps.c > @@ -182,8 +182,9 @@ dfs_traversal (struct link_map ***rpo, struct link_map *map, > > static void > _dl_sort_maps_dfs (struct link_map **maps, unsigned int nmaps, > - bool force_first __attribute__ ((unused)), bool for_fini) > + bool force_first, bool for_fini) > { > + struct link_map *first_map = maps[0]; > for (int i = nmaps - 1; i >= 0; i--) > maps[i]->l_visited = 0; > > @@ -208,14 +209,6 @@ _dl_sort_maps_dfs (struct link_map **maps, unsigned int nmaps, > Adjusting the order so that maps[0] is last traversed naturally avoids > this problem. > > - Further, the old "optimization" of skipping the main object at maps[0] > - from the call-site (i.e. _dl_sort_maps(maps+1,nmaps-1)) is in general > - no longer valid, since traversing along object dependency-links > - may "find" the main object even when it is not included in the initial > - order (e.g. a dlopen()'ed shared object can have circular dependencies > - linked back to itself). In such a case, traversing N-1 objects will > - create a N-object result, and raise problems. > - > To summarize, just passing in the full list, and iterating from back > to front makes things much more straightforward. */ > > @@ -274,6 +267,27 @@ _dl_sort_maps_dfs (struct link_map **maps, unsigned int nmaps, > } > > memcpy (maps, rpo, sizeof (struct link_map *) * nmaps); > + > + /* Skipping the first object at maps[0] is not valid in general, > + since traversing along object dependency-links may "find" that > + first object even when it is not included in the initial order > + (e.g., a dlopen'ed shared object can have circular dependencies > + linked back to itself). In such a case, traversing N-1 objects > + will create a N-object result, and raise problems. Instead, > + force the object back into first place after sorting. This naive > + approach may introduce further dependency ordering violations > + compared to rotating the cycle until the first map is again in > + the first position, but as there is a cycle, at least one > + violation is already present. */ > + if (force_first && maps[0] != first_map) > + { > + int i; > + for (i = 0; maps[i] != first_map; ++i) > + ; > + assert (i < nmaps); > + memmove (&maps[1], maps, i * sizeof (maps[0])); > + maps[0] = first_map; > + } > } > > void > diff --git a/elf/dso-sort-tests-1.def b/elf/dso-sort-tests-1.def > index 5f7f18ef27..4bf9052db1 100644 > --- a/elf/dso-sort-tests-1.def > +++ b/elf/dso-sort-tests-1.def > @@ -64,3 +64,10 @@ output: b>a>{} tst-bz15311: {+a;+e;+f;+g;+d;%d;-d;-g;-f;-e;-a};a->b->c->d;d=>[ba];c=>a;b=>e=>a;c=>f=>b;d=>g=>c > output(glibc.rtld.dynamic_sort=1): {+a[d>c>b>a>];+e[e>];+f[f>];+g[g>];+d[];%d(b(e(a()))a()g(c(a()f(b(e(a()))))));-d[];-g[];-f[];-e[];-a[ output(glibc.rtld.dynamic_sort=2): {+a[d>c>b>a>];+e[e>];+f[f>];+g[g>];+d[];%d(b(e(a()))a()g(c(a()f(b(e(a()))))));-d[];-g[];-f[];-e[];-a[ + > +# Test that even in the presence of dependency loops involving dlopen'ed > +# object, that object is initialized last (and not unloaded prematurely). > +# Final destructor order is indeterminate due to the cycle. > +tst-bz28937: {+a;+b;-b;+c;%c};a->a1;a->a2;a2->a;b->b1;c->a1;c=>a1 > +output(glibc.rtld.dynamic_sort=1): {+a[a2>a1>a>];+b[b1>b>];-b[];%c(a1());} +output(glibc.rtld.dynamic_sort=2): {+a[a2>a1>a>];+b[b1>b>];-b[];%c(a1());} > base-commit: dbb75513f5cf9285c77c9e55777c5c35b653f890 >