Warning in gcc/libiberty/dyn-string.c during build

Warning in gcc/libiberty/dyn-string.c during build

[nick]

Greetings all,
I just got this in my build output:
ar: `u' modifier ignored since `D' is the default (see `U')                                                                                                                    
configure: WARNING: cannot check for properly working vsnprintf when cross compiling, will assume it's ok                                                                      
../../gcc/libiberty/dyn-string.c: In function ‘dyn_string_insert_cstr’:                                                                                                        
 ../../gcc/libiberty/dyn-string.c:280:3: warning: ‘strncpy’ output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]   
 strncpy (dest->s + pos, src, length);                                                                                                                                
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                                                                                                 
 ../../gcc/libiberty/dyn-string.c:272:16: note: length computed here                                                                                                            
272 |   int length = strlen (src);                                                                                                                                           
|                ^~~~~~~~~~~~                                                                                                                                            
 ../../gcc/libiberty/dyn-string.c: In function ‘dyn_string_insert_cstr’:                                                                                                        
\ ../../gcc/libiberty/dyn-string.c:280:3: warning: ‘strncpy’ output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]   
280 |   strncpy (dest->s + pos, src, length);                                                                                                                                
|   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                                                                                                 
 ../../gcc/libiberty/dyn-string.c:272:16: note: length computed here                                                                                                            
272 |   int length = strlen (src);                                                                                                                                           
|                ^~~~~~~~~~~~    

I've already looked through git blame and it seems this code was last touched in 2000. That warning seems
to be  new to gcc 8 after a little research so is this a rather old bug that was not found and very
subtle or is this a mislabel. Seems to be a mislabel to me but I'm new to the code base so just thought
I would ask.

Cheers,
Nick

Re: Warning in gcc/libiberty/dyn-string.c during build

[Martin Sebor]

On 3/23/19 9:49 PM, nick wrote:
> Greetings all,
> I just got this in my build output:
> ar: `u' modifier ignored since `D' is the default (see `U')
> configure: WARNING: cannot check for properly working vsnprintf when cross compiling, will assume it's ok
> ../../gcc/libiberty/dyn-string.c: In function ‘dyn_string_insert_cstr’:
>   ../../gcc/libiberty/dyn-string.c:280:3: warning: ‘strncpy’ output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
>   strncpy (dest->s + pos, src, length);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   ../../gcc/libiberty/dyn-string.c:272:16: note: length computed here
> 272 |   int length = strlen (src);
> |                ^~~~~~~~~~~~
>   ../../gcc/libiberty/dyn-string.c: In function ‘dyn_string_insert_cstr’:
> \ ../../gcc/libiberty/dyn-string.c:280:3: warning: ‘strncpy’ output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
> 280 |   strncpy (dest->s + pos, src, length);
> |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   ../../gcc/libiberty/dyn-string.c:272:16: note: length computed here
> 272 |   int length = strlen (src);
> |                ^~~~~~~~~~~~
> 
> I've already looked through git blame and it seems this code was last touched in 2000. That warning seems
> to be  new to gcc 8 after a little research so is this a rather old bug that was not found and very
> subtle or is this a mislabel. Seems to be a mislabel to me but I'm new to the code base so just thought
> I would ask.

The warning detects strncpy calls that create unterminated string
copies.  That can happen for example when the function is misused
by specifying a bound that's equal to the length of the source,
as in:

   void f (char *d, const char *s)
   {
     int n = strlen (s);
     strncpy (d, s, n);
   }

But the warning is far from perfect and it cannot distinguish
all the incorrect misuses from the benign ones.  For instance,
it triggers in the case below even though the copy is nul
terminated:

   void g (char *d, const char *s)
   {
     int n = strlen (s);
     d[n] = 0;
     strncpy (d, s, n);
   }

In dyn_string_insert_cstr(), although the strnlen call itself
doesn't nul-terminate the copy (and so the warning isn't strictly
incorrect), the loop before the call does make sure the copy is
nul-terminated (similarly to function g above), and so the result
is a valid nul-terminated string.

I've been working on improving the warning to detect more instances
of nul-termination but I don't expect it to ever be smart enough
to figure out cases as complex as this one.  Using memcpy instead
of strncpy would avoid the warning.  (The loop above the strncpy
call could also be replaced by a call to memmove for efficiency.)

Martin

Re: Warning in gcc/libiberty/dyn-string.c during build

[Jeff Law]

On 3/25/19 10:39 AM, Martin Sebor wrote:
> On 3/23/19 9:49 PM, nick wrote:
>> Greetings all,
>> I just got this in my build output:
>> ar: `u' modifier ignored since `D' is the default (see `U')
>> configure: WARNING: cannot check for properly working vsnprintf when
>> cross compiling, will assume it's ok
>> ../../gcc/libiberty/dyn-string.c: In function ‘dyn_string_insert_cstr’:
>>   ../../gcc/libiberty/dyn-string.c:280:3: warning: ‘strncpy’ output
>> truncated before terminating nul copying as many bytes from a string
>> as its length [-Wstringop-truncation]
>>   strncpy (dest->s + pos, src, length);
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>   ../../gcc/libiberty/dyn-string.c:272:16: note: length computed here
>> 272 |   int length = strlen (src);
>> |                ^~~~~~~~~~~~
>>   ../../gcc/libiberty/dyn-string.c: In function ‘dyn_string_insert_cstr’:
>> \ ../../gcc/libiberty/dyn-string.c:280:3: warning: ‘strncpy’ output
>> truncated before terminating nul copying as many bytes from a string
>> as its length [-Wstringop-truncation]
>> 280 |   strncpy (dest->s + pos, src, length);
>> |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>   ../../gcc/libiberty/dyn-string.c:272:16: note: length computed here
>> 272 |   int length = strlen (src);
>> |                ^~~~~~~~~~~~
>>
>> I've already looked through git blame and it seems this code was last
>> touched in 2000. That warning seems
>> to be  new to gcc 8 after a little research so is this a rather old
>> bug that was not found and very
>> subtle or is this a mislabel. Seems to be a mislabel to me but I'm new
>> to the code base so just thought
>> I would ask.
> 
> The warning detects strncpy calls that create unterminated string
> copies.  That can happen for example when the function is misused
> by specifying a bound that's equal to the length of the source,
> as in:
> 
>   void f (char *d, const char *s)
>   {
>     int n = strlen (s);
>     strncpy (d, s, n);
>   }
> 
> But the warning is far from perfect and it cannot distinguish
> all the incorrect misuses from the benign ones.  For instance,
> it triggers in the case below even though the copy is nul
> terminated:
> 
>   void g (char *d, const char *s)
>   {
>     int n = strlen (s);
>     d[n] = 0;
>     strncpy (d, s, n);
>   }
The dynamic case is painful :-)






