From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id A2C3E3881D38 for ; Mon, 13 Feb 2023 13:30:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A2C3E3881D38 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1676295049; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=uJhGT2WhWB/PPVa4IatbVDrfOLCnjzUEJL3j6uF2VH0=; b=MK1gZX2ldxApRttA0X/zA8SSbVF/lSPKYoajX2iIHQOTwiEtk8sxDi3CDI4dKVhK/4EK9T KkC1QyIQ/V//QtBg8F1NMpTrwhP63y8dm5f2Mi7VKxQ25KNr7+GPSykDPpeJZTLKAFBjNA N0L7FEqyRBbN8RxaVr3XZvZwD9AmZDY= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-517-w_GsmfwXNwWEWLefpmHqWQ-1; Mon, 13 Feb 2023 08:30:47 -0500 X-MC-Unique: w_GsmfwXNwWEWLefpmHqWQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 6CB8A885621; Mon, 13 Feb 2023 13:30:47 +0000 (UTC) Received: from oldenburg.str.redhat.com (unknown [10.2.16.7]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 49E122166B26; Mon, 13 Feb 2023 13:30:46 +0000 (UTC) From: Florian Weimer To: Cupertino Miranda via Libc-alpha Cc: Cupertino Miranda , "Jose E. Marchesi" , Elena Zannoni Subject: Re: [PING] [PATCH v2] Resolve-flockfile-funlockfile-differences References: <87ilhj3bsp.fsf@oracle.com> Date: Mon, 13 Feb 2023 14:30:44 +0100 In-Reply-To: <87ilhj3bsp.fsf@oracle.com> (Cupertino Miranda via Libc-alpha's message of "Fri, 06 Jan 2023 15:47:18 +0000") Message-ID: <877cwlelq3.fsf@oldenburg.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.6 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,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: * Cupertino Miranda via Libc-alpha: > This is a request for review of the patch sent by Patrick McGehearty > based on an inconsistency in flockfile and funlockfile definitions > (macro/function) when used in glibc code. At the same time this is a > request for comments on the topic based on some personal concerns on > the patch. > > After reading the description of the problem as explained in our bug > tracking I decided to make a more targeted patch, which will also help > to better explain the problem. > > Unfortunately at this stage I cannot functionally verify this patch > due to how long it was identified and fixed at Oracle. > > The patch defines a local wrapper to flockfile and funlockfile such > that both the function pointer passed to libc_cleanup_region_start and > the direct call would point to the same definition. So here's the general concern I have regarding _IO_flockfile. In libio/libio.h, we have: 197 extern void _IO_flockfile (FILE *) __THROW; 198 extern void _IO_funlockfile (FILE *) __THROW; 199 extern int _IO_ftrylockfile (FILE *) __THROW; 200 201 #define _IO_peekc(_fp) _IO_peekc_unlocked (_fp) 202 #define _IO_flockfile(_fp) /**/ 203 #define _IO_funlockfile(_fp) ((void) 0) 204 #define _IO_ftrylockfile(_fp) /**/ 205 #ifndef _IO_cleanup_region_start 206 #define _IO_cleanup_region_start(_fct, _fp) /**/ 207 #endif 208 #ifndef _IO_cleanup_region_end 209 #define _IO_cleanup_region_end(_Doit) /**/ 210 #endif 275 #ifdef _IO_MTSAFE_IO 276 # undef _IO_peekc 277 # undef _IO_flockfile 278 # undef _IO_funlockfile 279 # undef _IO_ftrylockfile 280 281 # define _IO_peekc(_fp) _IO_peekc_locked (_fp) 282 # define _IO_flockfile(_fp) \ 283 if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_lock_lock (*(_fp)->_lock 283 ) 284 # define _IO_funlockfile(_fp) \ 285 if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_lock_unlock (*(_fp)->_lo 285 ck) 286 #endif /* _IO_MTSAFE_IO */ As you can see, the exact definition depends on _IO_MTSAFE_IO. It is set in Makeconfig, but again conditionally: 1397 # A sysdeps Makeconfig fragment may set libc-reentrant to yes. 1398 ifeq (yes,$(libc-reentrant)) 1399 defines += -D_LIBC_REENTRANT 1400 1401 libio-mtsafe = -D_IO_MTSAFE_IO 1402 endif $(libc-reentrant) should always evaluate to yes because it is set as such in sysdeps/pthread/Makeconfig. On the other hand, this construct requires further opt-in by including libio-mtsafe in CFLAGS or CPPFLAGS. There seem to be uses that are appear to be unconvered by -D_IO_MTSAFE_IO, for example the malloc subdirectory. Disassembling __malloc_stats seems to confirm that. Maybe you can confirm based on your case notes whether this issue might becaused by parts of glibc stomping over the flags due to the inconsistency described above. Or is it because the application sets __fsetlocking (FSETLOCKING_BYCALLER) concurrently? I don't see how we can support the latter, that really seems to be an application bug. Thanks, Florian