Bug 222247

Fails on i386 Stable 11. rev 323489:

In file included from /usr/src/lib/msun/tests/exponential_test.c:41:
In file included from /usr/obj/usr/src/tmp/usr/include/ieeefp.h:13:
/usr/obj/usr/src/tmp/usr/include/machine/ieeefp.h:111:9: error: '__fldcw' macro redefined
      [-Werror,-Wmacro-redefined]
#define __fldcw(addr)   __asm __volatile("fldcw %0" : : "m" (*(addr)))
        ^
/usr/obj/usr/src/tmp/usr/include/fenv.h:98:9: note: previous definition is here
#define __fldcw(__cw)           __asm __volatile("fldcw %0" : : "m" (__cw))
        ^
In file included from /usr/src/lib/msun/tests/exponential_test.c:41:
In file included from /usr/obj/usr/src/tmp/usr/include/ieeefp.h:13:
/usr/obj/usr/src/tmp/usr/include/machine/ieeefp.h:112:9: error: '__fldenv' macro redefined
      [-Werror,-Wmacro-redefined]
#define __fldenv(addr)  __asm __volatile("fldenv %0" : : "m" (*(addr)))
        ^
/usr/obj/usr/src/tmp/usr/include/fenv.h:99:9: note: previous definition is here
#define __fldenv(__env)         __asm __volatile("fldenv %0" : : "m" (__env))
        ^
In file included from /usr/src/lib/msun/tests/exponential_test.c:41:
In file included from /usr/obj/usr/src/tmp/usr/include/ieeefp.h:13:
/usr/obj/usr/src/tmp/usr/include/machine/ieeefp.h:114:9: error: '__fnstcw' macro redefined
      [-Werror,-Wmacro-redefined]
#define __fnstcw(addr)  __asm __volatile("fnstcw %0" : "=m" (*(addr)))
        ^
/usr/obj/usr/src/tmp/usr/include/fenv.h:105:9: note: previous definition is here
#define __fnstcw(__cw)          __asm __volatile("fnstcw %0" : "=m" (*(__cw)))
        ^
In file included from /usr/src/lib/msun/tests/exponential_test.c:41:
In file included from /usr/obj/usr/src/tmp/usr/include/ieeefp.h:13:
/usr/obj/usr/src/tmp/usr/include/machine/ieeefp.h:115:9: error: '__fnstenv' macro redefined
      [-Werror,-Wmacro-redefined]
#define __fnstenv(addr) __asm __volatile("fnstenv %0" : "=m" (*(addr)))
        ^
/usr/obj/usr/src/tmp/usr/include/fenv.h:104:9: note: previous definition is here
#define __fnstenv(__env)        __asm __volatile("fnstenv %0" : "=m" (*(__env)))
        ^
In file included from /usr/src/lib/msun/tests/exponential_test.c:41:
In file included from /usr/obj/usr/src/tmp/usr/include/ieeefp.h:13:
/usr/obj/usr/src/tmp/usr/include/machine/ieeefp.h:116:9: error: '__fnstsw' macro redefined
      [-Werror,-Wmacro-redefined]
#define __fnstsw(addr)  __asm __volatile("fnstsw %0" : "=m" (*(addr)))
        ^
/usr/obj/usr/src/tmp/usr/include/fenv.h:106:9: note: previous definition is here
#define __fnstsw(__sw)          __asm __volatile("fnstsw %0" : "=am" (*(__sw)))
        ^
5 errors generated.
*** Error code 1

Created attachment 186295 [details]
Simple proposed patch

Simple proposed patch that removes the offending macros and use those provided by fenv.h instead.

The patch results in the following:

In file included from /usr/src/include/ieeefp.h:13:
/usr/obj/usr/src/tmp/usr/include/machine/ieeefp.h:160:11: error: use of undeclared identifier '_env'
        __fldenv(_env);
                 ^
/usr/obj/usr/src/tmp/usr/include/machine/ieeefp.h:247:11: error: invalid lvalue in asm input for constraint 'm'
        __fldenv(&_env);
                 ^~~~~
/usr/src/lib/msun/x86/fenv.h:99:64: note: expanded from macro '__fldenv'
#define __fldenv(__env)         __asm __volatile("fldenv %0" : : "m" (__env))
                                                                      ^~~~~
2 errors generated.
*** Error code 1

Comment 4 Ryan Libby 2017-09-13 04:14:09 UTC

Thanks you both for the detailed reports.  Can you confirm this to be
fixed after updating sources?

It seems that this was introduced to stable/11 in r323299 [1] and I
think should be fixed in r323515 [2].  There is a Jenkins job that
indicates a successful build of stable/11 i386 after r323515 [3] but
reports from your build environments would close the loop for closing
this bug.

[1] https://svnweb.freebsd.org/base?view=revision&revision=323299
[2] https://svnweb.freebsd.org/base?view=revision&revision=323515
[3] https://ci.freebsd.org/job/FreeBSD-stable-11-i386-build/1558/

Created attachment 186320 [details]
Self-contained macros redefinitions fix

(In reply to Ryan Libby from comment #4)
I will do a test build of r323519 and keep you informed. In the meantime I propose a different patch that obsoletes the comment of r323515 ("This is a bandaid"). Of course r323515 would need to be reverted.

Comment 6 Ryan Libby 2017-09-13 06:53:46 UTC

(In reply to Bertrand Petit from comment #5)

For the underlying bug, yes I think deconflicting the namespace is the
right approach, rather than having libmsun try to use the kernel macros.
I think there are still two different problems:

 - Use of reserved identifiers.  Neither libmsum nor the kernel headers
   should be defining identifiers that start with double underscores.
 - Kernel header namespace pollution.  The kernel macros, after being
   renamed to something not reserved, should be #undef'd at the end of
   the headers.

Attachment 186320 [details] moves the i386 kernel header names out of the way, but
leaves the amd64 ones, and still uses inappropriate reserved names.

Thanks for your interest.  I'm also trying to remove places where we
silence warnings.  I suggest we get the bug at hand resolved and then
clone a new bug targeting head to do this cleanup, since we aren't going
to do it by direct commit to stable/11 anyway.  If you are familiar with
Phabricator (https://wiki.freebsd.org/Phabricator), feel free to submit
a patch there.

Comment 7 Konstantin Belousov 2017-09-13 09:47:38 UTC

(In reply to Ryan Libby from comment #6)
First, we are the implementation.  So at least in the hosted case, we are allowed to introduce the __* and _[A_Z] symbols.

Second, defining something in userspace and then undefining it at the end of the file still breaks user namespaces.  Consider what would happen if user defines an allowed symbol and then includes such file.

Also, note that the restrictions on the user namespace pollution are only applicable to the standard-specified files.

(In reply to Ryan Libby from comment #4)
I did as requested a build using rr323519 without any local modification and confirm that it finished without error.

(In reply to Konstantin Belousov from comment #7)
That is correct.