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-Status: No, score=-3.3 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, SPF_HELO_PASS,SPF_PASS,URIBL_BLACK shortcircuit=no autolearn=no 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 F14471F4B4 for ; Mon, 28 Sep 2020 14:05:55 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D2D653894C3B; Mon, 28 Sep 2020 14:05:54 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D2D653894C3B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1601301954; bh=JAUKt9NSTSEMYFHjozLVqjPGUb2pns+RHSDY5u5pS3Y=; h=To:Subject:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=pqxE/hoJqsiIPWhcr2eLyt2q99SS00VNXOEWq3SP40sPZSePJ3okEk4pwuaHORoLr 42NyUJMOcG5o2AuMSgyWRq9NFr/Yu5niNHmO5puSHTJzt7LLaeAiErzKqT6Rzoessh Esk76m/O3XaahaCkRNOaXK6qughSesQASTt+DqeA= Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id DEA0D3894C14 for ; Mon, 28 Sep 2020 14:05:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DEA0D3894C14 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-158-yqs_rn7iNEytmCar2eer3Q-1; Mon, 28 Sep 2020 10:05:35 -0400 X-MC-Unique: yqs_rn7iNEytmCar2eer3Q-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B5C151800D41; Mon, 28 Sep 2020 14:05:33 +0000 (UTC) Received: from oldenburg2.str.redhat.com (ovpn-114-84.ams2.redhat.com [10.36.114.84]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 06D9960DA0; Mon, 28 Sep 2020 14:05:32 +0000 (UTC) To: "H.J. Lu" Subject: Re: [PATCH 1/4] x86: Initialize CPU info via IFUNC relocation [BZ 26203] References: <20200918160709.949608-1-hjl.tools@gmail.com> <20200918160709.949608-2-hjl.tools@gmail.com> <87imby6obw.fsf@oldenburg2.str.redhat.com> Date: Mon, 28 Sep 2020 16:05:31 +0200 In-Reply-To: (H. J. Lu's message of "Mon, 28 Sep 2020 06:48:20 -0700") Message-ID: <87y2ku574k.fsf@oldenburg2.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain 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: Florian Weimer via Libc-alpha Reply-To: Florian Weimer Cc: "H.J. Lu via Libc-alpha" Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" * H. J. Lu: >> > diff --git a/sysdeps/x86/cacheinfo.c b/sysdeps/x86/cacheinfo.c >> > index 217c21c34f..7a325ab70e 100644 >> > --- a/sysdeps/x86/cacheinfo.c >> > +++ b/sysdeps/x86/cacheinfo.c >> >> > + assert (cpu_features->basic.kind != arch_kind_unknown); >> >> Why doesn't this assert fire occasionally? How do you ensure that > > See > > https://sourceware.org/bugzilla/show_bug.cgi?id=26203 > > It only happens in dlopen from a static executable. Sorry, I don't understand how this answers my question. Do you mean that for the non-static case, initialization has already happened. >> relocation processing is correctly ordered? > > cpu_features is also initialized by IFUNC relocation in ld.so which > is relocated before libc.so. Is that really true in all cases? Even if libc.so is preloaded? (Static dlopen probably ignores LD_PRELOAD.) Maybe put this information as a comment next to the assert? But since cacheinfo.os is linked into libc.so, I don't really think the assert is correct. >> > diff --git a/sysdeps/x86/dl-get-cpu-features.c b/sysdeps/x86/dl-get-cpu-features.c >> > index 5f9e46b0c6..da4697b895 100644 >> > --- a/sysdeps/x86/dl-get-cpu-features.c >> > +++ b/sysdeps/x86/dl-get-cpu-features.c >> > @@ -1,4 +1,4 @@ >> > -/* This file is part of the GNU C Library. >> > +/* Initialize CPU feature data via IFUNC relocation. >> > Copyright (C) 2015-2020 Free Software Foundation, Inc. >> > >> > The GNU C Library is free software; you can redistribute it and/or >> > @@ -18,6 +18,29 @@ >> > >> > #include >> > >> > +#ifdef SHARED >> > +# include >> > + >> > +/* NB: Normally, DL_PLATFORM_INIT calls init_cpu_features to initialize >> > + CPU features. But when loading ld.so inside of static executable, >> > + DL_PLATFORM_INIT isn't called. Call init_cpu_features by initializing >> > + a dummy function pointer via IFUNC relocation for ld.so. */ >> > +extern void __x86_cpu_features (void) attribute_hidden; >> > +const void (*__x86_cpu_features_p) (void) attribute_hidden >> > + = __x86_cpu_features; >> > + >> > +void >> > +_dl_x86_init_cpu_features (void) >> > +{ >> > + struct cpu_features *cpu_features = __get_cpu_features (); >> > + if (cpu_features->basic.kind == arch_kind_unknown) >> > + init_cpu_features (cpu_features); >> > +} >> > + >> > +__ifunc (__x86_cpu_features, __x86_cpu_features, NULL, void, >> > + _dl_x86_init_cpu_features); >> > +#endif >> > + >> > #undef __x86_get_cpu_features >> >> Why do we need both the conditional check and the function pointer hack? > > Because _dl_x86_init_cpu_features is called both indirectly and by IFUNC > reloc in dynamic executable, but it is only called by IFUNC reloc when > dlopen in static executable. I think we always need to call it eventually, as a dependency of filling in the cacheinfo data? >> I expect that one of the function pointers can go, probably the one >> here. The cache hierarchy data might be used by a string function that >> has not been selected by IFUNC. >> > > There are one IFUNC reloc in ld.so and the other in libc.so. We need > both. libc.so should not need the relocation hack because we have __libc_early_init, which is also called after static dlopen and before constructors. Thanks, Florian -- Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill