Add <limits.h> integer width macros
diff mbox

Message ID alpine.DEB.2.20.1610041549540.14477@digraph.polyomino.org.uk
State New
Headers show

Commit Message

Joseph Myers Oct. 4, 2016, 3:51 p.m. UTC
On Tue, 4 Oct 2016, Florian Weimer wrote:

> On 10/04/2016 05:07 PM, Paul Eggert wrote:
> > On 09/16/2016 05:32 PM, Paul Eggert wrote:
> > > the patch looks good; thanks.
> > 
> > Unfortunately I did not check the patch thoroughly enough, as the patch
> > turns out to define LONG_WIDTH incorrectly on i686 with
> > gcc-6.2.1-2.fc26.i686, and this causes grep's 'make check' to fail
> > (luckily these are side tests; grep itself works fine). See:
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1381582
> 
> See comment 5 there.  The problem seems to be that GCC <limits.h> is included
> first, followed by glibc <limits.h>, which then tries to get the definitions
> from GCC.  But at this point, nothing happens because the GCC include guards
> cause the GCC header to be skipped.

Presumably this is an area where include ordering with installed headers 
differs from include ordering when building the glibc testsuite.  Does 
this patch work in the installed header context to fix the reported issue?  
(I'm running tests of it in the normal glibc testsuite context.)

Fix LONG_WIDTH, ULONG_WIDTH include ordering issue.

As described in
<https://sourceware.org/ml/libc-alpha/2016-10/msg00047.html>, there is
an include ordering issue with the integer width macros in glibc's
<limits.h>, where definitions conditional on LONG_MAX do not work as
intended because when the headers are installed, this part of glibc's
<limits.h> is processed before the part of GCC's <limits.h> that will
define LONG_MAX.  This patch changes the definitions just to use
__WORDSIZE for the expansion of LONG_WIDTH and ULONG_WIDTH rather than
making those definitions conditional on LONG_MAX.

2016-10-04  Joseph Myers  <joseph@codesourcery.com>

	* include/limits.h [__GLIBC_USE (IEC_60559_BFP_EXT)] (LONG_WIDTH):
	Define to __WORDSIZE, not conditional on [LONG_MAX ==
	0x7fffffffL].
	[__GLIBC_USE (IEC_60559_BFP_EXT)] (ULONG_WIDTH): Likewise.

Comments

Florian Weimer Oct. 4, 2016, 4:03 p.m. UTC | #1
On 10/04/2016 05:51 PM, Joseph Myers wrote:
> On Tue, 4 Oct 2016, Florian Weimer wrote:
>
>> On 10/04/2016 05:07 PM, Paul Eggert wrote:
>>> On 09/16/2016 05:32 PM, Paul Eggert wrote:
>>>> the patch looks good; thanks.
>>>
>>> Unfortunately I did not check the patch thoroughly enough, as the patch
>>> turns out to define LONG_WIDTH incorrectly on i686 with
>>> gcc-6.2.1-2.fc26.i686, and this causes grep's 'make check' to fail
>>> (luckily these are side tests; grep itself works fine). See:
>>>
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1381582
>>
>> See comment 5 there.  The problem seems to be that GCC <limits.h> is included
>> first, followed by glibc <limits.h>, which then tries to get the definitions
>> from GCC.  But at this point, nothing happens because the GCC include guards
>> cause the GCC header to be skipped.
>
> Presumably this is an area where include ordering with installed headers
> differs from include ordering when building the glibc testsuite.  Does
> this patch work in the installed header context to fix the reported issue?
> (I'm running tests of it in the normal glibc testsuite context.)

I applied the patch to the installed header and it fixes the reported 
issue about LONG_WIDTH.

Thanks,
Florian
Joseph Myers Oct. 4, 2016, 4:19 p.m. UTC | #2
On Tue, 4 Oct 2016, Florian Weimer wrote:

> I applied the patch to the installed header and it fixes the reported issue
> about LONG_WIDTH.

Thanks, I've committed the patch after my testing on x86_64 and x86 
completed.

Patch
diff mbox

diff --git a/include/limits.h b/include/limits.h
index 93cac49..dd601d5 100644
--- a/include/limits.h
+++ b/include/limits.h
@@ -164,20 +164,11 @@ 
 # ifndef UINT_WIDTH
 #  define UINT_WIDTH 32
 # endif
-# if LONG_MAX == 0x7fffffffL
-#  ifndef LONG_WIDTH
-#   define LONG_WIDTH 32
-#  endif
-#  ifndef ULONG_WIDTH
-#   define ULONG_WIDTH 32
-#  endif
-# else
-#  ifndef LONG_WIDTH
-#   define LONG_WIDTH 64
-#  endif
-#  ifndef ULONG_WIDTH
-#   define ULONG_WIDTH 64
-#  endif
+# ifndef LONG_WIDTH
+#  define LONG_WIDTH __WORDSIZE
+# endif
+# ifndef ULONG_WIDTH
+#  define ULONG_WIDTH __WORDSIZE
 # endif
 # ifndef LLONG_WIDTH
 #  define LLONG_WIDTH 64