This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Add FE_NOMASK_ENV return value test
- From: "Joseph S. Myers" <joseph at codesourcery dot com>
- To: Wilco <wdijkstr at arm dot com>
- Cc: <libc-alpha at sourceware dot org>
- Date: Mon, 2 Jun 2014 14:28:56 +0000
- Subject: Re: [PATCH] Add FE_NOMASK_ENV return value test
- Authentication-results: sourceware.org; auth=none
- References: <000901cf7698$bf4a2e10$3dde8a30$ at com> <Pine dot LNX dot 4 dot 64 dot 1405231517080 dot 17788 at digraph dot polyomino dot org dot uk> <002401cf7e6c$aa051020$fe0f3060$ at com>
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.
--
Joseph S. Myers
joseph@codesourcery.com