From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x42a.google.com (mail-pf1-x42a.google.com [IPv6:2607:f8b0:4864:20::42a]) by sourceware.org (Postfix) with ESMTPS id C9E15385828D for ; Mon, 27 Nov 2023 13:46:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C9E15385828D Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C9E15385828D Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::42a ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701092797; cv=none; b=j+8xzvxb0Ap/b/WLvt3s3sEYpbZuFwHUAlVBHVKf9eMYTgz2qckqohU+NepDh5xYhFYzJ+s5iF4Pf5V2U2fs9Kmw2fJXnFG6IpxBcjCV1fpe3J7i0LiFEmvzZQWXLwRPs3/Pi8dmQc4ADfxL8r884ZLEASEs+3SWKOYn4HJceks= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701092797; c=relaxed/simple; bh=zmqduUCC/ZoE1xJyiJYlhQaI9DI9LpxQBUNNJ73Jfxg=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:From:To; b=HvGqHeWAL6a7hiVCoR/LOPRbMlCmGVN/cN0GlNhc1xb7lpP8SvJ96LpThijlboUyEsV6Kozw17I733/T8XY3AJjpN/I4iTMNQ5eStplSRwo0JyoFpz39DKZFHj/0wSYDMXT/29oYg/Jl2UJA0GYweBYnHlVGSMpUlMPHUgzpUeQ= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pf1-x42a.google.com with SMTP id d2e1a72fcca58-6bd0e1b1890so3410059b3a.3 for ; Mon, 27 Nov 2023 05:46:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1701092790; x=1701697590; darn=sourceware.org; h=content-transfer-encoding:in-reply-to:organization:references:to :from:content-language:subject:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=4foDFLppppxRCWKs32EYmgbZzoBQ+bh0DHwcnRwUJJk=; b=mdb8UItVNTPXe0+z7RVBLFnl+AlEMTPFLnfnTniWof75oP13GCn/plOT+um8y5Ntg8 jqp/e+U0uBVcRMjHHvSMbdDcEgiErcETQN6iblQPT6s/X7z5baP3HOIv0fWymO6topaa FPS9kXpgZuQL5BANz6nHSx2YBhYIeCj1bRNyaUCSUrZmOT3PCmTIr8M/38EuxE+/R2dv DJjid34BAd7PGlxSnMS3aHvHAb+xh+a9HxyULfb/NiKJrEsqYHTO3vGAgI3inc9GMqlg 4uvCCcRq/rImGucFCO6M2q43x70dYJ/aUWZCF8jcBwfXNZ3Eva2ShQixLUfg2xxZMaHF xS/g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701092790; x=1701697590; h=content-transfer-encoding:in-reply-to:organization:references:to :from:content-language:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=4foDFLppppxRCWKs32EYmgbZzoBQ+bh0DHwcnRwUJJk=; b=d189gxhbgvB3eLIEAPpV0QvNel+mU/wPcfaEnNwcfgPFB0jjDwCbZE/QsL0WNeq6ns 2ES/FWn2gwb+zsUKlcHArD1NdxrQghnQJGwQSP89Gc12hpm0qSjIcA55O1YywJUjoIV7 ky8tmwPP9awhn2Xp0Lvoh9TX3sV5ymGWHo6IMOG9FUAT5dHdk/2hCMOqX0JR5gxYCw7A BLRE/FicTyV/WoWAvM3dgzuBbStL1kgpV9uyIyKys/R2SCKV59AEjqugjdocgUSZXweW 2di2S5xjjtdfR1+g+nsp8E68capVDR98xtlDnq4sz3BtwtDgRGUK4DIxyLdkEf98cmWQ IzOw== X-Gm-Message-State: AOJu0YzRmrz68ZZUTU6u2oHbnhrlosec0AakFd+I7EMynXLLhU0TZwlW kM9bmOLiPy5OCxH4xUHFMRNEhM+OLOb7nmlSbC4+Xw== X-Google-Smtp-Source: AGHT+IEKptT2Dyt2h+B97ghxdxrNUZYvBwPUWV3z0SbQ/eu7P/Z336TzvydIpOo4WXtaY4ZZYXhwXQ== X-Received: by 2002:a05:6a20:1608:b0:18c:b6:ab4f with SMTP id l8-20020a056a20160800b0018c00b6ab4fmr10472329pzj.48.1701092790303; Mon, 27 Nov 2023 05:46:30 -0800 (PST) Received: from ?IPV6:2804:1b3:a7c2:bcdb:c5ce:4230:f416:58e2? ([2804:1b3:a7c2:bcdb:c5ce:4230:f416:58e2]) by smtp.gmail.com with ESMTPSA id e64-20020a636943000000b005b32d6b4f2fsm7700345pgc.81.2023.11.27.05.46.28 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 27 Nov 2023 05:46:29 -0800 (PST) Message-ID: <578ae9cd-a47a-47a6-b17d-646ed3d97e46@linaro.org> Date: Mon, 27 Nov 2023 10:46:27 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 1/7] powerpc: Do not raise exception traps for fesetexcept/fesetexceptflag (BZ 30988) Content-Language: en-US From: Adhemerval Zanella Netto To: Carlos O'Donell , libc-alpha@sourceware.org, Bruno Haible References: <20231106132713.953501-1-adhemerval.zanella@linaro.org> <20231106132713.953501-2-adhemerval.zanella@linaro.org> <6130f4c9-dab2-6f8e-5bc5-902b5a48e2dc@redhat.com> <5e031d35-5d3e-49a7-b354-809bb4a1dc8f@linaro.org> <6e4ff3c5-8504-79a4-8865-0239b0cd7185@redhat.com> <8aaf2565-5310-44aa-a331-6d12b26d2274@linaro.org> <3052e518-dac2-4b23-a070-787db5e13bf2@linaro.org> <6e52fa2f-9198-4a0a-9c77-80a1eccbd347@linaro.org> <0f892926-e336-f08d-c96e-0ad38d4ce75a@redhat.com> <0490564f-e7b0-9a62-7940-90dd32665143@redhat.com> <600c6e8e-1283-47fd-af00-db1204629fa4@linaro.org> Organization: Linaro In-Reply-To: <600c6e8e-1283-47fd-af00-db1204629fa4@linaro.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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 24/11/23 15:46, Adhemerval Zanella Netto wrote: > > > On 24/11/23 15:15, Carlos O'Donell wrote: >> On 11/24/23 12:53, Adhemerval Zanella Netto wrote: >>> I think the simplification applies here, because the issue is a >>> powerpc/x87 architecture limitation here setting the floating-point >>> register status will trigger a floating point exception (x87 would >>> trigger in the next floating point operation, but it is essentially >>> the same issue). >> >> In which case I'd rather the test code assert that the two macros can't be >> used that way in the test with a macro check or static assert that >> checks 'EXCEPTION_SET_FORCES_TRAP && !FLOATING_TESTS(float)' is true. > > I think you mean EXCEPTION_TESTS (float), and it seems reasonable: > > _Static_assert (!(EXCEPTION_SET_FORCES_TRAP && !EXCEPTION_TESTS(float)), > "EXCEPTION_SET_FORCES_TRAP only makes sense if the " > "architecture suports exceptions"); > >> >>> So the fesetexcept/fesetexceptflag would either: >>> >>> 1. Raise a floating point exception, aborting the testcase (current >>> code). >>> 2. Fail where it should not. >>> 3. Rail where it should (powerpc/x87). >>> 4. Succeeds. >> >> 5. Succeed where it should not (powerpc/x87) ? > > Indeed. > >> >>> >>> So 1. and 2. are considered a regression, where 3. and 4. is the >>> expected result. >> >> ... and 5? > > Yes, and 1., 2., and 5. should be considered a regression. Hi Carlos, Updated patch below: -- >From c10d1d59321d3fce0c532304b428dcef3c8cbb3a Mon Sep 17 00:00:00 2001 From: Adhemerval Zanella Date: Tue, 24 Oct 2023 08:37:14 -0300 Subject: [PATCH 1/7] powerpc: Do not raise exception traps for fesetexcept/fesetexceptflag (BZ 30988) According to ISO C23 (7.6.4.4), fesetexcept is supposed to set floating-point exception flags without raising a trap (unlike feraiseexcept, which is supposed to raise a trap if feenableexcept was called with the appropriate argument). This is a side-effect of how we implement the GNU extension feenableexcept, where feenableexcept/fesetenv/fesetmode/feupdateenv might issue prctl (PR_SET_FPEXC, PR_FP_EXC_PRECISE) depending of the argument. And on PR_FP_EXC_PRECISE, setting a floating-point exception flag triggers a trap. To make the both functions follow the C23, fesetexcept and fesetexceptflag now fail if the argument may trigger a trap. The math tests now check for an value different than 0, instead of bail out as unsupported for EXCEPTION_SET_FORCES_TRAP. Checked on powerpc64le-linux-gnu. --- math/test-fesetexcept-traps.c | 24 ++++++++++++++++-------- math/test-fexcept-traps.c | 16 +++++++++------- sysdeps/powerpc/fpu/fesetexcept.c | 5 +++++ sysdeps/powerpc/fpu/fsetexcptflg.c | 9 ++++++++- 4 files changed, 38 insertions(+), 16 deletions(-) diff --git a/math/test-fesetexcept-traps.c b/math/test-fesetexcept-traps.c index 71b6e45b33..7efcd0343a 100644 --- a/math/test-fesetexcept-traps.c +++ b/math/test-fesetexcept-traps.c @@ -39,16 +39,24 @@ do_test (void) return result; } - if (EXCEPTION_SET_FORCES_TRAP) - { - puts ("setting exceptions traps, cannot test on this architecture"); - return 77; - } - /* Verify fesetexcept does not cause exception traps. */ + /* Verify fesetexcept does not cause exception traps. For architectures + where setting the exception might result in traps the function should + return a nonzero value. */ ret = fesetexcept (FE_ALL_EXCEPT); + + _Static_assert (!(EXCEPTION_SET_FORCES_TRAP && !EXCEPTION_TESTS(float)), + "EXCEPTION_SET_FORCES_TRAP only makes sense if the " + "architecture suports exceptions"); + if (ret == 0) - puts ("fesetexcept (FE_ALL_EXCEPT) succeeded"); - else + { + if (EXCEPTION_SET_FORCES_TRAP) + { + puts ("unexpected fesetexcept success"); + result = 1; + } + } + else if (!EXCEPTION_SET_FORCES_TRAP) { puts ("fesetexcept (FE_ALL_EXCEPT) failed"); if (EXCEPTION_TESTS (float)) diff --git a/math/test-fexcept-traps.c b/math/test-fexcept-traps.c index 9701c3c320..998c241058 100644 --- a/math/test-fexcept-traps.c +++ b/math/test-fexcept-traps.c @@ -63,14 +63,16 @@ do_test (void) result = 1; } - if (EXCEPTION_SET_FORCES_TRAP) - { - puts ("setting exceptions traps, cannot test on this architecture"); - return 77; - } - /* The test is that this does not cause exception traps. */ + /* The test is that this does not cause exception traps. For architectures + where setting the exception might result in traps the function should + return a nonzero value. */ ret = fesetexceptflag (&saved, FE_ALL_EXCEPT); - if (ret != 0) + + _Static_assert (!(EXCEPTION_SET_FORCES_TRAP && !EXCEPTION_TESTS(float)), + "EXCEPTION_SET_FORCES_TRAP only makes sense if the " + "architecture suports exceptions"); + + if (ret != 0 && !EXCEPTION_SET_FORCES_TRAP) { puts ("fesetexceptflag failed"); result = 1; diff --git a/sysdeps/powerpc/fpu/fesetexcept.c b/sysdeps/powerpc/fpu/fesetexcept.c index 609a148a95..2850156d3a 100644 --- a/sysdeps/powerpc/fpu/fesetexcept.c +++ b/sysdeps/powerpc/fpu/fesetexcept.c @@ -31,6 +31,11 @@ fesetexcept (int excepts) & FE_INVALID_SOFTWARE)); if (n.l != u.l) { + if (n.l & fenv_exceptions_to_reg (excepts)) + /* Setting the exception flags may trigger a trap. ISO C 23 § 7.6.4.4 + does not allow it. */ + return -1; + fesetenv_register (n.fenv); /* Deal with FE_INVALID_SOFTWARE not being implemented on some chips. */ diff --git a/sysdeps/powerpc/fpu/fsetexcptflg.c b/sysdeps/powerpc/fpu/fsetexcptflg.c index 2b22f913c0..6517e8ea03 100644 --- a/sysdeps/powerpc/fpu/fsetexcptflg.c +++ b/sysdeps/powerpc/fpu/fsetexcptflg.c @@ -44,7 +44,14 @@ __fesetexceptflag (const fexcept_t *flagp, int excepts) This may cause floating-point exceptions if the restored state requests it. */ if (n.l != u.l) - fesetenv_register (n.fenv); + { + if (n.l & fenv_exceptions_to_reg (excepts)) + /* Setting the exception flags may trigger a trap. ISO C 23 § 7.6.4.4 + does not allow it. */ + return -1; + + fesetenv_register (n.fenv); + } /* Deal with FE_INVALID_SOFTWARE not being implemented on some chips. */ if (flag & FE_INVALID) -- 2.34.1