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=-4.2 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (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 423491F4B4 for ; Wed, 14 Oct 2020 15:14:18 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5DC503943418; Wed, 14 Oct 2020 15:14:17 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5DC503943418 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1602688457; bh=Od8HXlD6DW6ShLfG45n7V6T3PZri53KRw4MRcxbz458=; h=Date:To:Subject:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=NfsJFMdCBkKUhuo8Ho7QZrI5Z+A21Srv6uWjk/GO6jlq0g0B9qj96N8ksVwsQuaTV xXcezq32lzonJWWPfiBi6rve8ezxy/jvNwenZrgsiEuvBi4TI//hXB7ZH404Babbar XomeUd5SHYDL0zXdKu51o0rQDrcRSi+UbBjTMTm0= Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id DF98F3857C5F for ; Wed, 14 Oct 2020 15:14:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DF98F3857C5F Received: from pps.filterd (m0098420.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 09EF1Iqt078954 for ; Wed, 14 Oct 2020 11:14:14 -0400 Received: from pps.reinject (localhost [127.0.0.1]) by mx0b-001b2d01.pphosted.com with ESMTP id 34635k9utt-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Wed, 14 Oct 2020 11:14:14 -0400 Received: from m0098420.ppops.net (m0098420.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.36/8.16.0.36) with SMTP id 09EF1Y9P081157 for ; Wed, 14 Oct 2020 11:14:13 -0400 Received: from ppma03dal.us.ibm.com (b.bd.3ea9.ip4.static.sl-reverse.com [169.62.189.11]) by mx0b-001b2d01.pphosted.com with ESMTP id 34635k9ut9-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 14 Oct 2020 11:14:13 -0400 Received: from pps.filterd (ppma03dal.us.ibm.com [127.0.0.1]) by ppma03dal.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 09EFBl6f015089; Wed, 14 Oct 2020 15:14:13 GMT Received: from b03cxnp08028.gho.boulder.ibm.com (b03cxnp08028.gho.boulder.ibm.com [9.17.130.20]) by ppma03dal.us.ibm.com with ESMTP id 3434k92nrf-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 14 Oct 2020 15:14:12 +0000 Received: from b03ledav001.gho.boulder.ibm.com (b03ledav001.gho.boulder.ibm.com [9.17.130.232]) by b03cxnp08028.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 09EFEBeP64815402 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 14 Oct 2020 15:14:11 GMT Received: from b03ledav001.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id B8BA66E050; Wed, 14 Oct 2020 15:14:11 +0000 (GMT) Received: from b03ledav001.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 3E7F46E04E; Wed, 14 Oct 2020 15:14:11 +0000 (GMT) Received: from li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com (unknown [9.80.220.33]) by b03ledav001.gho.boulder.ibm.com (Postfix) with ESMTPS; Wed, 14 Oct 2020 15:14:11 +0000 (GMT) Date: Wed, 14 Oct 2020 10:14:08 -0500 To: Florian Weimer Subject: Re: [PATCH 1/3] elf: Add glibc-hwcaps support for LD_LIBRARY_PATH Message-ID: <20201014151408.GA293362@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com> References: <20201013162806.GA247259@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com> <87v9fcoqn0.fsf@oldenburg2.str.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87v9fcoqn0.fsf@oldenburg2.str.redhat.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235, 18.0.687 definitions=2020-10-14_08:2020-10-14, 2020-10-14 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxlogscore=999 malwarescore=0 spamscore=0 adultscore=0 bulkscore=0 clxscore=1015 phishscore=0 lowpriorityscore=0 priorityscore=1501 mlxscore=0 suspectscore=0 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2010140106 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: "Paul A. Clarke via Libc-alpha" Reply-To: "Paul A. Clarke" Cc: libc-alpha@sourceware.org Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" On Wed, Oct 14, 2020 at 03:58:59PM +0200, Florian Weimer via Libc-alpha wrote: > * Paul A. Clarke: > > >> +/* Returns true if the colon-separated HWCAP list HWCAPS contains the > >> + capability NAME (with length NAME_LENGTH). If HWCAPS is NULL, the > >> + function returns true. */ > >> +_Bool _dl_hwcaps_contains (const char *hwcaps, const char *name, > >> + size_t name_length) attribute_hidden; > >> + > >> +/* Colon-separated string of glibc-hwcaps subdirectories, without the > >> + "glibc-hwcaps/" prefix. The most preferred subdirectory needs to > >> + be listed first. */ > >> +extern const char _dl_hwcaps_subdirs[] attribute_hidden; > > > > Should we note the limitations, that the number of subdirectories must > > be <= 32? > > Fair enough, I'm going to expand the comment. OK. > >> +/* Returns a bitmap of active subdirectories in _dl_hwcaps_subdirs. > >> + Bit 0 (the LSB) corresponds to the first substring in > >> + _dl_hwcaps_subdirs, bit 1 to the second substring, and so on. > >> + There is no direct correspondence between HWCAP bitmasks and this > >> + bitmask. */ > >> +uint32_t _dl_hwcaps_subdirs_active (void) attribute_hidden; > >> + > >> +/* Returns a bitmask that marks the last ACTIVE subdirectories in a > >> + _dl_hwcaps_subdirs_active string (containing SUBDIRS directories in > >> + total) as active. Intended for use in _dl_hwcaps_subdirs_active > >> + implementations. */ > >> +static inline uint32_t > >> +_dl_hwcaps_subdirs_build_bitmask (int subdirs, int active) > >> +{ > >> + /* Leading subdirectories that are not active. */ > >> + int inactive = subdirs - active; > >> + if (inactive == 32) > >> + return 0; > >> + > >> + uint32_t mask; > >> + if (subdirs < 32) > >> + mask = (1U << subdirs) - 1; > >> + else > >> + mask = -1; > >> + return mask ^ ((1U << inactive) - 1); > > > > Should we validate any inputs in this function, that: > > - subdirs <= 32 > > - active <= 32 and active <= subdirs > > Violating these preconditions result in undefined behavior at compile > time, so I expected GCC (and Clang) to warn about that. But no such > luck there. I asked two colleagues about what we can do on the GCC > side. I do think GCC should warn about this under -Wall because it > returns a totally made-up value. > > I think if we can get that fixed in GCC mainline, we don't have to > clutter our code with asserts. > With sufficient visibility, GCC can issue such warnings: test.c:4:34: warning: left shift count >= width of type [-Wshift-count-overflow] printf ("%x << 32 = %x\n", r, r << 32); ...so maybe it already "just works", but your patches don't exercise that because they aren't broken. :-) > > While validating this function, I created an equivalent: > > if (subdirs == 0) return 0; > > if (active == 32) return -1; > > uint32_t mask = -1; > > /* Mask to include all subdirs. */ > > mask >>= 32 - s; > > /* Unmask all inactive. */ > > mask &= ~(mask >> a); > > return mask; > > ...I found this more readable, but it's subjective. > > Yeah, what we really want here is LDB or DPB, or Erlang's bit syntax. 8-/ > > > Also, this routine makes a broad assumption that active subdirectories > > are all contiguous and at the head of the list. Maybe this should be > > renamed _dl_hwcaps_subdirs_build_range_bitmask (or ..._top_range_...), > > with an updated comment that reflects its limited use-case. > > That makes sense. I think we can delay that until such a targer > arrives. I'm not sure what you mean here. PC