Patchwork ToT build error with ToT GCC on Aarch64

login
register
mail settings
Submitter Andreas Schwab
Date July 24, 2018, 4:10 p.m.
Message ID <mvm8t601uoc.fsf@suse.de>
Download mbox | patch
Permalink /patch/28589/
State New
Headers show

Comments

Andreas Schwab - July 24, 2018, 4:10 p.m.
On Jul 23 2018, Steve Ellcey <sellcey@cavium.com> wrote:

> In file included from fnmatch.c:244:
> fnmatch_loop.c: In function ‘internal_fnwmatch’:
> ../locale/weightwc.h:124:28: error: array subscript 1 is outside array bounds of
>  ‘wint_t[1]’ {aka ‘unsigned int[1]’} [-Werror=array-bounds]
>     if (cp[nhere - 1] > usrc[nhere -1])
>                         ~~~~^~~~~~~~~~
> In file included from fnmatch.c:244:
> fnmatch_loop.c: In function ‘internal_fnwmatch’:
> ../locale/weightwc.h:124:28: error: array subscript 1 is outside array bounds of
>  ‘wint_t[1]’ {aka ‘unsigned int[1]’} [-Werror=array-bounds]
>     if (cp[nhere - 1] > usrc[nhere -1])
>                         ~~~~^~~~~~~~~~
> cc1: all warnings being treated as errors
> ../o-iterator.mk:9: recipe for target '/home/sellcey/tot/obj/glibc64/posix/fnmat
> ch.o' failed

I think this is the correct change.  The cnt == len check matches what
is done in weight.h, and is needed when nhere - 1 == len and usrc is a
prefix of cp.

Andreas.

	* locale/weightwc.h (findidx): Handle the case where usrc is a
	prefix of cp but one character too short.
---
 locale/weightwc.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Steve Ellcey - July 24, 2018, 4:26 p.m.
On Tue, 2018-07-24 at 18:10 +0200, Andreas Schwab wrote:

>         * locale/weightwc.h (findidx): Handle the case where usrc is a
>         prefix of cp but one character too short.

Were you able to build glibc after this patch?  I hacked around the
problem by using -Wno-error=array-bounds when compiling fnmatch.c
and I hit the problem again when compiling ibm1399.c and ibm1390.c
in the inconvdata directory.

Steve Ellcey
sellcey@cavium.com
Andreas Schwab - July 24, 2018, 4:33 p.m.
On Jul 24 2018, Steve Ellcey <sellcey@cavium.com> wrote:

> I hacked around the problem by using -Wno-error=array-bounds when
> compiling fnmatch.c and I hit the problem again when
> compiling ibm1399.c and ibm1390.c in the inconvdata directory.

This is already fixed.

Andreas.
Steve Ellcey - July 24, 2018, 5:38 p.m.
On Tue, 2018-07-24 at 18:33 +0200, Andreas Schwab wrote:
> 
> On Jul 24 2018, Steve Ellcey <sellcey@cavium.com> wrote:
> 
> > 
> > I hacked around the problem by using -Wno-error=array-bounds when
> > compiling fnmatch.c and I hit the problem again when
> > compiling ibm1399.c and ibm1390.c in the inconvdata directory.
> This is already fixed.
> 
> Andreas.

OK, I picked up that patch and then applied this patch one to
weightwc.h and glibc builds for me now.

