diff mbox

[v1.1,20/20] Include config.h in MIN-CPPFLAGS

Message ID 20140822053122.GF16835@spoyarek.pnq.redhat.com
State Superseded
Headers show

Commit Message

Siddhesh Poyarekar Aug. 22, 2014, 5:31 a.m. UTC
On Thu, Aug 21, 2014 at 11:29:54PM +0530, Siddhesh Poyarekar wrote:
> This is needed when processing the Versions files since they could
> refer to macros defined in config.h.  config.h was earlier included
> through libc-symbols.h but since MIN-CPPFLAGS does not include the
> latter anymore, it needs to at least include config.h.
> 
> This was causing a difference in generated code on s390x.  With this
> change, s390x code is also unchanged with this and other 19 patches
> (barring the IN_LIB patch of course).
> 
> 	* Makeconfig (MIN-CPPFLAGS): Include config.h.

config.h checks a few macros, most of which are set at command line,
with the exception of _LIBC, which is set in libc-symbols.h.  Set it
on the commandline to be consistent with the behaviour we ought to get
with libc-symbols.h included.

    
	* Makeconfig (MIN-CPPFLAGS): Include config.h.

Comments

Stefan Liebler Aug. 22, 2014, 1:31 p.m. UTC | #1
Hi Siddhesh,

I´ve tested your is_in_module branch on s390x.

Now there are only Wundef warnings with _POSIX*/_XBS*, which are 
mentioned in release wiki.
Thanks.

But building the testsuite fails while building nptl_db/db-symbols.v.iT 
because NOT_IN is not defined in structs.def:
structs.def:81:12: error: missing binary operator before token "("
  #if NOT_IN (libpthread) || TLS_TCB_AT_TP
             ^

The defining NOT_IN to zero of patch 19 is not done, because of patch 
v1.1 20.

patch 19:
+#ifndef _LIBC
+# define NOT_IN(lib) (0)
+#endif

patch v1.1 20:
+	   -D_LIBC -include $(common-objpfx)config.h \


After changing patch 19 to
#ifndef NOT_IN
# define NOT_IN(lib) (0)
#endif
the following tests are failing:
FAIL: conform/ISO/stdlib.h/conform
FAIL: conform/ISO11/stdlib.h/conform
FAIL: conform/ISO99/stdlib.h/conform
FAIL: conform/POSIX/mqueue.h/conform
FAIL: conform/POSIX/stdlib.h/conform
FAIL: conform/POSIX2008/mqueue.h/conform
FAIL: conform/POSIX2008/stdlib.h/conform
FAIL: conform/UNIX98/mqueue.h/conform
FAIL: conform/XOPEN2K/stdlib.h/conform
FAIL: conform/XOPEN2K8/mqueue.h/conform
FAIL: conform/XOPEN2K8/stdlib.h/conform
with either:
 >Namespace violation: "IS_IN"
or:
 >../include/bits/stdlib-float.h:2:12: error: missing binary operator 
before token "("
      #if NOT_IN (rtld)
                 ^

FAIL: stdlib/isomac:
 >...
stdlib.h
system() returned nonzero
...


I´ve also compared the obj-files and got the mentioned diffs of patch 5.

Bye

