ToT build error with ToT GCC on Aarch64

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

ToT build error with ToT GCC on Aarch64

Steve Ellcey-5
I have run into a problem when building the ToT glibc with the ToT GCC on
Aarch64.  I haven't dug into this enough to know if this is a GCC problem,
a glibc problem, or just a message that needs to be ignored but I wanted to
send out an email in case this is something that needs to be addressed
before GCC 2.28 is released.

Steve Ellcey
[hidden email]


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
Reply | Threaded
Open this post in threaded view
|

Re: ToT build error with ToT GCC on Aarch64

Jeff Law
On 07/23/2018 09:51 AM, Steve Ellcey wrote:
> I have run into a problem when building the ToT glibc with the ToT GCC on
> Aarch64.  I haven't dug into this enough to know if this is a GCC problem,
> a glibc problem, or just a message that needs to be ignored but I wanted to
> send out an email in case this is something that needs to be addressed
> before GCC 2.28 is released.
Martin S. and I are already looking at this and I've already asked
Martin to bring in Carlos and Florian.

Our initial read is that it's a valid warning, but neither of us knows
this code, so it's a very preliminary finding.

jeff
Reply | Threaded
Open this post in threaded view
|

Re: ToT build error with ToT GCC on Aarch64

Martin Sebor-3
On 07/23/2018 10:19 AM, Jeff Law wrote:

> On 07/23/2018 09:51 AM, Steve Ellcey wrote:
>> I have run into a problem when building the ToT glibc with the ToT GCC on
>> Aarch64.  I haven't dug into this enough to know if this is a GCC problem,
>> a glibc problem, or just a message that needs to be ignored but I wanted to
>> send out an email in case this is something that needs to be addressed
>> before GCC 2.28 is released.
> Martin S. and I are already looking at this and I've already asked
> Martin to bring in Carlos and Florian.
>
> Our initial read is that it's a valid warning, but neither of us knows
> this code, so it's a very preliminary finding.
I spent some time reducing it to a smaller test case over
the weekend to better see what's going on here.  My reading
of the code is below but as Jeff already suggested, having
someone familiar with it either confirm it or point out what
I missed would be helpful.

internal_fnwmatch() has this:

     wint_t str;
     ...
     const wint_t *cp = (const wint_t *) &str;
     ...
     idx = findidxwc (table, indirect, extra, &cp, 1);

findidxwc (const int32_t *table, const int32_t *indirect,
            const wint_t *extra, const wint_t **cpp, size_t len)

has this (comments mine):

   wint_t ch = *(*cpp)++;   // advance *cpp past the end of &str
   ...
     const int32_t *usrc = (const int32_t *) *cpp;
     ...
     for (cnt = 0; cnt < nhere && cnt < len; ++cnt)
       if (cp[cnt] != usrc[cnt])   // access (&str + 1)[0]
          break;

It looks to me like the first and only iteration of the loop
accesses (&str + 1)[0].

Martin

PS The test case I boiled it down to is attached.

