Bug 51009 - xproto restrict problem
Summary: xproto restrict problem
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Protocol/Core (show other bugs)
Version: 7.7 (2012.06)
Hardware: x86 (IA32) Solaris
: medium major
Assignee: Alan Coopersmith
QA Contact: Xorg Project Team
URL:
Whiteboard: 2012BRB_Reviewed
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-12 06:16 UTC by Thomas Klausner
Modified: 2012-09-07 06:16 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Patch to Xfuncproto.h (1.57 KB, patch)
2012-08-27 16:01 UTC, Alan Coopersmith
no flags Details | Splinter Review

Description Thomas Klausner 2012-06-12 06:16:31 UTC
Joern Clausen reported in
http://gnats.netbsd.org/46586
that libXt-1.1.3 doesn't build on
"Environment is Solaris 10/i86 with stand-alone GCC 4.7.0."

The error is:
libtool: compile:  g++ -DHAVE_CONFIG_H -I. -I.. -I./../include -DLIBPLOT -DLIBPLOTTER -I/pkgsrc/source/pkgsrc/graphics/plotutils/work.pkgsrc-i86/.buildlink/include -O -MT apioldcc.lo -MD -MP -MF .deps/apioldcc.Tpo -c apioldcc.cc  -fPIC -DPIC -o .libs/apioldcc.o
In file included from ./../include/plotter.h:68:0,
                 from extern.h:63,
                 from apioldcc.cc:37:
/pkgsrc/source/pkgsrc/graphics/plotutils/work.pkgsrc-i86/.buildlink/include/X11/Intrinsic.h:1863:37: error: expected ',' or '...' before 'format'
/pkgsrc/source/pkgsrc/graphics/plotutils/work.pkgsrc-i86/.buildlink/include/X11/Intrinsic.h:1865:3: error: args to be formatted is not '...'
*** Error code 1

The problem comes from:
     #ifndef _X_RESTRICT_KYWD
     # define _X_RESTRICT_KYWD
     #endif
     extern Cardinal XtAsprintf(
         String *new_string,
 1863	_Xconst char * _X_RESTRICT_KYWD format,
  	...
     ) _X_ATTRIBUTE_PRINTF(2,3);

 
     #ifdef XTTRACEMEMORY


It seems that _X_RESTRICT_KWYD in xproto-7.0.23 is expanded to "restrict" which doesn't work for some reason, while __restrict__ or the empty string do work.
The corresponding code in xproto is:
 > /* C99 keyword "restrict" or equivalent extensions in pre-C99 compilers */
 > /* requires xproto >= 7.0.21 */
 > #ifndef _X_RESTRICT_KYWD
 > # if defined(restrict) /* assume autoconf set it correctly */ || \
 >    (defined(__STDC_VERSION__) && (__STDC_VERSION__ - 0 >= 199901L)) /* C99 */
 > #  define _X_RESTRICT_KYWD  restrict
 > # elif defined(__GNUC__) && !defined(__STRICT_ANSI__) /* gcc w/C89+extensions */
 > #  define _X_RESTRICT_KYWD __restrict__
 > # else
 > #  define _X_RESTRICT_KYWD
 > # endif
 > #endif

We're not sure how to define this properly so this will work, so please suggest a solution. Thanks.
Comment 1 Alan Coopersmith 2012-06-12 08:15:02 UTC
Hmm, does the newer g++ (i.e. C++ mode) claim C99 mode but not support the 
C99 restrict keyword?

Seems to work fine here (Solaris 11/x86 with bundled gcc 4.5.2), but it's
expanding to the gnu __restrict__ version instead, since it's not claiming C99:

alanc@also:/tmp [8:12am - 5] gcc --version
gcc (GCC) 4.5.2
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

alanc@also:/tmp [8:12am - 6] cat test.cc
#include <X11/Intrinsic.h>
alanc@also:/tmp [8:12am - 7] g++ -c test.cc
alanc@also:/tmp [8:13am - 8] g++ -E test.cc | grep restrict
    const char * __restrict__ format,
