[PATCH] Fix build warnings in locale/programs/ld-ctype.c

[PATCH] Fix build warnings in locale/programs/ld-ctype.c

[Stefan Liebler]

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

Hi,

this patch fixes the gcc warnings seen with gcc 9 -march>=z13 on s390x:
programs/ld-ctype.c: In function ‘ctype_read’:
programs/ld-ctype.c:1392:13: error: ‘wch’ may be used uninitialized in 
this function [-Werror=maybe-uninitialized]
  1392 |    uint32_t wch;
       |             ^~~
programs/ld-ctype.c:1401:7: error: ‘seq’ may be used uninitialized in 
this function [-Werror=maybe-uninitialized]
  1401 |    if (seq != NULL && seq->nbytes == 1)
       |       ^
programs/ld-ctype.c:1391:20: note: ‘seq’ was declared here
  1391 |    struct charseq *seq;
       |                    ^~~

Both seq and wch are uninitialized if get_character fails.
Thus we are now returning with an error.

Bye
Stefan

ChangeLog:

	* locale/programs/ld-ctype.c (charclass_symbolic_ellipsis):
	Return error if get_character fails.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 20190625_warnings_locale_programs_ld-ctype.patch --]
[-- Type: text/x-patch; name="20190625_warnings_locale_programs_ld-ctype.patch", Size: 1667 bytes --]

commit 0dca38c4afd868da65e0b5c0d76fc30bd3114994
Author: Stefan Liebler <stli@linux.ibm.com>
Date:   Tue Jun 25 08:57:40 2019 +0200

    Fix build warnings in locale/programs/ld-ctype.c
    
    This patch fixes the gcc warnings seen with gcc 9 -march>=z13 on s390x:
    programs/ld-ctype.c: In function ‘ctype_read’:
    programs/ld-ctype.c:1392:13: error: ‘wch’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     1392 |    uint32_t wch;
          |             ^~~
    programs/ld-ctype.c:1401:7: error: ‘seq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     1401 |    if (seq != NULL && seq->nbytes == 1)
          |       ^
    programs/ld-ctype.c:1391:20: note: ‘seq’ was declared here
     1391 |    struct charseq *seq;
          |                    ^~~
    
    Both seq and wch are uninitialized if get_character fails.
    Thus we are now returning with an error.
    
    ChangeLog:
    
            * locale/programs/ld-ctype.c (charclass_symbolic_ellipsis):
            Return error if get_character fails.

diff --git a/locale/programs/ld-ctype.c b/locale/programs/ld-ctype.c
index e6105928da..cfc9c43fd5 100644
--- a/locale/programs/ld-ctype.c
+++ b/locale/programs/ld-ctype.c
@@ -1396,7 +1396,8 @@ charclass_symbolic_ellipsis (struct linereader *ldfile,
 		   (int) (now->val.str.lenmb - (cp - last_str)),
 		   from);
 
-	  get_character (now, charmap, repertoire, &seq, &wch);
+	  if (get_character (now, charmap, repertoire, &seq, &wch))
+	    goto invalid_range;
 
 	  if (seq != NULL && seq->nbytes == 1)
 	    /* Yep, we can store information about this byte sequence.  */

Re: [PATCH] Fix build warnings in locale/programs/ld-ctype.c

[Florian Weimer]

* Stefan Liebler:

> diff --git a/locale/programs/ld-ctype.c b/locale/programs/ld-ctype.c
> index e6105928da..cfc9c43fd5 100644
> --- a/locale/programs/ld-ctype.c
> +++ b/locale/programs/ld-ctype.c
> @@ -1396,7 +1396,8 @@ charclass_symbolic_ellipsis (struct linereader *ldfile,
>  		   (int) (now->val.str.lenmb - (cp - last_str)),
>  		   from);
>  
> -	  get_character (now, charmap, repertoire, &seq, &wch);
> +	  if (get_character (now, charmap, repertoire, &seq, &wch))
> +	    goto invalid_range;

Maybe write:

  if (get_character (now, charmap, repertoire, &seq, &wch) != 0)

to match the other function calls?

Otherwise, looks good.

Thanks,
Florian

Re: [PATCH] Fix build warnings in locale/programs/ld-ctype.c

[Stefan Liebler]

On 6/25/19 3:23 PM, Florian Weimer wrote:
> * Stefan Liebler:
> 
>> diff --git a/locale/programs/ld-ctype.c b/locale/programs/ld-ctype.c
>> index e6105928da..cfc9c43fd5 100644
>> --- a/locale/programs/ld-ctype.c
>> +++ b/locale/programs/ld-ctype.c
>> @@ -1396,7 +1396,8 @@ charclass_symbolic_ellipsis (struct linereader *ldfile,
>>   		   (int) (now->val.str.lenmb - (cp - last_str)),
>>   		   from);
>>   
>> -	  get_character (now, charmap, repertoire, &seq, &wch);
>> +	  if (get_character (now, charmap, repertoire, &seq, &wch))
>> +	    goto invalid_range;
> 
> Maybe write:
> 
>    if (get_character (now, charmap, repertoire, &seq, &wch) != 0)
> 
> to match the other function calls?
Okay. That's no problem. If no one opposes, I'll commit the patch 
tomorrow with "!= 0".