Steve Ellcey
sellcey@cavium.com
Carlos O'Donell - July 24, 2018, 7:21 p.m.
On 07/24/2018 12:10 PM, Andreas Schwab wrote:
> On Jul 23 2018, Steve Ellcey <sellcey@cavium.com> wrote:
> 
>> In file included from fnmatch.c:244:
>> fnmatch_loop.c: In function ‘internal_fnwmatch’:
>> ../locale/weightwc.h:124:28: error: array subscript 1 is outside array bounds of
>>  ‘wint_t[1]’ {aka ‘unsigned int[1]’} [-Werror=array-bounds]
>>     if (cp[nhere - 1] > usrc[nhere -1])
>>                         ~~~~^~~~~~~~~~
>> In file included from fnmatch.c:244:
>> fnmatch_loop.c: In function ‘internal_fnwmatch’:
>> ../locale/weightwc.h:124:28: error: array subscript 1 is outside array bounds of
>>  ‘wint_t[1]’ {aka ‘unsigned int[1]’} [-Werror=array-bounds]
>>     if (cp[nhere - 1] > usrc[nhere -1])
>>                         ~~~~^~~~~~~~~~
>> cc1: all warnings being treated as errors
>> ../o-iterator.mk:9: recipe for target '/home/sellcey/tot/obj/glibc64/posix/fnmat
>> ch.o' failed
> 
> I think this is the correct change.  The cnt == len check matches what
> is done in weight.h, and is needed when nhere - 1 == len and usrc is a
> prefix of cp.

I haven't had a chance to review this and probably won't get to it until
August. If you think this is correct, then feel free to commit this for
glibc 2.28. You really only make one logical change here and we can continue
to validate it. I will have to audit all of this because I still see widechar
failures in my C.UTF-8 full code-point sorting, so something is wrong in this
code.

> Andreas.
> 

https://sourceware.org/bugzilla/show_bug.cgi?id=23442

	[BZ #23442]
> 	* locale/weightwc.h (findidx): Handle the case where usrc is a
> 	prefix of cp but one character too short.
> ---
>  locale/weightwc.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/locale/weightwc.h b/locale/weightwc.h
> index 36c65b5623..7ee335dc9a 100644
> --- a/locale/weightwc.h
> +++ b/locale/weightwc.h
> @@ -109,7 +109,7 @@ findidx (const int32_t *table,
>  	      break;
>  	  DIAG_POP_NEEDS_COMMENT;
>  
> -	  if (cnt < nhere - 1)
> +	  if (cnt < nhere - 1 || cnt == len)
>  	    {
>  	      cp += 2 * nhere;
>  	      continue;
> @@ -121,14 +121,14 @@ findidx (const int32_t *table,
>  	     same reason as described above.  */
>  	  DIAG_PUSH_NEEDS_COMMENT;
>  	  DIAG_IGNORE_Os_NEEDS_COMMENT (7, "-Wmaybe-uninitialized");
> -	  if (cp[nhere - 1] > usrc[nhere -1])
> +	  if (cp[nhere - 1] > usrc[nhere - 1])
>  	    {
>  	      cp += 2 * nhere;
>  	      continue;
>  	    }
>  	  DIAG_POP_NEEDS_COMMENT;
>  
> -	  if (cp[2 * nhere - 1] < usrc[nhere -1])
> +	  if (cp[2 * nhere - 1] < usrc[nhere - 1])
>  	    {
>  	      cp += 2 * nhere;
>  	      continue;
>

Patch

diff --git a/locale/weightwc.h b/locale/weightwc.h
index 36c65b5623..7ee335dc9a 100644
--- a/locale/weightwc.h
+++ b/locale/weightwc.h
@@ -109,7 +109,7 @@  findidx (const int32_t *table,
 	      break;
 	  DIAG_POP_NEEDS_COMMENT;
 
-	  if (cnt < nhere - 1)
+	  if (cnt < nhere - 1 || cnt == len)
 	    {
 	      cp += 2 * nhere;
 	      continue;
@@ -121,14 +121,14 @@  findidx (const int32_t *table,
 	     same reason as described above.  */
 	  DIAG_PUSH_NEEDS_COMMENT;
 	  DIAG_IGNORE_Os_NEEDS_COMMENT (7, "-Wmaybe-uninitialized");
-	  if (cp[nhere - 1] > usrc[nhere -1])
+	  if (cp[nhere - 1] > usrc[nhere - 1])
 	    {
 	      cp += 2 * nhere;
 	      continue;
 	    }
 	  DIAG_POP_NEEDS_COMMENT;
 
-	  if (cp[2 * nhere - 1] < usrc[nhere -1])
+	  if (cp[2 * nhere - 1] < usrc[nhere - 1])
 	    {
 	      cp += 2 * nhere;
 	      continue;