Re: [PATCH 3/6] commands/i386/pc/sendkey: Fix "writing 1 byte into a region of size 0" build error

[Daniel Kiper]

On Mon, Mar 14, 2022 at 12:16:16PM +0800, Michael Chang wrote:
> On Fri, Mar 11, 2022 at 12:35:57AM +0100, Daniel Kiper wrote:
> > Latest GCC may complain in that way:
> >
> >   commands/i386/pc/sendkey.c: In function ‘grub_sendkey_postboot’:
> >   commands/i386/pc/sendkey.c:223:21: error: writing 1 byte into a region of 
> > size 0 [-Werror=stringop-overflow=]
> >     223 |   *((char *) 0x41a) = 0x1e;
> >         |   ~~~~~~~~~~~~~~~~~~^~~~~~
> >
> > The volatile keyword addition helps and additionally assures us the
> > compiler will not optimize out fixed assignments.
> >
> > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> > ---
> >  grub-core/commands/i386/pc/sendkey.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/grub-core/commands/i386/pc/sendkey.c 
> > b/grub-core/commands/i386/pc/sendkey.c
> > index 26d9acd3d..ab4bca9e9 100644
> > --- a/grub-core/commands/i386/pc/sendkey.c
> > +++ b/grub-core/commands/i386/pc/sendkey.c
> > @@ -220,8 +220,8 @@ grub_sendkey_postboot (void)
> >
> >    *flags = oldflags;
> >
> > -  *((char *) 0x41a) = 0x1e;
> > -  *((char *) 0x41c) = 0x1e;
> > +  *((volatile char *) 0x41a) = 0x1e;
> > +  *((volatile char *) 0x41c) = 0x1e;
> >
> >    return GRUB_ERR_NONE;
> >  }
> > @@ -236,8 +236,8 @@ grub_sendkey_preboot (int noret __attribute__ 
> > ((unused)))
> >    oldflags = *flags;
> >
> >    /* Set the sendkey.  */
> > -  *((char *) 0x41a) = 0x1e;
> > -  *((char *) 0x41c) = keylen + 0x1e;
> > +  *((volatile char *) 0x41a) = 0x1e;
> > +  *((volatile char *) 0x41c) = keylen + 0x1e;
> >    grub_memcpy ((char *) 0x41e, sendkey, 0x20);
> >
> >    /* Transform "any ctrl" to "right ctrl" flag.  */
> > --
> > 2.11.0
>
> Unfortunately the volatile keyword doesn't help on a more recent gcc-12
> build [1]. With that -Warray-bounds warning [2] is emitted this time
> than -Wstringop-overflow.

Ugh... Though I have taken the patch because it at least fix the problem
partially.

> I believe this is caused by this gcc regression in which hardwired
> literal address is treated as NULL plus an offset, while there's no good
> way to tell that literal address being an invalid accesses at non-zero
> offsets to null pointers or not:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
>
> Before upstream comes up with a solution or introducing new annotation
> for literal pointer. For the time being their recommedation is to
> suppress the warning with #pragma or use volatile pointer which appears
> to not work for gcc-12.
>
> Instead of inserting #pragma all over the places to avoid the diagnose
> of Warray-bounds. I think we can try to port absolute_pointer [3] and
> RELOC_HIDE [4] used by the linux kernel to disconnect a pointer using
> literal address from its original object hence gcc won't make
> assumptions on the boundary while doing pointer arithmetic. This sounds
> a better way to go as it can work across gcc versions.
>
> I am halfway in preparing the patch to wrap literal pointers in
> absolute_pointer. Before posting it here, I'd like to know do we have
> any other prefernece like just disabling it via #pragma as that sounds
> more like a norm?

I think absolute_pointer() looks like a better solution.

Thank you for taking a stab at this.

Daniel