Shall I also update the following occurrence in ctype_read?
	      if (ellipsis_token == tok_none)
		{
		  if (get_character (now, charmap, repertoire, &seq, &wch))
		    goto err_label;

> 
> Otherwise, looks good.
> 
> Thanks,
> Florian
> 

Re: [PATCH] Fix build warnings in locale/programs/ld-ctype.c

[Florian Weimer]

* Stefan Liebler:

> On 6/25/19 3:23 PM, Florian Weimer wrote:
>> * Stefan Liebler:
>>
>>> diff --git a/locale/programs/ld-ctype.c b/locale/programs/ld-ctype.c
>>> index e6105928da..cfc9c43fd5 100644
>>> --- a/locale/programs/ld-ctype.c
>>> +++ b/locale/programs/ld-ctype.c
>>> @@ -1396,7 +1396,8 @@ charclass_symbolic_ellipsis (struct linereader *ldfile,
>>>   		   (int) (now->val.str.lenmb - (cp - last_str)),
>>>   		   from);
>>>   -	  get_character (now, charmap, repertoire, &seq, &wch);
>>> +	  if (get_character (now, charmap, repertoire, &seq, &wch))
>>> +	    goto invalid_range;
>>
>> Maybe write:
>>
>>    if (get_character (now, charmap, repertoire, &seq, &wch) != 0)
>>
>> to match the other function calls?
> Okay. That's no problem. If no one opposes, I'll commit the patch
> tomorrow with "!= 0".
>
> Shall I also update the following occurrence in ctype_read?
> 	      if (ellipsis_token == tok_none)
> 		{
> 		  if (get_character (now, charmap, repertoire, &seq, &wch))
> 		    goto err_label;

Oh, I had missed that.  If the calls are already inconsistent, you can
use your original patch, too.

To be honest, I'm more concerned about the other calls to get_character
without error checking.

Thanks,
Florian

Re: [PATCH] Fix build warnings in locale/programs/ld-ctype.c

[Stefan Liebler]

On 6/25/19 3:39 PM, Florian Weimer wrote:
> * Stefan Liebler:
> 
>> On 6/25/19 3:23 PM, Florian Weimer wrote:
>>> * Stefan Liebler:
>>>
>>>> diff --git a/locale/programs/ld-ctype.c b/locale/programs/ld-ctype.c
>>>> index e6105928da..cfc9c43fd5 100644
>>>> --- a/locale/programs/ld-ctype.c
>>>> +++ b/locale/programs/ld-ctype.c
>>>> @@ -1396,7 +1396,8 @@ charclass_symbolic_ellipsis (struct linereader *ldfile,
>>>>    		   (int) (now->val.str.lenmb - (cp - last_str)),
>>>>    		   from);
>>>>    -	  get_character (now, charmap, repertoire, &seq, &wch);
>>>> +	  if (get_character (now, charmap, repertoire, &seq, &wch))
>>>> +	    goto invalid_range;
>>>
>>> Maybe write:
>>>
>>>     if (get_character (now, charmap, repertoire, &seq, &wch) != 0)
>>>
>>> to match the other function calls?
>> Okay. That's no problem. If no one opposes, I'll commit the patch
>> tomorrow with "!= 0".
>>
>> Shall I also update the following occurrence in ctype_read?
>> 	      if (ellipsis_token == tok_none)
>> 		{
>> 		  if (get_character (now, charmap, repertoire, &seq, &wch))
>> 		    goto err_label;
> 
> Oh, I had missed that.  If the calls are already inconsistent, you can
> use your original patch, too.
Okay. Then I'll use the original patch.
> 
> To be honest, I'm more concerned about the other calls to get_character
> without error checking.
Where do you see other ones?
With my patch applied, I just see the following occurrences of 
get_character which are now all checking the return value:
1257:get_character (struct token *now, const struct charmap_t *charmap,
1399:if (get_character (now, charmap, repertoire, &seq, &wch))
2291:if (get_character (now, charmap, repertoire, &seq, &wch))
2574:if (get_character (now, charmap, repertoire, &from_seq,
2585:if (get_character (now, charmap, repertoire, &to_seq,
> 
> Thanks,
> Florian
> 

Re: [PATCH] Fix build warnings in locale/programs/ld-ctype.c

[Florian Weimer]

* Stefan Liebler:

>> To be honest, I'm more concerned about the other calls to get_character
>> without error checking.

> Where do you see other ones?
> With my patch applied, I just see the following occurrences of
> get_character which are now all checking the return value:
> 1257:get_character (struct token *now, const struct charmap_t *charmap,
> 1399:if (get_character (now, charmap, repertoire, &seq, &wch))
> 2291:if (get_character (now, charmap, repertoire, &seq, &wch))
> 2574:if (get_character (now, charmap, repertoire, &from_seq,
> 2585:if (get_character (now, charmap, repertoire, &to_seq,

Ah, my bad, you are right.  Please commit the original patch.

Thanks,
Florian

Re: [PATCH] Fix build warnings in locale/programs/ld-ctype.c

[Stefan Liebler]

On 6/25/19 3:54 PM, Florian Weimer wrote:
> * Stefan Liebler:
> 
>>> To be honest, I'm more concerned about the other calls to get_character
>>> without error checking.
> 
>> Where do you see other ones?
>> With my patch applied, I just see the following occurrences of
>> get_character which are now all checking the return value:
>> 1257:get_character (struct token *now, const struct charmap_t *charmap,
>> 1399:if (get_character (now, charmap, repertoire, &seq, &wch))
>> 2291:if (get_character (now, charmap, repertoire, &seq, &wch))
>> 2574:if (get_character (now, charmap, repertoire, &from_seq,
>> 2585:if (get_character (now, charmap, repertoire, &to_seq,
> 
> Ah, my bad, you are right.  Please commit the original patch.
> 
> Thanks,
> Florian
> 
Committed.

Thanks