diff mbox

Add FE_NOMASK_ENV return value test

Message ID 002401cf7e6c$aa051020$fe0f3060$@com
State New
Headers show

Commit Message

Wilco June 2, 2014, 2:12 p.m. UTC 
> > From: Joseph Myers wrote:
> > On Fri, 23 May 2014, Wilco wrote:
> >
> > > This patch rewrites feupdateenv to improve performance by avoiding
> > > unnecessary FPSCR reads/writes and to fix bug 16918
> > > (https://sourceware.org/bugzilla/show_bug.cgi?id=16918).
> >
> > It would be desirable to add an architecture-independent testcase that, if
> > FE_NOMASK_ENV is defined and feupdateenv (FE_NOMASK_ENV) succeeds, the
> > exceptions are then enabled as indicated by fegetexcept (or by getting
> > SIGFPE, as test-fenv.c tests various cases, but a test using fegetexcept
> > is simpler to write).  (It's best for this to be a new test rather than
> > adding to the things in test-fenv.c.)

This patch adds a new test to ensure the return values of fesetenv, feupdateenv 
and feenbleexcept are correct when FE_NOMASK_ENV is not supported. This ensures 
BZ16918/BZ17009 remain fixed.

OK?

Wilco

ChangeLog:
2014-06-02  Wilco  <wdijkstr@arm.com>

	* math/test-fenv-return.c: New file. Test FE_NOMASK_ENV return values.
	* math/Makefile: Add new test test-fenv-return.
---
 math/Makefile           |    2 +-
 math/test-fenv-return.c |  147 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 148 insertions(+), 1 deletion(-)
 create mode 100644 math/test-fenv-return.c

Comments

Joseph Myers June 2, 2014, 2:28 p.m. UTC | #1 
On Mon, 2 Jun 2014, Wilco wrote:

> > > From: Joseph Myers wrote:
> > > On Fri, 23 May 2014, Wilco wrote:
> > >
> > > > This patch rewrites feupdateenv to improve performance by avoiding
> > > > unnecessary FPSCR reads/writes and to fix bug 16918
> > > > (https://sourceware.org/bugzilla/show_bug.cgi?id=16918).
> > >
> > > It would be desirable to add an architecture-independent testcase that, if
> > > FE_NOMASK_ENV is defined and feupdateenv (FE_NOMASK_ENV) succeeds, the
> > > exceptions are then enabled as indicated by fegetexcept (or by getting
> > > SIGFPE, as test-fenv.c tests various cases, but a test using fegetexcept
> > > is simpler to write).  (It's best for this to be a new test rather than
> > > adding to the things in test-fenv.c.)
> 
> This patch adds a new test to ensure the return values of fesetenv, 
> feupdateenv and feenbleexcept are correct when FE_NOMASK_ENV is not 
> supported. This ensures BZ16918/BZ17009 remain fixed.
> 
> OK?

This test doesn't look correct.  You have

> +  if (!EXCEPTION_ENABLE_SUPPORTED (FE_ALL_EXCEPT))
> +    {
> +      if (res == -1)
> +	{
> +	  puts ("feenableexcept (FE_ALL_EXCEPT) not supported, cannot test.");
> +	  return;
> +	}
> +      else
> +	{
> +	  printf ("feenableexcept incorrect return value %d\n", res);
> +	  count_errors++;
> +	}

but !EXCEPTION_ENABLE_SUPPORTED simply means enabling traps *might or 
might not* be supported at runtime.  Thus, if feenableexcept returns 
success (some value other than -1), this should not make the test fail; it 
should simply continue on to the test with fegetexcept of whether the 
exceptions did in fact get enabled.  The same applies to the tests of the 
other two functions.  With the existing patch, I'd expect the test to fail 
when run on ARM implementations that support exception traps.
diff mbox

Patch

diff --git a/math/Makefile b/math/Makefile
index f337a0e..cd59ce5 100644
--- a/math/Makefile
+++ b/math/Makefile
@@ -90,7 +90,7 @@  tests = test-matherr test-fenv atest-exp atest-sincos atest-exp2 basic-test \
 	test-misc test-fpucw test-fpucw-ieee tst-definitions test-tgmath \
 	test-tgmath-ret bug-nextafter bug-nexttoward bug-tgmath1 \
 	test-tgmath-int test-tgmath2 test-powl tst-CMPLX tst-CMPLX2 test-snan \
-	test-fenv-tls test-fenv-preserve $(tests-static)
+	test-fenv-tls test-fenv-preserve test-fenv-return $(tests-static)
 tests-static = test-fpucw-static test-fpucw-ieee-static
 # We do the `long double' tests only if this data type is available and
 # distinct from `double'.
diff --git a/math/test-fenv-return.c b/math/test-fenv-return.c
new file mode 100644
index 0000000..5c727c6
--- /dev/null
+++ b/math/test-fenv-return.c
@@ -0,0 +1,147 @@ 
+/* Test fegetenv preserves exception mask (bug 16198).
+   Copyright (C) 2014 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <fenv.h>
+#include <stdio.h>
+#include <math-tests.h>
+
+static int count_errors;
+
+static void
+test_feenableexcept (void)
+{
+#if defined FE_ALL_EXCEPT
+  int res;
+
+  fedisableexcept (FE_ALL_EXCEPT);
+
+  res = feenableexcept (FE_ALL_EXCEPT);
+
+  if (!EXCEPTION_ENABLE_SUPPORTED (FE_ALL_EXCEPT))
+    {
+      if (res == -1)
+	{
+	  puts ("feenableexcept (FE_ALL_EXCEPT) not supported, cannot test.");
+	  return;
+	}
+      else
+	{
+	  printf ("feenableexcept incorrect return value %d\n", res);
+	  count_errors++;
+	}
+    }
+  else if (res != 0)
+    {
+      puts ("feenableexcept (FE_ALL_EXCEPT) failed");
+      count_errors++;
+    }
+
+  if (fegetexcept () != FE_ALL_EXCEPT)
+    {
+      puts ("feenableexcept did not set all exceptions");
+      count_errors++;
+    }
+#endif
+}
+
+static void
+test_fesetenv (void)
+{
+#if defined FE_NOMASK_ENV && defined FE_ALL_EXCEPT
+  int res;
+
+  fedisableexcept (FE_ALL_EXCEPT);
+
+  res = fesetenv (FE_NOMASK_ENV);
+
+  if (!EXCEPTION_ENABLE_SUPPORTED (FE_ALL_EXCEPT))
+    {
+      if (res != 0)
+	{
+	  puts ("fesetenv (FE_NOMASK_ENV) not supported, cannot test.");
+	  return;
+	}
+      else
+	{
+	  printf ("fesetenv incorrect return value %d\n", res);
+	  count_errors++;
+	}
+    }
+  else if (res != 0)
+    {
+      puts ("fesetenv (FE_NOMASK_ENV) failed");
+      count_errors++;
+    }
+
+  if (fegetexcept () != FE_ALL_EXCEPT)
+    {
+      puts ("fesetenv did not set all exceptions");
+      count_errors++;
+    }
+#endif
+}
+
+static void
+test_feupdateenv (void)
+{
+#if defined FE_NOMASK_ENV && defined FE_ALL_EXCEPT
+  int res;
+
+  fedisableexcept (FE_ALL_EXCEPT);
+
+  res = feupdateenv (FE_NOMASK_ENV);
+
+  if (!EXCEPTION_ENABLE_SUPPORTED (FE_ALL_EXCEPT))
+    {
+      if (res != 0)
+	{
+	  puts ("feupdateenv (FE_NOMASK_ENV)) not supported, cannot test.");
+	  return;
+	}
+      else
+	{
+	  printf ("feupdateenv incorrect return value %d\n", res);
+	  count_errors++;
+	}
+    }
+  else if (res != 0)
+    {
+      puts ("feupdateenv (FE_NOMASK_ENV) failed");
+      count_errors++;
+    }
+
+  if (fegetexcept () != FE_ALL_EXCEPT)
+    {
+      puts ("feupdateenv did not set all exceptions");
+      count_errors++;
+    }
+#endif
+}
+
+static int
+do_test (void)
+{
+  test_feenableexcept ();
+  test_fesetenv ();
+  test_feupdateenv ();
+
+  return count_errors != 0 ? 1 : 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"