Comment 2 Jeremy Huddleston Sequoia 2012-06-14 01:06:18 UTC
Seems fine here (OS X) with various g++ versions.
Comment 3 Jörn Clausen 2012-06-28 00:27:18 UTC
With GCC 3.4.6, I get

    const char * __restrict__ format,

But with GCC 4.7.0 (which is the compiler I want to use), I get

    const char * restrict format,
Comment 4 Alan Coopersmith 2012-06-28 06:41:24 UTC
(In reply to comment #3)
> With GCC 3.4.6, I get
> 
>     const char * __restrict__ format,
> 
> But with GCC 4.7.0 (which is the compiler I want to use), I get
> 
>     const char * restrict format,

Which is the correct and expected behavior for a C99-capable compiler.
Comment 5 Jörn Clausen 2012-06-28 23:28:01 UTC
So it's a bug in GCC 4.7?

BTW: We are talking about g++. Does C99 apply here, anyway?
Comment 6 Alan Coopersmith 2012-06-28 23:31:30 UTC
(In reply to comment #5)
> So it's a bug in GCC 4.7?

It's seeming likely.

> BTW: We are talking about g++. Does C99 apply here, anyway?

That path is being hit because it claims to implement C99 without doing 
so - which is why it seems to be a bug in g++ - it shouldn't set the
__STDC_VERSION__ to C99 and then not recognize C99 keywords.
Comment 7 Jörn Clausen 2012-07-25 12:12:14 UTC
While tracking another problem I found this:

$ diff -u /usr/include/sys/feature_tests.h .../localgcc470/lib/gcc/i386-pc-solaris2.10/4.7.0/include-fixed/sys/feature_tests.h
@@ -345,7 +354,11 @@
  * declarations regardless of compiler version.
  */
 #if (defined(__STDC__) && defined(_STDC_C99))
+#ifdef __cplusplus
+#define        _RESTRICT_KYWD  __restrict
+#else
 #define        _RESTRICT_KYWD  restrict
+#endif
 #else
 #define        _RESTRICT_KYWD
 #endif

I can't comment on the usefulness of overriding the native header files, but this seems to indicate that g++ would like to see "__restrict".

And I wasn't aware that the OS and/or the compiler define _RESTRICT_KYWD. Shouldn't Xfuncproto.h use that definition and revert to it's own logic only in the absence of a predefined value?
Comment 8 Alan Coopersmith 2012-07-25 15:14:24 UTC
(In reply to comment #7)
> And I wasn't aware that the OS and/or the compiler define _RESTRICT_KYWD.
> Shouldn't Xfuncproto.h use that definition and revert to it's own logic only in
> the absence of a predefined value?

No.  Xfuncproto.h is cross platform and used on many OS'es, it cannot rely
on a Solaris-only, undocumented, private definition in the Solaris system
header files.

I have talked about this a bit with Rainer Orth, who maintains the Solaris port
for gcc upstream, and he explained why g++ does this on Solaris, even though
it's not the most kosher.   Since g++ has to do this for ABI compatibility with
libm on Solaris, we'll probably have to workaround it in the Xfuncproto.h header.
Comment 9 Alan Coopersmith 2012-08-27 16:01:45 UTC
Created attachment 66181 [details] [review]
Patch to Xfuncproto.h

This is the patch I sent to xorg-devel for review - it works for me on
Solaris 11 with Studio & gcc 4.5 - but I don't have gcc 4.6 or later, which
is where this problem occurs, so can't verify it solves the problem for you.
Comment 10 Jonathan Perkin 2012-09-05 16:51:11 UTC
Yep, this works, thanks.  I've applied the patch to pkgsrc.
Comment 11 Alan Coopersmith 2012-09-07 06:16:58 UTC
Thanks for confirming, pushed to git master.

To ssh://git.freedesktop.org/git/xorg/proto/x11proto
   f1b8b4d..c76d514  master -> master


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.