[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3] tools/libs/stat: fix broken build

  • To: Jürgen Groß <jgross@xxxxxxxx>
  • From: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Date: Fri, 2 Oct 2020 11:03:55 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=XmvwajEXxGbQ5LSycLATrQubKMcuPuTfBR8p711VpQk=; b=WKsTCc0cJesXVcekAaQJXUTfWHY3MUBt6Af/swam91JlwxyyRuH+hFcK1cJYSNcQhI5JhMjSTK0tgb5MTQtvQ7VPhzQEcgwPakGhbk6pJgsGBUnX4ETF5rKdDCP+M7+GB5xteidwhzXG7gx8NLltQ3XNyfeMJgScTMYVCLZSXuAoUUxC8WjtynLMVmm9ia3AYrZkZqxdWRmRFHT0f33+AIRYzAsipfYMrj7LVgCgrf6LRuX7ql7+rSKilNpmXDJZCX4KrMA1peS1b0V4dfZ64pT1Z6MOOW8D+j3K6TF8ynV/D1jqCqn2BVfGnz867aL9ahnrXTDaJeXMRBijLinstg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Sn2KHq/UQdVcNDHF4XDuZ+kikLT2kSpT60bbFsQHbxr19C5xnbnha6UzhdRqYFeVITT2VCNlhD3kur5vQYZ6FXw27CDBCIJAeabNDN2QqapFVBQ5r9buIEHfV5LxfZHWNvX5p8cf+Zs1p6rngbJRE84hSOEXI653/bNLXaQwttwqASpiBUE/CsBJSrobQXMiourtE6s+6EDM++JMgR03lJr5aFrQ1aZarMkEeToTelO02onxpuvS0L4Bfm6Pjay1Matoms9KY7QWltma2qhmDjKpnokevpcA0nE/UWHEqlRsldI1ShriFO5EfoTkDts747MzMl814NinT4OfsUpVLw==
  • Authentication-results-original: suse.com; dkim=none (message not signed) header.d=none;suse.com; dmarc=none action=none header.from=arm.com;
  • Cc: Jan Beulich <jbeulich@xxxxxxxx>, "open list:X86" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Fri, 02 Oct 2020 11:04:17 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: suse.com; dkim=none (message not signed) header.d=none;suse.com; dmarc=none action=none header.from=arm.com;
  • Thread-topic: [PATCH v3] tools/libs/stat: fix broken build

> On 2 Oct 2020, at 11:44, Jürgen Groß <jgross@xxxxxxxx> wrote:
> On 02.10.20 12:12, Bertrand Marquis wrote:
>> Hi,
>>> On 2 Oct 2020, at 08:25, Jürgen Groß <jgross@xxxxxxxx> wrote:
>>> On 02.10.20 08:59, Jan Beulich wrote:
>>>> On 02.10.2020 08:51, Jürgen Groß wrote:
>>>>> On 02.10.20 08:20, Jan Beulich wrote:
>>>>>> On 02.10.2020 06:50, Jürgen Groß wrote:
>>>>>>> On 01.10.20 18:38, Bertrand Marquis wrote:
>>>>>>>> Hi Juergen,
>>>>>>>>> On 14 Sep 2020, at 11:58, Bertrand Marquis <bertrand.marquis@xxxxxxx> 
>>>>>>>>> wrote:
>>>>>>>>>> On 12 Sep 2020, at 14:08, Juergen Gross <jgross@xxxxxxxx> wrote:
>>>>>>>>>> Making getBridge() static triggered a build error with some gcc 
>>>>>>>>>> versions:
>>>>>>>>>> error: 'strncpy' output may be truncated copying 15 bytes from a 
>>>>>>>>>> string of
>>>>>>>>>> length 255 [-Werror=stringop-truncation]
>>>>>>>>>> Fix that by using a buffer with 256 bytes instead.
>>>>>>>>>> Fixes: 6d0ec053907794 ("tools: split libxenstat into new 
>>>>>>>>>> tools/libs/stat directory")
>>>>>>>>>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>>>>>>>>> Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
>>>>>>>> Sorry i have to come back on this one.
>>>>>>>> I still see an error compiling with Yocto on this one:
>>>>>>>> |     inlined from 'xenstat_collect_networks' at xenstat_linux.c:306:2:
>>>>>>>> | xenstat_linux.c:81:6: error: 'strncpy' output may be truncated 
>>>>>>>> copying 255 bytes from a string of length 255 
>>>>>>>> [-Werror=stringop-truncation]
>>>>>>>> |    81 |      strncpy(result, de->d_name, resultLen);
>>>>>>>> |       |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>>>>> To solve it, I need to define devBridge[257] as devNoBrideg.
>>>>>>> IMHO this is a real compiler error.
>>>>>>> de->d_name is an array of 256 bytes, so doing strncpy() from that to
>>>>>>> another array of 256 bytes with a length of 256 won't truncate anything.
>>>>>> That's a matter of how you look at it, I think: If the original array
>>>>>> doesn't hold a nul-terminated string, the destination array won't
>>>>>> either, yet the common goal of strncpy() is to yield a properly nul-
>>>>>> terminated string. IOW the warning may be since the standard even has
>>>>>> a specific foot note to point out this possible pitfall.
>>>>> If the source doesn't hold a nul-terminated string there will still be
>>>>> 256 bytes copied, so there is no truncation done during strncpy().
>>>>> In fact there is no way to use strncpy() in a safe way on a fixed sized
>>>>> source array with the above semantics: either the target is larger than
>>>>> the source and length is at least sizeof(source) + 1, resulting in a
>>>>> possible read beyond the end of source, or the target is the same length
>>>>> leading to the error.
>>>> I agree with all of what you say, but I can also see why said foot note
>>>> alone may have motivated the emission of the warning.
>>> The motivation can be explained, yes, but it is wrong. strncpy() is not
>>> limited to source arrays of unknown length. So this warning is making
>>> strncpy() unusable for fixed sized source strings and -Werror. And that
>>> is nothing a compiler should be allowed to do, hence a compiler bug.
>> I do agree that in this case the compiler is doing to much.
> It is plain wrong here. Rendering a Posix defined function unusable for
> a completely legal use case is in no way a matter of taste or of "doing
> to much". It is a bug.


>> We could also choose to turn off the warning either using pragma (which
>> i really do not like) or by adding a cflag for this specific file (but this 
>> might
>> hit us later in other places).
>> All in all this currently makes Xen master and staging not possible to
>> compile with Yocto so we need to find a solution as this will also
>> come in any distribution using a new compiler,
> A variant you didn't mention would be open coding of strncpy() (or
> having a related inline function in a common header). This route would
> be the one I'd prefer in case the compiler guys insist on the behavior
> being fine.

True this possible, even though I do not like to modify the code that deeply
for one specific compiler.

> You didn't tell us which compiler is being used and whether it really is
> up to date. A workaround might be to set EXTRA_CFLAGS_XEN_TOOLS to
> "-Wno-stringop-truncation" for the build.

That’s what i meant by “adding a cflag”, as you suggest we could also
make it global (and not limit it to this file).

The compiler I am using is the one from Yocto Gatesgarth (release coming in
october): gcc version 10.2.0 (released in july 2020).





Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.