From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 99C8C3858C27 for ; Tue, 14 Mar 2023 16:41:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 99C8C3858C27 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.ibm.com Received: from pps.filterd (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 32EGCekg014166 for ; Tue, 14 Mar 2023 16:41:54 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : subject : to : references : from : in-reply-to : content-type : content-transfer-encoding : mime-version; s=pp1; bh=l4YH8M9HR1AcMNu2fV2bwp4k60M1a1yy3bcj7NB81KQ=; b=fWOelQIW7WuU2WkUxEscCRx6EoIchkaztXWrYfcL1r3tuJZBq7WoJ1jJyclZh0suFr6E BmkxzCJHbZqVVT8BTRwfhvWxK3OAkqp/dETRts12DnK3ECRsHK01s+NgnB7IvtlmdUL7 wtqW+JnvnR1sz77OmhVG8Q0wlOe5MdHs9krwXVAsf9MgvP8hDo1jBR4zZgdf733e13Hx 7nv34Bga8OMjbI6dmJ8GW7ePqayiw4S18RIkozWVZ2kV3W/5Ub/PNH/ANeM4/SZfHJyV z2J/TQM8Ge/ZeBjJPNhx2KpqV3y1U+ERPd9cg/s6w8gV9gijYNVRkrlWE6bxNSTbaSMs Ng== Received: from ppma02wdc.us.ibm.com (aa.5b.37a9.ip4.static.sl-reverse.com [169.55.91.170]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3pav82run2-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 14 Mar 2023 16:41:54 +0000 Received: from pps.filterd (ppma02wdc.us.ibm.com [127.0.0.1]) by ppma02wdc.us.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 32EE2BbY017186 for ; Tue, 14 Mar 2023 16:41:53 GMT Received: from smtprelay04.dal12v.mail.ibm.com ([9.208.130.102]) by ppma02wdc.us.ibm.com (PPS) with ESMTPS id 3p8h97mt67-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 14 Mar 2023 16:41:53 +0000 Received: from smtpav04.wdc07v.mail.ibm.com (smtpav04.wdc07v.mail.ibm.com [10.39.53.231]) by smtprelay04.dal12v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 32EGfpax11797244 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 14 Mar 2023 16:41:52 GMT Received: from smtpav04.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 9C5195805E; Tue, 14 Mar 2023 16:41:51 +0000 (GMT) Received: from smtpav04.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 6593D58050; Tue, 14 Mar 2023 16:41:51 +0000 (GMT) Received: from [9.163.35.102] (unknown [9.163.35.102]) by smtpav04.wdc07v.mail.ibm.com (Postfix) with ESMTP; Tue, 14 Mar 2023 16:41:51 +0000 (GMT) Message-ID: Date: Tue, 14 Mar 2023 11:41:51 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [PATCH] Added Redirects to longdouble error functions [BZ #29033] Content-Language: en-US To: Sachin Monga , libc-alpha@sourceware.org References: <20230304174541.403411-1-smonga@linux.ibm.com> From: Paul E Murphy In-Reply-To: <20230304174541.403411-1-smonga@linux.ibm.com> Content-Type: text/plain; charset=UTF-8; format=flowed X-TM-AS-GCONF: 00 X-Proofpoint-GUID: S4ewsWv08lRVQNvzEY4JbsVLA9Oa1J49 X-Proofpoint-ORIG-GUID: S4ewsWv08lRVQNvzEY4JbsVLA9Oa1J49 Content-Transfer-Encoding: 7bit X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22 definitions=2023-03-14_10,2023-03-14_02,2023-02-09_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 malwarescore=0 clxscore=1015 spamscore=0 bulkscore=0 suspectscore=0 lowpriorityscore=0 adultscore=0 priorityscore=1501 mlxscore=0 mlxlogscore=999 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000 definitions=main-2303140129 X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,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 3/4/23 11:45 AM, Sachin Monga via Libc-alpha wrote: > This patch redirects the error functions to the appropriate > longdouble variants which enables the compiler to optimize > for the abi ieeelongdouble. > > Signed-off-by: Sachin Monga > --- > misc/Makefile | 2 +- > misc/bits/error-ldbl.h | 53 ++++++++- > misc/sys/cdefs.h | 3 +- > misc/tst-ldbl-errorfptr.c | 114 ++++++++++++++++++++ > sysdeps/ieee754/ldbl-128ibm-compat/Makefile | 3 + > sysdeps/ieee754/ldbl-opt/Makefile | 5 + > 6 files changed, 176 insertions(+), 4 deletions(-) > create mode 100644 misc/tst-ldbl-errorfptr.c > > diff --git a/misc/Makefile b/misc/Makefile > index 1a09f777fa..9f42321206 100644 > --- a/misc/Makefile > +++ b/misc/Makefile > @@ -90,7 +90,7 @@ tests := tst-dirname tst-tsearch tst-fdset tst-mntent tst-hsearch \ > tst-preadvwritev2 tst-preadvwritev64v2 tst-warn-wide \ > tst-ldbl-warn tst-ldbl-error tst-dbl-efgcvt tst-ldbl-efgcvt \ > tst-mntent-autofs tst-syscalls tst-mntent-escape tst-select \ > - tst-ioctl > + tst-ioctl tst-ldbl-errorfptr > > tests-time64 := \ > tst-select-time64 \ > diff --git a/misc/bits/error-ldbl.h b/misc/bits/error-ldbl.h > index 599a7d6e06..638e030c96 100644 > --- a/misc/bits/error-ldbl.h > +++ b/misc/bits/error-ldbl.h > @@ -20,5 +20,54 @@ > # error "Never include directly; use instead." > #endif > > -__LDBL_REDIR_DECL (error) > -__LDBL_REDIR_DECL (error_at_line) > + > +extern void __REDIRECT_LDBL (__error_alias, (int __status, int __errnum, > + const char *__format, ...), > + error) > + __attribute__ ((__format__ (__printf__, 3, 4))); > +extern void __REDIRECT_LDBL (__error_noreturn, (int __status, int __errnum, > + const char *__format, ...), > + error) > + __attribute__ ((__noreturn__, __format__ (__printf__, 3, 4))); > + > + > +/* If we know the function will never return make sure the compiler > + realizes that, too. */ > +__extern_always_inline void > +error (int __status, int __errnum, const char *__format, ...) > +{ > + if (__builtin_constant_p (__status) && __status != 0) > + __error_noreturn (__status, __errnum, __format, __va_arg_pack ()); > + else > + __error_alias (__status, __errnum, __format, __va_arg_pack ()); > +} > + > + > +extern void __REDIRECT_LDBL (__error_at_line_alias, (int __status, int __errnum, > + const char *__fname, > + unsigned int __line, > + const char *__format, ...), > + error_at_line) > + __attribute__ ((__format__ (__printf__, 5, 6))); > +extern void __REDIRECT_LDBL (__error_at_line_noreturn, (int __status, int __errnum, > + const char *__fname, > + unsigned int __line, > + const char *__format, > + ...), > + error_at_line) > + __attribute__ ((__noreturn__, __format__ (__printf__, 5, 6))); > + > + > +/* If we know the function will never return make sure the compiler > + realizes that, too. */ > +__extern_always_inline void > +error_at_line (int __status, int __errnum, const char *__fname, > + unsigned int __line, const char *__format, ...) > +{ > + if (__builtin_constant_p (__status) && __status != 0) > + __error_at_line_noreturn (__status, __errnum, __fname, __line, __format, > + __va_arg_pack ()); > + else > + __error_at_line_alias (__status, __errnum, __fname, __line, > + __format, __va_arg_pack ()); > +} > diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h > index 23ec0ebd2a..285191482a 100644 > --- a/misc/sys/cdefs.h > +++ b/misc/sys/cdefs.h > @@ -569,6 +569,8 @@ > # define __LDBL_REDIR(name, proto) ... unused__ldbl_redir > # define __LDBL_REDIR_DECL(name) \ > extern __typeof (name) name __asm (__ASMNAME ("__" #name "ieee128")); > +# define __REDIRECT_LDBL(name, proto, alias) \ > + name proto __asm (__ASMNAME ("__" #alias "ieee128")) > > /* Alias name defined automatically, with leading underscores. */ > # define __LDBL_REDIR2_DECL(name) \ > @@ -586,7 +588,6 @@ > __LDBL_REDIR1_NTH (name, proto, __##alias##ieee128) > > /* Unused. */ > -# define __REDIRECT_LDBL(name, proto, alias) ... unused__redirect_ldbl > # define __LDBL_REDIR_NTH(name, proto) ... unused__ldbl_redir_nth > > # else > diff --git a/misc/tst-ldbl-errorfptr.c b/misc/tst-ldbl-errorfptr.c > new file mode 100644 > index 0000000000..4896694fb9 > --- /dev/null > +++ b/misc/tst-ldbl-errorfptr.c > @@ -0,0 +1,114 @@ > +/* Test for the long double conversions in error* functions > + when they are returned as function pointer. > + Copyright (C) 2018-2023 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + . */ > + > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > + > +struct tests > +{ > + void *callback; > + const char *expected; > +}; > + > +va_list args; > + > +typedef void (*error_func_t) (int ,int ,const char* ,...); > +typedef void (*error_at_line_func_t) (int ,int ,const char* , > + unsigned int ,const char* ,...); > + > +error_func_t > +get_error_func(void) { > + return &error; > +} > + > +error_at_line_func_t > +get_error_at_line_func(void) { > + return &error_at_line; > +} You are almost there. Under optimization, this gets inlined and transformed into a direct call when I test with AT15. Maybe a noinline attribute is sufficient to prevent this, but I think this test is difficult to verify it is testing the desired behavior. The test could be simplified further. You really want to verify the function pointer is correctly redirected. E.g for the nldbl case, you want to verify: get_error_at_line_func() == dlsym(RTLD_DEFAULT, "__nldb_error_at_line"); get_error_func() == dlsym(RTLD_DEFAULT, "__nldb_error"); The function and wrappers are already verified by existing tests, so it isn't necessary to test the function behind the pointer. > + > +static void > +callback_error (void *closure) > +{ > + errno = 0; > + error_func_t fp; > + fp = get_error_func(); > + fp (0, 0, "%Lf", (long double) -1); > +} > + > +static void > +callback_error_at_line (void *closure) > +{ > + errno = 0; > + error_at_line_func_t fpat; > + fpat = get_error_at_line_func(); > + fpat (0, 0, "", 0, "%Lf", (long double) -1); > +} > + > +static void > +do_one_test (void *callback, const char *expected, ...) > +{ > + struct support_capture_subprocess result; > + > + va_start (args, expected); > + > + /* Call 'callback', which fills in the output and error buffers. */ > + result = support_capture_subprocess (callback, NULL); > + > + /* Filter out the name of the program (which should always end with > + -errorfptr), so that the test case can be reused by ldbl-opt and > + ldbl-128ibm-compat. */ > + const char *needle = "-errorfptr:"; > + char *message; > + message = strstr (result.err.buffer, needle); > + printf("\nresult.err.buffer=%s",result.err.buffer); > + if (message == NULL) > + FAIL_EXIT1 ("test case error"); > + message += strlen (needle); > + > + /* Verify that the output message is as expected. */ > + TEST_COMPARE_STRING (message, expected); > + > + va_end (args); > +} > + > +static int > +do_test (void) > +{ > + /* Test if error and error_at_line are returned as function pointers > + * to the callback functions (BZ #29033). */ > + struct tests tests[] = { > + { &callback_error, " -1.000000\n" }, > + { &callback_error_at_line, ":0: -1.000000\n" } > + }; > + > + for (int i = 0; i < sizeof (tests) / sizeof (tests[0]); i++) > + { > + do_one_test (tests[i].callback, tests[i].expected, (long double) -1); > + } > + > + return 0; > +} > + > +#include > diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/Makefile b/sysdeps/ieee754/ldbl-128ibm-compat/Makefile > index 67d476383a..6aa73d2798 100644 > --- a/sysdeps/ieee754/ldbl-128ibm-compat/Makefile > +++ b/sysdeps/ieee754/ldbl-128ibm-compat/Makefile > @@ -252,6 +252,7 @@ CFLAGS-ieee128-qefgcvt_r.c += -mabi=ieeelongdouble -Wno-psabi -mno-gnu-attribute > tests-internal += tst-ibm128-warn tst-ieee128-warn > tests-internal += tst-ibm128-error tst-ieee128-error > tests-internal += tst-ibm128-efgcvt tst-ieee128-efgcvt > +tests-internal += tst-ibm128-errorfptr tst-ieee128-errorfptr > > $(objpfx)tst-ibm128-%.c: tst-ldbl-%.c > cp $< $@ > @@ -262,10 +263,12 @@ $(objpfx)tst-ieee128-%.c: tst-ldbl-%.c > CFLAGS-tst-ibm128-warn.c += -mabi=ibmlongdouble -Wno-psabi > CFLAGS-tst-ibm128-error.c += -mabi=ibmlongdouble -Wno-psabi > CFLAGS-tst-ibm128-efgcvt.c += -mabi=ibmlongdouble -Wno-psabi > +CFLAGS-tst-ibm128-errorfptr.c += -mabi=ibmlongdouble -Wno-psabi > > CFLAGS-tst-ieee128-warn.c += -mfloat128 -mabi=ieeelongdouble -Wno-psabi > CFLAGS-tst-ieee128-error.c += -mfloat128 -mabi=ieeelongdouble -Wno-psabi > CFLAGS-tst-ieee128-efgcvt.c += -mfloat128 -mabi=ieeelongdouble -Wno-psabi > +CFLAGS-tst-ieee128-errorfptr.c += -mfloat128 -mabi=ieeelongdouble -Wno-psabi > > tests-container += test-syslog-ieee128 test-syslog-ibm128 > CFLAGS-test-syslog-ieee128.c += -mfloat128 -mabi=ieeelongdouble -Wno-psabi > diff --git a/sysdeps/ieee754/ldbl-opt/Makefile b/sysdeps/ieee754/ldbl-opt/Makefile > index 1d01846476..1baf065654 100644 > --- a/sysdeps/ieee754/ldbl-opt/Makefile > +++ b/sysdeps/ieee754/ldbl-opt/Makefile > @@ -211,6 +211,7 @@ endif > ifeq ($(subdir), misc) > tests-internal += tst-nldbl-warn > tests-internal += tst-nldbl-error > +tests-internal += tst-nldbl-errorfptr > > $(objpfx)tst-nldbl-warn.c: tst-ldbl-warn.c > cp $< $@ > @@ -218,6 +219,10 @@ $(objpfx)tst-nldbl-warn.c: tst-ldbl-warn.c > $(objpfx)tst-nldbl-error.c: tst-ldbl-error.c > cp $< $@ > > +$(objpfx)tst-nldbl-errorfptr.c: tst-ldbl-errorfptr.c > + cp $< $@ > + > CFLAGS-tst-nldbl-warn.c += -mlong-double-64 > CFLAGS-tst-nldbl-error.c += -mlong-double-64 > +CFLAGS-tst-nldbl-errorfptr.c += -mlong-double-64 > endif