Re: [PATCH] sparc: mdesc: Fix compile error seen with gcc 11.x

[Anatoly Pugachev]

On Tue, Sep 14, 2021 at 3:54 AM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxx>
> >> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> >> ---
> >> 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.
>
> Guenter
>
> ---
> Test code I had used:
>
> #include <stddef.h>
> #include <stdio.h>
>
> typedef unsigned int u32;
>
> struct mdesc_hdr {
>          u32     version; /* Transport version */
>          u32     node_sz; /* node block size */
>          u32     name_sz; /* name block size */
>          u32     data_sz; /* data block size */
> } __attribute__((aligned(16)));
>
> struct mdesc_hdr2 {
>          u32     version; /* Transport version */
>          u32     node_sz; /* node block size */
>          u32     name_sz; /* name block size */
>          u32     data_sz; /* data block size */
>          char    data[0];
> } __attribute__((aligned(16)));
>
> int main()
> {
>         printf("%ld %ld\n", sizeof(struct mdesc_hdr), sizeof(struct mdesc_hdr2));
>
>         return 0;
> }

used the code above on my sparc64 installation:

copy-paste code to 123.c file

$ gcc-11 123.c
$ ./a.out
16 16
$ uname -a
Linux ttip 5.15.0-rc1 #273 SMP Mon Sep 13 12:47:14 MSK 2021 sparc64 GNU/Linux