From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS3215 2.6.0.0/16 X-Spam-Status: No, score=-4.2 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI,NICE_REPLY_A, RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 7251A1F8C6 for ; Wed, 14 Jul 2021 13:53:18 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4DC053984076 for ; Wed, 14 Jul 2021 13:53:17 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4DC053984076 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1626270797; bh=xmvSYQLN7UYsnD5v2py3VS4CIRn1/bzvqnL/8CQJmjc=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=JaGyq97vjsxmmEVAhKPpGJta7pEdvlf7KYVRd1quxc9uykZHIN62N36gU6id3qtCZ mWtWQy4Xx4CVFtp6b+V2DQl1CssSf2ixdlPh7k3Skgsn5UzdZ3bKs5OCBp2e40R8Os 0rYN5TrRm7n3s4v4U9VRaHsd/OMv+9NEnYBOJaH4= Received: from mail-pj1-x102e.google.com (mail-pj1-x102e.google.com [IPv6:2607:f8b0:4864:20::102e]) by sourceware.org (Postfix) with ESMTPS id AF88C3860C37 for ; Wed, 14 Jul 2021 13:52:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AF88C3860C37 Received: by mail-pj1-x102e.google.com with SMTP id bt15so1617862pjb.2 for ; Wed, 14 Jul 2021 06:52:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=xmvSYQLN7UYsnD5v2py3VS4CIRn1/bzvqnL/8CQJmjc=; b=JoOaCzy4g2KVHqcIAHIhDPwv7jaC2z/R3xubWqvJ8JMPhN6gRd5NHnbQrldGdXFibZ zZbIdqOJ5QHWRxk+0lUKL38QSwp9PsQYO+/2MlNQczFCtzb9yG4Z51CQ3QZ6MwZ3VFmG IS6NUJxYGTgCJuF1xddQ7ynIFTBhTSo+cF/WqET9rXADMD9nsvHf432O45k3nHcevUFa hw9h5AS7ht7E7Le0OS4eCJk9tsQhhz/LETat1CBJVrDg3W54kemjnPLSRqKJ35utq/Kf xElg9UQK1KXtYZTa31OJDOyfDkVB5R5QqiCH9z3caAb/5lcCYTXycPW2VANFAkZjmCkk b70Q== X-Gm-Message-State: AOAM5321KqGITcNrk7qEdUCAxVvAVQf9FmxHZC289k3HvL8C7/lWLoGU IBs924Avab0vCnJTRgd1FvW3c4B1WpQ+gA== X-Google-Smtp-Source: ABdhPJwLncF669vAsU0PWaA2QvBAh1VF6lWgE+Uv66gLqH/b1ZggQzWC5y1Bcm3oj1oepGBGDeheIQ== X-Received: by 2002:a17:90a:4cc4:: with SMTP id k62mr3950279pjh.110.1626270772512; Wed, 14 Jul 2021 06:52:52 -0700 (PDT) Received: from [192.168.1.108] ([177.194.59.218]) by smtp.gmail.com with ESMTPSA id f6sm2858861pfj.28.2021.07.14.06.52.50 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 14 Jul 2021 06:52:52 -0700 (PDT) Subject: Re: [PATCH v3] elf: Fix DTV gap reuse logic (BZ #27135) To: Szabolcs Nagy , Carlos O'Donell References: <20210709135001.505521-1-adhemerval.zanella@linaro.org> <20210709150512.GT14854@arm.com> Message-ID: <0c977f4a-248d-c035-a615-852adee670a1@linaro.org> Date: Wed, 14 Jul 2021 10:52:49 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210709150512.GT14854@arm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Adhemerval Zanella via Libc-alpha Reply-To: Adhemerval Zanella Cc: libc-alpha@sourceware.org Errors-To: libc-alpha-bounces+e=80x24.org@sourceware.org Sender: "Libc-alpha" On 09/07/2021 12:05, Szabolcs Nagy wrote: > The 07/09/2021 10:50, Adhemerval Zanella wrote: >> Changes from previous version: >> >> - Fix commit message and add a line about the bug fixes. >> - Use atomic operation while setting the slotinfo. >> - Use test_verbose on tst-tls20.c. >> >> --- >> >> This is updated version of the 572bd547d57a (reverted by 40ebfd016ad2) >> that fixes the _dl_next_tls_modid issues. >> >> This issue with 572bd547d57a patch is the DTV entry will be only >> update on dl_open_worker() with the update_tls_slotinfo() call after >> all dependencies are being processed by _dl_map_object_deps(). However >> _dl_map_object_deps() itself might call _dl_next_tls_modid(), and since >> the _dl_tls_dtv_slotinfo_list::map is not yet set the entry will be >> wrongly reused. >> >> This patch fixes by renaming the _dl_next_tls_modid() function to >> _dl_assign_tls_modid() and by passing the link_map so it can set >> the slotinfo value so a so subsequente _dl_next_tls_modid() call will >> see the entry as allocated. > > this paragraph still has 'so a so subsequente' > and i would add the bug number into the first sentence. Fixed. > >> >> The intermediary value is cleared up on remove_slotinfo() for the case >> a library fails to load with RTLD_NOW. >> >> This patch fixes BZ #27135. >> >> Checked on x86_64-linux-gnu. > > the patch looks ok to me, with the commit message > and the comment issue below fixed. > > Reviewed-by: Szabolcs Nagy Carlos, is it for push? > >> + >> +/* The following test check DTV gaps handling with shared libraries that has >> + dependencies. It defines 5 different sets: >> + >> + 1. Single dependency: >> + mod0 -> mod1 >> + 2. Double dependency: >> + mod2 -> [mod3,mod4] >> + 3. Double dependency with each dependency depent of another module: >> + mod5 -> [mod6,mod7] -> mod8 >> + 4. Long chain with one double dependency in the middle: >> + mod9 -> [mod10, mod11] -> mod12 -> mod13 >> + 5. Long chain with two double depedencies in the middle: >> + mod15 -> mod15 -> [mod16, mod17] >> + mod15 -> [mod18, mod19] > > mod14 -> ... Fixed. > >> + >> + This does not cover all the possible gaps and configuration, but it >> + should check if different dynamic shared sets are placed correctly in >> + different gaps configurations. */ >> + >> +static int >> +nmodules (uint32_t v) >> +{ >> + unsigned int r = 0; >> + while (v >>= 1) >> + r++; >> + return r + 1; >> +} >> + >> +static inline bool >> +is_mod_set (uint32_t g, uint32_t n) >> +{ >> + return (1U << (n - 1)) & g; >> +} >> + >> +static void >> +print_gap (uint32_t g) >> +{ >> + if (!test_verbose) >> + return; >> + printf ("gap: "); >> + int nmods = nmodules (g); >> + for (int n = 1; n <= nmods; n++) >> + printf ("%c", ((1 << (n - 1)) & g) == 0 ? 'G' : 'M'); >> + printf ("\n"); >> +} >> + >> +static void >> +do_test_dependency (void) >> +{ >> + /* Maps the module and its dependencies, use thread to access the TLS on >> + each loaded module. */ >> + static const int tlsmanydeps0[] = { 1 }; >> + static const int tlsmanydeps1[] = { 3, 4 }; >> + static const int tlsmanydeps2[] = { 6, 7, 8 }; >> + static const int tlsmanydeps3[] = { 10, 11, 12 }; >> + static const int tlsmanydeps4[] = { 15, 16, 17, 18, 19 }; >> + static const struct tlsmanydeps_t >> + { >> + int modi; >> + int ndeps; >> + const int *deps; >> + } tlsmanydeps[] = >> + { >> + { 0, array_length (tlsmanydeps0), tlsmanydeps0 }, >> + { 2, array_length (tlsmanydeps1), tlsmanydeps1 }, >> + { 5, array_length (tlsmanydeps2), tlsmanydeps2 }, >> + { 9, array_length (tlsmanydeps3), tlsmanydeps3 }, >> + { 14, array_length (tlsmanydeps4), tlsmanydeps4 }, >> + };