fnmatch.c (9K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: ToT build error with ToT GCC on Aarch64

Florian Weimer-5
In reply to this post by Jeff Law
On 07/23/2018 06:19 PM, Jeff Law wrote:
> On 07/23/2018 09:51 AM, Steve Ellcey wrote:
>> I have run into a problem when building the ToT glibc with the ToT GCC on
>> Aarch64.  I haven't dug into this enough to know if this is a GCC problem,
>> a glibc problem, or just a message that needs to be ignored but I wanted to
>> send out an email in case this is something that needs to be addressed
>> before GCC 2.28 is released.

> Martin S. and I are already looking at this and I've already asked
> Martin to bring in Carlos and Florian.

Would it be possible to print the full inlining stack for these GCC
warnings?  I don't see a %K format specifier (or whatever is used for this).

I think it would make it a bit easier to track these down.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: ToT build error with ToT GCC on Aarch64

Zach van Rijn
In reply to this post by Martin Sebor-3
On Mon, 2018-07-23 at 11:13 -0600, Martin Sebor wrote:

> On 07/23/2018 10:19 AM, Jeff Law wrote:
> > On 07/23/2018 09:51 AM, Steve Ellcey wrote:
> > > I have run into a problem when building the ToT glibc with
> > > the ToT GCC on
> > > Aarch64.  I haven't dug into this enough to know if this is
> > > a GCC problem,
> > > a glibc problem, or just a message that needs to be ignored
> > > but I wanted to
> > > send out an email in case this is something that needs to
> > > be addressed
> > > before GCC 2.28 is released.
> >
> > Martin S. and I are already looking at this and I've already
> > asked
> > Martin to bring in Carlos and Florian.
> >
> > Our initial read is that it's a valid warning, but neither of
> > us knows
> > this code, so it's a very preliminary finding.
>
> I spent some time reducing it to a smaller test case over
> the weekend to better see what's going on here.  My reading
> of the code is below but as Jeff already suggested, having
> someone familiar with it either confirm it or point out what
> I missed would be helpful.
>
> internal_fnwmatch() has this:
...

>      ...
>      for (cnt = 0; cnt < nhere && cnt < len; ++cnt)
>        if (cp[cnt] != usrc[cnt])   // access (&str + 1)[0]
>           break;
>
> It looks to me like the first and only iteration of the loop
> accesses (&str + 1)[0].
>
> Martin
>
> PS The test case I boiled it down to is attached.

Hi Martin,

I'm not sure if this is the source of the issue, but variable
`c1` is initialized to be zero, so when `nrules` is non-zero AND
the condition -- if (c == L'.' && p[1] == L']') -- is true, then
`c1` remains at zero even during the declaration of array `str`.

...
size_t c1 = 0;
while (1)
{
    c = *++p;
    if (c == L'.'  && p[1] == L']')
    {
        p += 2;
        break;        // [A] below `c1` increment will not occur
    }
    if (c == '\0')
        return 1;
    ++c1;
}
if (nrules == 0)
{
    ...
}
else                  // [B] this branch is usually taken
{
    char str[c1];     // [C] causing this to be `char str[0];`
    ...
}
...

So when [A] and [B], [C] is a problem. This occurs in two places.

ZV

Reply | Threaded
Open this post in threaded view
|

Re: ToT build error with ToT GCC on Aarch64

Martin Sebor-3
In reply to this post by Florian Weimer-5
On 07/23/2018 11:26 AM, Florian Weimer wrote:

> On 07/23/2018 06:19 PM, Jeff Law wrote:
>> On 07/23/2018 09:51 AM, Steve Ellcey wrote:
>>> I have run into a problem when building the ToT glibc with the ToT
>>> GCC on
>>> Aarch64.  I haven't dug into this enough to know if this is a GCC
>>> problem,
>>> a glibc problem, or just a message that needs to be ignored but I
>>> wanted to
>>> send out an email in case this is something that needs to be addressed
>>> before GCC 2.28 is released.
>
>> Martin S. and I are already looking at this and I've already asked
>> Martin to bring in Carlos and Florian.
>
> Would it be possible to print the full inlining stack for these GCC
> warnings?  I don't see a %K format specifier (or whatever is used for
> this).
>
> I think it would make it a bit easier to track these down.

It's one of my own pet peeves with the -Warray-bounds warnings
that they don't do that.  I've quickly hacked GCC to print
the inlining stack.  It doesn't look to me like it helps that
much but I'll let you be the judge:

In file included from fnmatch.c:244:
In function ‘findidxwc’,
     inlined from ‘internal_fnwmatch’ at fnmatch_loop.c:404:10:
../locale/weightwc.h:124:28: warning: array subscript 1 is outside array
bounds of ‘wint_t[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]

Martin
Reply | Threaded
Open this post in threaded view
|

Re: ToT build error with ToT GCC on Aarch64

Martin Sebor-3
On 07/23/2018 12:29 PM, Martin Sebor wrote:

