cgcc and -Dx86_64

cgcc and -Dx86_64

[Joe Lawrence]

Hello linux-sparse,

I was trying to run sparse against the upstream kpatch project and ran
into problems with an include file that defined an "x86_64" variable:

% git clone https://github.com/dynup/kpatch
% cd kpatch/kpatch-build

% ~/sparse/cgcc -v -MMD -MP -I../kmod/patch -Iinsn -Wall -g -Werror -c
-o create-diff-object.o create-diff-object.c

sparse -v --debug -MMD -MP -I../kmod/patch -Iinsn -Wall -g -Werror -c -o
create-diff-object.o create-diff-object.c -Dx86_64=1 -D__x86_64=1
-D__x86_64__=1 -D__LP64__=1 -D__CHAR_BIT__=8 -D__SCHAR_MAX__=127
-D__SHRT_MAX__=32767 -D__INT_MAX__=2147483647
-D__LONG_MAX__=9223372036854775807L
-D__LONG_LONG_MAX__=9223372036854775807LL
-D__LONG_LONG_LONG_MAX__=170141183460469231731687303715884105727LLL
-D__FLT_RADIX__=2 -D__FINITE_MATH_ONLY__=0 -D__DECIMAL_DIG__=33
-D__FLT_MANT_DIG__=24 -D__FLT_DIG__=6 -D__FLT_MIN_EXP__='(-125)'
-D__FLT_MAX_EXP__=128 -D__FLT_MIN_10_EXP__='(-37)'
-D__FLT_MAX_10_EXP__=38 -D__FLT_HAS_INFINITY__=1
-D__FLT_HAS_QUIET_NAN__=1 -D__FLT_DENORM_MIN__=1.40129846e-45F
-D__FLT_EPSILON__=1.19209290e-7F -D__FLT_MAX__=3.40282347e+38F
-D__FLT_MIN__=1.17549435e-38F -D__DBL_MANT_DIG__=53 -D__DBL_DIG__=15
-D__DBL_MIN_EXP__='(-1021)' -D__DBL_MAX_EXP__=1024
-D__DBL_MIN_10_EXP__='(-307)' -D__DBL_MAX_10_EXP__=308
-D__DBL_HAS_INFINITY__=1 -D__DBL_HAS_QUIET_NAN__=1
-D__DBL_DENORM_MIN__=4.9406564584124654e-324
-D__DBL_EPSILON__=2.2204460492503131e-16
-D__DBL_MAX__=1.7976931348623157e+308
-D__DBL_MIN__=2.2250738585072014e-308 -D__LDBL_MANT_DIG__=113
-D__LDBL_DIG__=33 -D__LDBL_MIN_EXP__='(-16381)' -D__LDBL_MAX_EXP__=16384
-D__LDBL_MIN_10_EXP__='(-4931)' -D__LDBL_MAX_10_EXP__=4932
-D__LDBL_HAS_INFINITY__=1 -D__LDBL_HAS_QUIET_NAN__=1
-D__LDBL_DENORM_MIN__=6.47517511943802511092443895822764655e-4966L
-D__LDBL_EPSILON__=1.92592994438723585305597794258492732e-34L
-D__LDBL_MAX__=1.18973149535723176508575932662800702e+4932L
-D__LDBL_MIN__=3.36210314311209350626267781732175260e-4932L
-U__SIZE_TYPE__ -D__SIZE_TYPE__=long\ unsigned\ int
-D__SIZEOF_POINTER__=8 -Dunix=1 -D__unix=1 -D__unix__=1 -D__linux__=1
-D__linux=1 -Dlinux=linux -gcc-base-dir /usr/lib/gcc/x86_64-linux-gnu/5/
-multiarch-dir x86_64-linux-gnu

insn/asm/insn.h:65:23: error: expected ; at end of declaration
insn/asm/insn.h:65:23: error: Expected } at end of
struct-union-enum-specifier
insn/asm/insn.h:65:23: error: got 1
insn/asm/insn.h:69:1: error: Expected ; at the end of type declaration
insn/asm/insn.h:69:1: error: got }
insn/asm/insn.h:99:65: error: Expected ) in function declarator
insn/asm/insn.h:99:65: error: got 1
create-diff-object.c:914:53: error: no member 'next_byte' in struct insn


I can avoid this by renaming the structure member to something like
"foo_x86_64".  I believe the problem stems from cgcc passing "-Dx86_64"
to gcc... sparse later gets confused as there is now a preprocessor
variable defined with the same name.

We could s/x86_64/something_else/g across the whole project to avoid
this glitch, but was wondering if there was a better way.  IMHO,
"x86_64" is a reasonable name that a developer might want to use... I
can't find any documentation that this is a (gcc) reserved keyword.

Advice appreciated... thanks!

-- Joe

Re: cgcc and -Dx86_64