> In dyn_string_insert_cstr(), although the strnlen call itself
> doesn't nul-terminate the copy (and so the warning isn't strictly
> incorrect), the loop before the call does make sure the copy is
> nul-terminated (similarly to function g above), and so the result
> is a valid nul-terminated string.
This reminds me a bit of some of the failure to eliminate dead stores
problems that are in BZ as well as some of the uninit issues for memory
references that's been rattling around in my head.   It's also related
to SRA.  Richi has stated (and I tend to agree) there's a goodly amount
of common analysis that can probably be shared across these problems.

I don't know if there's a grand unifying analysis that will tackle all
of this, but it certainly feels like there's at least something worth
exploring in this space.

Jeff

Re: Warning in gcc/libiberty/dyn-string.c during build

[nick]

On 2019-03-25 3:45 p.m., Jeff Law wrote:
> On 3/25/19 10:39 AM, Martin Sebor wrote:
>> On 3/23/19 9:49 PM, nick wrote:
>>> Greetings all,
>>> I just got this in my build output:
>>> ar: `u' modifier ignored since `D' is the default (see `U')
>>> configure: WARNING: cannot check for properly working vsnprintf when
>>> cross compiling, will assume it's ok
>>> ../../gcc/libiberty/dyn-string.c: In function ‘dyn_string_insert_cstr’:
>>>   ../../gcc/libiberty/dyn-string.c:280:3: warning: ‘strncpy’ output
>>> truncated before terminating nul copying as many bytes from a string
>>> as its length [-Wstringop-truncation]
>>>   strncpy (dest->s + pos, src, length);
>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>   ../../gcc/libiberty/dyn-string.c:272:16: note: length computed here
>>> 272 |   int length = strlen (src);
>>> |                ^~~~~~~~~~~~
>>>   ../../gcc/libiberty/dyn-string.c: In function ‘dyn_string_insert_cstr’:
>>> \ ../../gcc/libiberty/dyn-string.c:280:3: warning: ‘strncpy’ output
>>> truncated before terminating nul copying as many bytes from a string
>>> as its length [-Wstringop-truncation]
>>> 280 |   strncpy (dest->s + pos, src, length);
>>> |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>   ../../gcc/libiberty/dyn-string.c:272:16: note: length computed here
>>> 272 |   int length = strlen (src);
>>> |                ^~~~~~~~~~~~
>>>
>>> I've already looked through git blame and it seems this code was last
>>> touched in 2000. That warning seems
>>> to be  new to gcc 8 after a little research so is this a rather old
>>> bug that was not found and very
>>> subtle or is this a mislabel. Seems to be a mislabel to me but I'm new
>>> to the code base so just thought
>>> I would ask.
>>
>> The warning detects strncpy calls that create unterminated string
>> copies.  That can happen for example when the function is misused
>> by specifying a bound that's equal to the length of the source,
>> as in:
>>
>>   void f (char *d, const char *s)
>>   {
>>     int n = strlen (s);
>>     strncpy (d, s, n);
>>   }
>>
>> But the warning is far from perfect and it cannot distinguish
>> all the incorrect misuses from the benign ones.  For instance,
>> it triggers in the case below even though the copy is nul
>> terminated:
>>
>>   void g (char *d, const char *s)
>>   {
>>     int n = strlen (s);
>>     d[n] = 0;
>>     strncpy (d, s, n);
>>   }
> The dynamic case is painful :-)
> 
> 
> 
> 
> 
> 
>> In dyn_string_insert_cstr(), although the strnlen call itself
>> doesn't nul-terminate the copy (and so the warning isn't strictly
>> incorrect), the loop before the call does make sure the copy is
>> nul-terminated (similarly to function g above), and so the result
>> is a valid nul-terminated string.
> This reminds me a bit of some of the failure to eliminate dead stores
> problems that are in BZ as well as some of the uninit issues for memory
> references that's been rattling around in my head.   It's also related
> to SRA.  Richi has stated (and I tend to agree) there's a goodly amount
> of common analysis that can probably be shared across these problems.
> 
> I don't know if there's a grand unifying analysis that will tackle all
> of this, but it certainly feels like there's at least something worth
> exploring in this space.
> 
> Jeff
> 

The easiest way forward for me is should I just use W=1 or something or
even better is just ctags and parse the calls in the parts you mentioned.
It's worth looking into as there may be longs of small callers that
actually really add up.

As for Martin don't know if I would be much help but if you want help with
the work on -nul-terimated let me known.

Cheers,

Nick