On 08/22/2014 07:31 AM, Siddhesh Poyarekar wrote:
> On Thu, Aug 21, 2014 at 11:29:54PM +0530, Siddhesh Poyarekar wrote:
>> This is needed when processing the Versions files since they could
>> refer to macros defined in config.h.  config.h was earlier included
>> through libc-symbols.h but since MIN-CPPFLAGS does not include the
>> latter anymore, it needs to at least include config.h.
>>
>> This was causing a difference in generated code on s390x.  With this
>> change, s390x code is also unchanged with this and other 19 patches
>> (barring the IN_LIB patch of course).
>>
>> 	* Makeconfig (MIN-CPPFLAGS): Include config.h.
>
> config.h checks a few macros, most of which are set at command line,
> with the exception of _LIBC, which is set in libc-symbols.h.  Set it
> on the commandline to be consistent with the behaviour we ought to get
> with libc-symbols.h included.
>
>
> 	* Makeconfig (MIN-CPPFLAGS): Include config.h.
>
> diff --git a/Makeconfig b/Makeconfig
> index df26cd0..c6eae06 100644
> --- a/Makeconfig
> +++ b/Makeconfig
> @@ -845,6 +845,7 @@ override CXXFLAGS = $(c++-sysincludes) \
>   MIN-CPPFLAGS = $(config-extra-cppflags) $(CPPUNDEFS) $(CPPFLAGS-config) \
>   	   $($(subdir)-CPPFLAGS) \
>   	   $(+includes) $(defines) $(sysdep-CPPFLAGS) \
> +	   -D_LIBC -include $(common-objpfx)config.h \
>   	   $(CPPFLAGS-$(suffix $@)) \
>   	   $(foreach lib,$(libof-$(basename $(@F))) \
>   			 $(libof-$(<F)) $(libof-$(@F)),$(CPPFLAGS-$(lib))) \
>
Patchwork Bot Aug. 22, 2014, 2:22 p.m. UTC | #2
On 22 August 2014 19:01, Stefan Liebler <stli@linux.vnet.ibm.com> wrote:
> Hi Siddhesh,
>
> I扉e tested your is_in_module branch on s390x.

Thanks!

> But building the testsuite fails while building nptl_db/db-symbols.v.iT
> because NOT_IN is not defined in structs.def:
> structs.def:81:12: error: missing binary operator before token "("
>  #if NOT_IN (libpthread) || TLS_TCB_AT_TP
>             ^
>
> The defining NOT_IN to zero of patch 19 is not done, because of patch v1.1
> 20.
>
> patch 19:
> +#ifndef _LIBC
> +# define NOT_IN(lib) (0)
> +#endif
>
> patch v1.1 20:
> +          -D_LIBC -include $(common-objpfx)config.h \
>
>
> After changing patch 19 to
> #ifndef NOT_IN
> # define NOT_IN(lib) (0)
> #endif

I think I need to generate libc-modules.h without depending on soversions.i.

> the following tests are failing:
> FAIL: conform/ISO/stdlib.h/conform
> FAIL: conform/ISO11/stdlib.h/conform
> FAIL: conform/ISO99/stdlib.h/conform
> FAIL: conform/POSIX/mqueue.h/conform
> FAIL: conform/POSIX/stdlib.h/conform
> FAIL: conform/POSIX2008/mqueue.h/conform
> FAIL: conform/POSIX2008/stdlib.h/conform
> FAIL: conform/UNIX98/mqueue.h/conform
> FAIL: conform/XOPEN2K/stdlib.h/conform
> FAIL: conform/XOPEN2K8/mqueue.h/conform
> FAIL: conform/XOPEN2K8/stdlib.h/conform
> with either:
>>Namespace violation: "IS_IN"
> or:

OK, that's porobably a good reason to not try to define IS_IN or
NOT_IN and just use _LIBC in the conditionals.

I'll fix these things up and update the branch.

Thanks,
Siddhesh
diff mbox

Patch

diff --git a/Makeconfig b/Makeconfig
index df26cd0..c6eae06 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -845,6 +845,7 @@  override CXXFLAGS = $(c++-sysincludes) \
 MIN-CPPFLAGS = $(config-extra-cppflags) $(CPPUNDEFS) $(CPPFLAGS-config) \
 	   $($(subdir)-CPPFLAGS) \
 	   $(+includes) $(defines) $(sysdep-CPPFLAGS) \
+	   -D_LIBC -include $(common-objpfx)config.h \
 	   $(CPPFLAGS-$(suffix $@)) \
 	   $(foreach lib,$(libof-$(basename $(@F))) \
 			 $(libof-$(<F)) $(libof-$(@F)),$(CPPFLAGS-$(lib))) \