> On 07/23/2018 11:26 AM, Florian Weimer wrote:
>> On 07/23/2018 06:19 PM, Jeff Law wrote:
>>> On 07/23/2018 09:51 AM, Steve Ellcey wrote:
>>>> I have run into a problem when building the ToT glibc with the ToT
>>>> GCC on
>>>> Aarch64.  I haven't dug into this enough to know if this is a GCC
>>>> problem,
>>>> a glibc problem, or just a message that needs to be ignored but I
>>>> wanted to
>>>> send out an email in case this is something that needs to be addressed
>>>> before GCC 2.28 is released.
>>
>>> Martin S. and I are already looking at this and I've already asked
>>> Martin to bring in Carlos and Florian.
>>
>> Would it be possible to print the full inlining stack for these GCC
>> warnings?  I don't see a %K format specifier (or whatever is used for
>> this).
>>
>> I think it would make it a bit easier to track these down.
>
> It's one of my own pet peeves with the -Warray-bounds warnings
> that they don't do that.  I've quickly hacked GCC to print
> the inlining stack.  It doesn't look to me like it helps that
> much but I'll let you be the judge:
>
> In file included from fnmatch.c:244:
> In function ‘findidxwc’,
>     inlined from ‘internal_fnwmatch’ at fnmatch_loop.c:404:10:
> ../locale/weightwc.h:124:28: warning: array subscript 1 is outside array
> bounds of ‘wint_t[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]

Here's a bit more context showing the variable being referenced
(for simplicity, GCC takes singleton objects as arrays of one
element):

In file included from fnmatch.c:244:
In function ‘findidxwc’,
     inlined from ‘internal_fnwmatch’ at fnmatch_loop.c:404:10:
../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:315:
fnmatch_loop.c: In function ‘internal_fnwmatch’:
fnmatch_loop.c:342:13: note: while referencing ‘str’
        UCHAR str;
              ^~~

There's an interesting comment above the declaration:

     /* It's important that STR be a scalar variable rather
        than a one-element array, because GCC (at least 4.9.2
        -O2 on x86-64) can be confused by the array and
        diagnose a "used initialized" in a dead branch in the
        findidx function.  */
     UCHAR str;

Martin
Reply | Threaded
Open this post in threaded view
|

Re: ToT build error with ToT GCC on Aarch64

Jeff Law
On 07/23/2018 12:51 PM, Martin Sebor wrote:

