Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: Guenter Roeck <groeck7@gmail.com>
Subject: Re: [PATCH] sparc: mdesc: Fix compile error seen with gcc 11.x
To:     David Laight <David.Laight@ACULAB.COM>,
        Sam Ravnborg <sam@ravnborg.org>
Cc:     "David S . Miller" <davem@davemloft.net>,
        "sparclinux@vger.kernel.org" <sparclinux@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Arnd Bergmann <arnd@kernel.org>
References: <20210913163712.922188-1-linux@roeck-us.net>
 <YT+SPIAl0IdWOAn/@ravnborg.org>
 <d0a4b46a-2f0e-f6a2-1342-777e738d9525@roeck-us.net>
 <2d8f45425f024fd9a3d91a4b4a1304cf@AcuMS.aculab.com>
From:   Guenter Roeck <linux@roeck-us.net>
Message-ID: <3b069cde-6f15-1df1-fbed-e8d94d0ef173@roeck-us.net>
Date:   Tue, 14 Sep 2021 07:24:56 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
MIME-Version: 1.0
In-Reply-To: <2d8f45425f024fd9a3d91a4b4a1304cf@AcuMS.aculab.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 9/14/21 7:17 AM, David Laight wrote:
> 
> 
>> -----Original Message-----
>> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
>> Sent: 13 September 2021 19:53
>> To: Sam Ravnborg <sam@ravnborg.org>
>> Cc: David S . Miller <davem@davemloft.net>; sparclinux@vger.kernel.org; linux-kernel@vger.kernel.org;
>> Arnd Bergmann <arnd@kernel.org>
>> Subject: Re: [PATCH] sparc: mdesc: Fix compile error seen with gcc 11.x
>>
>> On 9/13/21 11:02 AM, Sam Ravnborg wrote:
>>> Hi Guenter,
>>>
>>> On Mon, Sep 13, 2021 at 09:37:12AM -0700, Guenter Roeck wrote:
>>>> sparc64 images fail to compile with gcc 11.x, reporting the following
>>>> errors.
>>>>
>>>> arch/sparc/kernel/mdesc.c:647:22: error:
>>>> 	'strcmp' reading 1 or more bytes from a region of size 0
>>>> arch/sparc/kernel/mdesc.c:692:22: error:
>>>> 	'strcmp' reading 1 or more bytes from a region of size 0
>>>> arch/sparc/kernel/mdesc.c:719:21:
>>>> 	error: 'strcmp' reading 1 or more bytes from a region of size 0
>>>>
>>>> The underlying problem is that node_block() returns a pointer beyond
>>>> the end of struct mdesc_hdr. gcc 11.x detects that and reports the error.
>>>> Adding an additional zero-length field to struct mdesc_hdr and pointing
>>>> to that field fixes the problem.
>>>>
>>>> Cc: Arnd Bergmann <arnd@kernel.org>
>>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>>> ---
>>>> My apologies if a similar patch was submitted already; I was unable to find it.
>>>> I did find the following patch:
>>>>       https://git.busybox.net/buildroot/commit/?id=6e1106b4a9aee25d1556310d5cd1cb6dde2e6e3f
>>>> but I failed to find it in patchwork or on lore.kernel.org, and it
>>>> seems to be more expensive than the solution suggested here.
>>>>
>>>>    arch/sparc/kernel/mdesc.c | 3 ++-
>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/sparc/kernel/mdesc.c b/arch/sparc/kernel/mdesc.c
>>>> index 8e645ddac58e..c67bdcc23727 100644
>>>> --- a/arch/sparc/kernel/mdesc.c
>>>> +++ b/arch/sparc/kernel/mdesc.c
>>>> @@ -39,6 +39,7 @@ struct mdesc_hdr {
>>>>    	u32	node_sz; /* node block size */
>>>>    	u32	name_sz; /* name block size */
>>>>    	u32	data_sz; /* data block size */
>>>> +	char	data[0];
>>>>    } __attribute__((aligned(16)));
>>>
>>> I do not think this will works.
>>> See following comment:
>>>    * mdesc_hdr and mdesc_elem describe the layout of the data structure
>>>    * we get from the Hypervisor.
>>>
>>> With the above change you increased the size from 16 to 32 bytes,
>>> and any code using sizeof(struct mdesc_hdr) will now point too far in
>>> memory for the second and subsequent entries.
>>>
>>> I did not take any closer look, but this was from a quick analysis.
>>>
>>
>> Sorry, I didn't realize that a field of size 0 increases the structure size
>> on sparc. I had checked the size of the old and the new structure with gcc
>> on x86_64 and didn't see a field size increase.
> 
> clang output doesn't change:
> 
> https://godbolt.org/z/bTeeq19j1
> 
> gcc ought to generate the same size.
> 
> It ought to be 'char data[];' though.
> 

I am never sure if [] or [0] is "correct". Anyway, is there agreement that this
is an acceptable solution ? I'll be happy to resend if that is the case.

Guenter