[Linus Torvalds]

[-- Attachment #1: Type: text/plain, Size: 1475 bytes --]

On Wed, Dec 21, 2016 at 8:07 AM, Joe Lawrence <joe.lawrence@redhat.com> wrote:
>
> I was trying to run sparse against the upstream kpatch project and ran
> into problems with an include file that defined an "x86_64" variable:

Yeah,. that's bogus. It should be removed. It goes back to the
original x86-64 specific cgcc patch from 2007 (commit 0fcbcbf:
"Implement x86-64 support in cgcc").

It may be that old versions of gcc did the same, who knows. They
definitely don't any more, I checked:

  $ gcc -dM -E - < /dev/null | grep -v 'define __'

  #define _STDC_PREDEF_H 1
  #define unix 1
  #define linux 1
  #define _LP64 1

and so gcc itself definitely doesn't do the x86_64 thing (it does
pre-define versions with double underscores before and after):

  $ gcc -dM -E - < /dev/null | grep x86
  #define __x86_64 1
  #define __x86_64__ 1

> I can avoid this by renaming the structure member to something like
> "foo_x86_64".  I believe the problem stems from cgcc passing "-Dx86_64"
> to gcc... sparse later gets confused as there is now a preprocessor
> variable defined with the same name.

You shouldn't need that.

> We could s/x86_64/something_else/g across the whole project to avoid
> this glitch, but was wondering if there was a better way.

The fix it so just remove x86_64 from cgcc. It already does define the
underscored versions.

Same for the other architectures, for that matter. Obvios trivial (and
totally untested) patch attached.

              Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/plain, Size: 1778 bytes --]

 cgcc | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/cgcc b/cgcc