> On 07/23/2018 12:29 PM, Martin Sebor wrote:
>> On 07/23/2018 11:26 AM, Florian Weimer wrote:
>>> On 07/23/2018 06:19 PM, Jeff Law wrote:
>>>> On 07/23/2018 09:51 AM, Steve Ellcey wrote:
>>>>> I have run into a problem when building the ToT glibc with the ToT
>>>>> GCC on
>>>>> Aarch64.  I haven't dug into this enough to know if this is a GCC
>>>>> problem,
>>>>> a glibc problem, or just a message that needs to be ignored but I
>>>>> wanted to
>>>>> send out an email in case this is something that needs to be addressed
>>>>> before GCC 2.28 is released.
>>>
>>>> Martin S. and I are already looking at this and I've already asked
>>>> Martin to bring in Carlos and Florian.
>>>
>>> Would it be possible to print the full inlining stack for these GCC
>>> warnings?  I don't see a %K format specifier (or whatever is used for
>>> this).
>>>
>>> I think it would make it a bit easier to track these down.
>>
>> It's one of my own pet peeves with the -Warray-bounds warnings
>> that they don't do that.  I've quickly hacked GCC to print
>> the inlining stack.  It doesn't look to me like it helps that
>> much but I'll let you be the judge:
>>
>> In file included from fnmatch.c:244:
>> In function ‘findidxwc’,
>>     inlined from ‘internal_fnwmatch’ at fnmatch_loop.c:404:10:
>> ../locale/weightwc.h:124:28: warning: array subscript 1 is outside array
>> bounds of ‘wint_t[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>
> Here's a bit more context showing the variable being referenced
> (for simplicity, GCC takes singleton objects as arrays of one
> element):
>
> In file included from fnmatch.c:244:
> In function ‘findidxwc’,
>     inlined from ‘internal_fnwmatch’ at fnmatch_loop.c:404:10:
> ../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:315:
> fnmatch_loop.c: In function ‘internal_fnwmatch’:
> fnmatch_loop.c:342:13: note: while referencing ‘str’
>        UCHAR str;
>              ^~~
>
> There's an interesting comment above the declaration:
>
>     /* It's important that STR be a scalar variable rather
>        than a one-element array, because GCC (at least 4.9.2
>        -O2 on x86-64) can be confused by the array and
>        diagnose a "used initialized" in a dead branch in the
>        findidx function.  */
>     UCHAR str;
And it gets even more interesting if you dig into the history of this code.

https://sourceware.org/ml/libc-alpha/2014-11/msg00459.html

The implication is this is dead code in the case we're looking at.

I wonder if the solution here is to disable the oob warning along with
the uninit warning.

jeff
Reply | Threaded
Open this post in threaded view
|

Re: ToT build error with ToT GCC on Aarch64

Jeff Law
On 07/23/2018 02:31 PM, Jeff Law wrote:

> On 07/23/2018 12:51 PM, Martin Sebor wrote:
>> On 07/23/2018 12:29 PM, Martin Sebor wrote:
>>> On 07/23/2018 11:26 AM, Florian Weimer wrote:
>>>> On 07/23/2018 06:19 PM, Jeff Law wrote:
>>>>> On 07/23/2018 09:51 AM, Steve Ellcey wrote:
>>>>>> I have run into a problem when building the ToT glibc with the ToT
>>>>>> GCC on
>>>>>> Aarch64.  I haven't dug into this enough to know if this is a GCC
>>>>>> problem,
>>>>>> a glibc problem, or just a message that needs to be ignored but I
>>>>>> wanted to
>>>>>> send out an email in case this is something that needs to be addressed
>>>>>> before GCC 2.28 is released.
>>>>
>>>>> Martin S. and I are already looking at this and I've already asked
>>>>> Martin to bring in Carlos and Florian.
>>>>
>>>> Would it be possible to print the full inlining stack for these GCC
>>>> warnings?  I don't see a %K format specifier (or whatever is used for
>>>> this).
>>>>
>>>> I think it would make it a bit easier to track these down.
>>>
>>> It's one of my own pet peeves with the -Warray-bounds warnings
>>> that they don't do that.  I've quickly hacked GCC to print
>>> the inlining stack.  It doesn't look to me like it helps that
>>> much but I'll let you be the judge:
>>>
>>> In file included from fnmatch.c:244:
>>> In function ‘findidxwc’,
>>>     inlined from ‘internal_fnwmatch’ at fnmatch_loop.c:404:10:
>>> ../locale/weightwc.h:124:28: warning: array subscript 1 is outside array
>>> bounds of ‘wint_t[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>>
>> Here's a bit more context showing the variable being referenced
>> (for simplicity, GCC takes singleton objects as arrays of one
>> element):
>>
>> In file included from fnmatch.c:244:
>> In function ‘findidxwc’,
>>     inlined from ‘internal_fnwmatch’ at fnmatch_loop.c:404:10:
>> ../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:315:
>> fnmatch_loop.c: In function ‘internal_fnwmatch’:
>> fnmatch_loop.c:342:13: note: while referencing ‘str’
>>        UCHAR str;
>>              ^~~
>>
>> There's an interesting comment above the declaration:
>>
>>     /* It's important that STR be a scalar variable rather
>>        than a one-element array, because GCC (at least 4.9.2
>>        -O2 on x86-64) can be confused by the array and
>>        diagnose a "used initialized" in a dead branch in the
>>        findidx function.  */
>>     UCHAR str;
> And it gets even more interesting if you dig into the history of this code.
>
> https://sourceware.org/ml/libc-alpha/2014-11/msg00459.html
>
> The implication is this is dead code in the case we're looking at.
>
> I wonder if the solution here is to disable the oob warning along with
> the uninit warning.

A key follow-up from Joseph (easy to get lost in the thread...)

https://sourceware.org/ml/libc-alpha/2014-11/msg00616.html
https://sourceware.org/ml/libc-alpha/2014-11/msg00635.html

Leading to...

https://sourceware.org/ml/libc-alpha/2014-11/msg00639.html

Jeff
Reply | Threaded
Open this post in threaded view
|

Re: Re: ToT build error with ToT GCC on Aarch64

Andre Vieira (lists)
On 23/07/18 21:36, Jeff Law wrote:

> On 07/23/2018 02:31 PM, Jeff Law wrote:
>> On 07/23/2018 12:51 PM, Martin Sebor wrote:
>>> On 07/23/2018 12:29 PM, Martin Sebor wrote:
>>>> On 07/23/2018 11:26 AM, Florian Weimer wrote:
>>>>> On 07/23/2018 06:19 PM, Jeff Law wrote:
>>>>>> On 07/23/2018 09:51 AM, Steve Ellcey wrote:
>>>>>>> I have run into a problem when building the ToT glibc with the ToT
>>>>>>> GCC on
>>>>>>> Aarch64.  I haven't dug into this enough to know if this is a GCC
>>>>>>> problem,
>>>>>>> a glibc problem, or just a message that needs to be ignored but I
>>>>>>> wanted to
>>>>>>> send out an email in case this is something that needs to be addressed
>>>>>>> before GCC 2.28 is released.
>>>>>
>>>>>> Martin S. and I are already looking at this and I've already asked
>>>>>> Martin to bring in Carlos and Florian.
>>>>>
>>>>> Would it be possible to print the full inlining stack for these GCC
>>>>> warnings?  I don't see a %K format specifier (or whatever is used for
>>>>> this).
>>>>>
>>>>> I think it would make it a bit easier to track these down.
>>>>
>>>> It's one of my own pet peeves with the -Warray-bounds warnings
>>>> that they don't do that.  I've quickly hacked GCC to print
>>>> the inlining stack.  It doesn't look to me like it helps that
>>>> much but I'll let you be the judge:
>>>>
>>>> In file included from fnmatch.c:244:
>>>> In function ‘findidxwc’,
>>>>     inlined from ‘internal_fnwmatch’ at fnmatch_loop.c:404:10:
>>>> ../locale/weightwc.h:124:28: warning: array subscript 1 is outside array
>>>> bounds of ‘wint_t[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>>>
>>> Here's a bit more context showing the variable being referenced
>>> (for simplicity, GCC takes singleton objects as arrays of one
>>> element):
>>>
>>> In file included from fnmatch.c:244:
>>> In function ‘findidxwc’,
>>>     inlined from ‘internal_fnwmatch’ at fnmatch_loop.c:404:10:
>>> ../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:315:
>>> fnmatch_loop.c: In function ‘internal_fnwmatch’:
>>> fnmatch_loop.c:342:13: note: while referencing ‘str’
>>>        UCHAR str;
>>>              ^~~
>>>
>>> There's an interesting comment above the declaration:
>>>
>>>     /* It's important that STR be a scalar variable rather
>>>        than a one-element array, because GCC (at least 4.9.2
>>>        -O2 on x86-64) can be confused by the array and
>>>        diagnose a "used initialized" in a dead branch in the
>>>        findidx function.  */
>>>     UCHAR str;
>> And it gets even more interesting if you dig into the history of this code.
>>
>> https://sourceware.org/ml/libc-alpha/2014-11/msg00459.html
>>
>> The implication is this is dead code in the case we're looking at.
>>
>> I wonder if the solution here is to disable the oob warning along with
>> the uninit warning.
>
> A key follow-up from Joseph (easy to get lost in the thread...)
>
> https://sourceware.org/ml/libc-alpha/2014-11/msg00616.html
> https://sourceware.org/ml/libc-alpha/2014-11/msg00635.html
>
> Leading to...
>
> https://sourceware.org/ml/libc-alpha/2014-11/msg00639.html
>
> Jeff
>
Hi,

I am seeing the same failure in our aarch64 builds, though when I first
saw this I thought it was something similar to BZ23396.

Carlos hints at that in
https://sourceware.org/ml/libc-alpha/2018-07/msg00388.html

If you look at how nhere is initialized in findidx in locale/weightwc.h:
      /* Next is the length of the byte sequence.  These are always
         short byte sequences so there is no reason to call any
         function (even if they are inlined).  */
      nhere = *cp++;

As far as I can tell, when we call this using a single char array, the
next element will not be the length of the byte sequence.

Exiting early when 'nhere >= len' makes the warning go away. I am not at
all familiar with this code though so I might be swinging at windmills here.

Cheers,
Andre
Reply | Threaded
Open this post in threaded view
|

Re: ToT build error with ToT GCC on Aarch64

Florian Weimer-5
In reply to this post by Martin Sebor-3
On 07/23/2018 08:51 PM, Martin Sebor wrote:

> Here's a bit more context showing the variable being referenced
> (for simplicity, GCC takes singleton objects as arrays of one
> element):
>
> In file included from fnmatch.c:244:
> In function ‘findidxwc’,
>      inlined from ‘internal_fnwmatch’ at fnmatch_loop.c:404:10:
> ../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:315:
> fnmatch_loop.c: In function ‘internal_fnwmatch’:
> fnmatch_loop.c:342:13: note: while referencing ‘str’
>         UCHAR str;
>               ^~~

I think that's more useful.  Even the %K change helps a little bit IMHO.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: ToT build error with ToT GCC on Aarch64

Jeff Law
On 07/24/2018 07:57 AM, Florian Weimer wrote:

> On 07/23/2018 08:51 PM, Martin Sebor wrote:
>> Here's a bit more context showing the variable being referenced
>> (for simplicity, GCC takes singleton objects as arrays of one
>> element):
>>
>> In file included from fnmatch.c:244:
>> In function ‘findidxwc’,
>>      inlined from ‘internal_fnwmatch’ at fnmatch_loop.c:404:10:
>> ../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:315:
>> fnmatch_loop.c: In function ‘internal_fnwmatch’:
>> fnmatch_loop.c:342:13: note: while referencing ‘str’
>>         UCHAR str;
>>               ^~~
>
> I think that's more useful.  Even the %K change helps a little bit IMHO.
Agreed.
jeff
Reply | Threaded
Open this post in threaded view
|

Re: ToT build error with ToT GCC on Aarch64

Andreas Schwab
In reply to this post by Steve Ellcey-5
On Jul 23 2018, Steve Ellcey <[hidden email]> 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(-)

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;
--
2.18.0


--
Andreas Schwab, SUSE Labs, [hidden email]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Reply | Threaded
Open this post in threaded view
|

Re: ToT build error with ToT GCC on Aarch64

Steve Ellcey-5
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
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: ToT build error with ToT GCC on Aarch64

Andreas Schwab
On Jul 24 2018, Steve Ellcey <[hidden email]> 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.

--
Andreas Schwab, SUSE Labs, [hidden email]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Reply | Threaded
Open this post in threaded view
|

Re: ToT build error with ToT GCC on Aarch64

Steve Ellcey-5
On Tue, 2018-07-24 at 18:33 +0200, Andreas Schwab wrote:

>
> On Jul 24 2018, Steve Ellcey <[hidden email]> 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
[hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: ToT build error with ToT GCC on Aarch64

Carlos O'Donell-6
In reply to this post by Andreas Schwab
On 07/24/2018 12:10 PM, Andreas Schwab wrote:

> On Jul 23 2018, Steve Ellcey <[hidden email]> 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;
>