index d7b1c99..c29fa58 100755
--- a/cgcc
+++ b/cgcc
@@ -250,25 +250,25 @@ sub add_specs {
 	    " -D'__fastcall=__attribute__((__fastcall__))'" .
 	    " -D'__declspec(x)=__attribute__((x))'";
     } elsif ($spec eq 'i86') {
-	return (' -Di386=1 -D__i386=1 -D__i386__=1' .
+	return (' -D__i386=1 -D__i386__=1' .
 		&integer_types (8, 16, 32, $m64 ? 64 : 32, 64) .
 		&float_types (1, 1, 21, [24,8], [53,11], [64,15]) .
 		&define_size_t ($m64 ? "long unsigned int" : "unsigned int") .
 		' -D__SIZEOF_POINTER__=' . ($m64 ? '8' : '4'));
     } elsif ($spec eq 'sparc') {
-	return (' -Dsparc=1 -D__sparc=1 -D__sparc__=1' .
+	return (' -D__sparc=1 -D__sparc__=1' .
 		&integer_types (8, 16, 32, $m64 ? 64 : 32, 64) .
 		&float_types (1, 1, 33, [24,8], [53,11], [113,15]) .
 		&define_size_t ($m64 ? "long unsigned int" : "unsigned int") .
 		' -D__SIZEOF_POINTER__=' . ($m64 ? '8' : '4'));
     } elsif ($spec eq 'sparc64') {
-	return (' -Dsparc=1 -D__sparc=1 -D__sparc__=1 -D__sparcv9__=1 -D__sparc64__=1 -D__arch64__=1 -D__LP64__=1' .
+	return (' -D__sparc=1 -D__sparc__=1 -D__sparcv9__=1 -D__sparc64__=1 -D__arch64__=1 -D__LP64__=1' .
 		&integer_types (8, 16, 32, 64, 64, 128) .
 		&float_types (1, 1, 33, [24,8], [53,11], [113,15]) .
 		&define_size_t ("long unsigned int") .
 		' -D__SIZEOF_POINTER__=8');
     } elsif ($spec eq 'x86_64') {
-	return (' -Dx86_64=1 -D__x86_64=1 -D__x86_64__=1' . ($m32 ? '' : ' -D__LP64__=1') .
+	return (' -D__x86_64=1 -D__x86_64__=1' . ($m32 ? '' : ' -D__LP64__=1') .
 		&integer_types (8, 16, 32, $m32 ? 32 : 64, 64, 128) .
 		&float_types (1, 1, 33, [24,8], [53,11], [113,15]) .
 		&define_size_t ($m32 ? "unsigned int" : "long unsigned int") .

Re: cgcc and -Dx86_64

[Joe Lawrence]

On 12/21/2016 12:13 PM, Linus Torvalds wrote:
> On Wed, Dec 21, 2016 at 8:07 AM, Joe Lawrence <joe.lawrence@redhat.com> wrote:
>>
>> I was trying to run sparse against the upstream kpatch project and ran
>> into problems with an include file that defined an "x86_64" variable:
> 
> Yeah,. that's bogus. It should be removed. It goes back to the
> original x86-64 specific cgcc patch from 2007 (commit 0fcbcbf:
> "Implement x86-64 support in cgcc").
> 
> It may be that old versions of gcc did the same, who knows. They
> definitely don't any more, I checked:
> 
>   $ gcc -dM -E - < /dev/null | grep -v 'define __'
> 
>   #define _STDC_PREDEF_H 1
>   #define unix 1
>   #define linux 1
>   #define _LP64 1
> 
> and so gcc itself definitely doesn't do the x86_64 thing (it does
> pre-define versions with double underscores before and after):
> 
>   $ gcc -dM -E - < /dev/null | grep x86
>   #define __x86_64 1
>   #define __x86_64__ 1
> 
>> I can avoid this by renaming the structure member to something like
>> "foo_x86_64".  I believe the problem stems from cgcc passing "-Dx86_64"
>> to gcc... sparse later gets confused as there is now a preprocessor
>> variable defined with the same name.
> 
> You shouldn't need that.
> 
>> We could s/x86_64/something_else/g across the whole project to avoid
>> this glitch, but was wondering if there was a better way.
> 
> The fix it so just remove x86_64 from cgcc. It already does define the
> underscored versions.
> 
> Same for the other architectures, for that matter. Obvios trivial (and
> totally untested) patch attached.
> 

Thanks, Linus!

The patch worked well for me, so consider this tested (on x86_64 at least).

-- Joe

[PATCH] cgcc should not define non-reserved identifiers

[Luc Van Oostenryck]

Since the original x86-64 support in cgcc
(commit 0fcbcbf4: "Implement x86-64 support in cgcc")
cgcc define some non-reserved identifiers like i386, x86_64, ...
It should not since these are valid identifiers which could be used
for some variable names.

Current versions of gcc only define those prefixed with a double
underscore and cgcc also define those. So ...

Remove the defines of the non-prefixed ones.

Reported-by: Joe Lawrence <joe.lawrence@redhat.com>
Investigated-by: Joe Lawrence <joe.lawrence@redhat.com>
Originally-From: Linus Torvalds <torvalds@linux-foundation.org>
Tested-by: Joe Lawrence <joe.lawrence@redhat.com>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 cgcc | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/cgcc b/cgcc
index d7b1c99c..c29fa587 100755
--- a/cgcc
+++ b/cgcc
@@ -250,25 +250,25 @@ sub add_specs {
 	    " -D'__fastcall=__attribute__((__fastcall__))'" .
 	    " -D'__declspec(x)=__attribute__((x))'";
     } elsif ($spec eq 'i86') {
-	return (' -Di386=1 -D__i386=1 -D__i386__=1' .
+	return (' -D__i386=1 -D__i386__=1' .
 		&integer_types (8, 16, 32, $m64 ? 64 : 32, 64) .
 		&float_types (1, 1, 21, [24,8], [53,11], [64,15]) .
 		&define_size_t ($m64 ? "long unsigned int" : "unsigned int") .
 		' -D__SIZEOF_POINTER__=' . ($m64 ? '8' : '4'));
     } elsif ($spec eq 'sparc') {
-	return (' -Dsparc=1 -D__sparc=1 -D__sparc__=1' .
+	return (' -D__sparc=1 -D__sparc__=1' .
 		&integer_types (8, 16, 32, $m64 ? 64 : 32, 64) .
 		&float_types (1, 1, 33, [24,8], [53,11], [113,15]) .
 		&define_size_t ($m64 ? "long unsigned int" : "unsigned int") .
 		' -D__SIZEOF_POINTER__=' . ($m64 ? '8' : '4'));
     } elsif ($spec eq 'sparc64') {
-	return (' -Dsparc=1 -D__sparc=1 -D__sparc__=1 -D__sparcv9__=1 -D__sparc64__=1 -D__arch64__=1 -D__LP64__=1' .
+	return (' -D__sparc=1 -D__sparc__=1 -D__sparcv9__=1 -D__sparc64__=1 -D__arch64__=1 -D__LP64__=1' .
 		&integer_types (8, 16, 32, 64, 64, 128) .
 		&float_types (1, 1, 33, [24,8], [53,11], [113,15]) .
 		&define_size_t ("long unsigned int") .
 		' -D__SIZEOF_POINTER__=8');
     } elsif ($spec eq 'x86_64') {
-	return (' -Dx86_64=1 -D__x86_64=1 -D__x86_64__=1' . ($m32 ? '' : ' -D__LP64__=1') .
+	return (' -D__x86_64=1 -D__x86_64__=1' . ($m32 ? '' : ' -D__LP64__=1') .
 		&integer_types (8, 16, 32, $m32 ? 32 : 64, 64, 128) .
 		&float_types (1, 1, 33, [24,8], [53,11], [113,15]) .
 		&define_size_t ($m32 ? "unsigned int" : "long unsigned int") .
-- 
2.9.3