1. Patchew
  2. Seabios
  3. View series

[SeaBIOS] [PATCH] memmap: Fix gcc out-of-bounds warning

Kevin O'Connor posted 1 patch 2 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/seabios tags/patchew/20211218181056.3848842-1-kevin@koconnor.net
src/memmap.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[SeaBIOS] [PATCH] memmap: Fix gcc out-of-bounds warning
Posted by Kevin O'Connor 2 years, 4 months ago 
Use a different definition for the linker script symbol to avoid a gcc
warning.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
---
 src/memmap.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/memmap.h b/src/memmap.h
index 22bd4bc..32ca265 100644
--- a/src/memmap.h
+++ b/src/memmap.h
@@ -15,7 +15,7 @@ static inline void *memremap(u32 addr, u32 len) {
 }
 
 // Return the value of a linker script symbol (see scripts/layoutrom.py)
-#define SYMBOL(SYM) ({ extern char SYM; (u32)&SYM; })
+#define SYMBOL(SYM) ({ extern char SYM[]; (u32)SYM; })
 #define VSYMBOL(SYM) ((void*)SYMBOL(SYM))
 
 #endif // memmap.h
-- 
2.31.1

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH] memmap: Fix gcc out-of-bounds warning
Posted by Paul Menzel 2 years, 4 months ago 
Dear Kevin,


Am 18.12.21 um 19:10 schrieb Kevin O'Connor:
> Use a different definition for the linker script symbol to avoid a gcc
> warning.

What GCC version did you use, and which warning is it exactly?

> Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
> ---
>   src/memmap.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/memmap.h b/src/memmap.h
> index 22bd4bc..32ca265 100644
> --- a/src/memmap.h
> +++ b/src/memmap.h
> @@ -15,7 +15,7 @@ static inline void *memremap(u32 addr, u32 len) {
>   }
>   
>   // Return the value of a linker script symbol (see scripts/layoutrom.py)
> -#define SYMBOL(SYM) ({ extern char SYM; (u32)&SYM; })
> +#define SYMBOL(SYM) ({ extern char SYM[]; (u32)SYM; })
>   #define VSYMBOL(SYM) ((void*)SYMBOL(SYM))
>   
>   #endif // memmap.h

gcc (Debian 11.2.0-13) 11.2.0 still shows a lot of:

       Compile checking out/src/fw/smm.o
     In file included from src/fw/smm.c:18:
     src/fw/smm.c: In function 'smm_save_and_copy':
     src/string.h:23:16: warning: '__builtin_memcpy' offset [0, 511] is 
out of the bounds [0, 0] [-Warray-bounds]
        23 | #define memcpy __builtin_memcpy
     src/fw/smm.c:148:5: note: in expansion of macro 'memcpy'
       148 |     memcpy(&smm->cpu, &initsmm->cpu, sizeof(smm->cpu));
           |     ^~~~~~


Kind regards,

Paul
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH] memmap: Fix gcc out-of-bounds warning
Posted by Kevin O'Connor 2 years, 4 months ago 
On Sat, Dec 18, 2021 at 09:13:57PM +0100, Paul Menzel wrote:
> Dear Kevin,
> 
> 
> Am 18.12.21 um 19:10 schrieb Kevin O'Connor:
> > Use a different definition for the linker script symbol to avoid a gcc
> > warning.
> 
> What GCC version did you use, and which warning is it exactly?

$ gcc --version
gcc (GCC) 11.2.1 20210728 (Red Hat 11.2.1-1)


src/post.c: In function 'reloc_preinit':
src/post.c:248:34: warning: array subscript 'u32 {aka unsigned int}[0]' is partly outside array bounds of 'char[1]' [-Warray-bounds]
  248 |         *((u32*)(dest + *reloc)) += delta;
      |                                  ^~
In file included from src/biosvar.h:11,
                 from src/post.c:8:
src/post.c:278:26: note: while referencing 'code32flat_start'
  278 |     updateRelocs(VSYMBOL(code32flat_start), VSYMBOL(_reloc_init_start)
      |                          ^~~~~~~~~~~~~~~~
src/memmap.h:18:36: note: in definition of macro 'SYMBOL'
   18 | #define SYMBOL(SYM) ({ extern char SYM; (u32)&SYM; })
      |                                    ^~~


> 
> > Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
> > ---
> >   src/memmap.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/memmap.h b/src/memmap.h
> > index 22bd4bc..32ca265 100644
> > --- a/src/memmap.h
> > +++ b/src/memmap.h
> > @@ -15,7 +15,7 @@ static inline void *memremap(u32 addr, u32 len) {
> >   }
> >   // Return the value of a linker script symbol (see scripts/layoutrom.py)
> > -#define SYMBOL(SYM) ({ extern char SYM; (u32)&SYM; })
> > +#define SYMBOL(SYM) ({ extern char SYM[]; (u32)SYM; })
> >   #define VSYMBOL(SYM) ((void*)SYMBOL(SYM))
> >   #endif // memmap.h
> 
> gcc (Debian 11.2.0-13) 11.2.0 still shows a lot of:
> 
>       Compile checking out/src/fw/smm.o
>     In file included from src/fw/smm.c:18:
>     src/fw/smm.c: In function 'smm_save_and_copy':
>     src/string.h:23:16: warning: '__builtin_memcpy' offset [0, 511] is out
> of the bounds [0, 0] [-Warray-bounds]
>        23 | #define memcpy __builtin_memcpy
>     src/fw/smm.c:148:5: note: in expansion of macro 'memcpy'
>       148 |     memcpy(&smm->cpu, &initsmm->cpu, sizeof(smm->cpu));
>           |     ^~~~~~
> 

Yes - I see that as well in smm.c.  Alas, I don't have a fix for it.
It seems to me that gcc is producing bogus warnings here.  It looks
like anything of the form "memcpy((void*)0x1234, p, n)" where n is
greater than 8 produces this warning.  It's a requirement to memcpy to
a physical memory address.  Disabling the warning would require adding
both "-Wno-array-bounds -Wno-stringop-overflow" to the build.

Maybe someone else has an idea on how to suppress this warning.

-Kevin
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH] memmap: Fix gcc out-of-bounds warning
Posted by Paul Menzel 2 years, 4 months ago 
Dear Kevin,


Am 18.12.21 um 22:40 schrieb Kevin O'Connor:
> On Sat, Dec 18, 2021 at 09:13:57PM +0100, Paul Menzel wrote:

>> Am 18.12.21 um 19:10 schrieb Kevin O'Connor:
>>> Use a different definition for the linker script symbol to avoid a gcc
>>> warning.
>>
>> What GCC version did you use, and which warning is it exactly?
> 
> $ gcc --version
> gcc (GCC) 11.2.1 20210728 (Red Hat 11.2.1-1)
> 
> 
> src/post.c: In function 'reloc_preinit':
> src/post.c:248:34: warning: array subscript 'u32 {aka unsigned int}[0]' is partly outside array bounds of 'char[1]' [-Warray-bounds]
>    248 |         *((u32*)(dest + *reloc)) += delta;
>        |                                  ^~
> In file included from src/biosvar.h:11,
>                   from src/post.c:8:
> src/post.c:278:26: note: while referencing 'code32flat_start'
>    278 |     updateRelocs(VSYMBOL(code32flat_start), VSYMBOL(_reloc_init_start)
>        |                          ^~~~~~~~~~~~~~~~
> src/memmap.h:18:36: note: in definition of macro 'SYMBOL'
>     18 | #define SYMBOL(SYM) ({ extern char SYM; (u32)&SYM; })
>        |                                    ^~~


Thank you. I see that too, and your patch fixes it.

Tested-by: Paul Menzel <pmenzel@molgen.mpg.de>

>>> Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
>>> ---
>>>    src/memmap.h | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/memmap.h b/src/memmap.h
>>> index 22bd4bc..32ca265 100644
>>> --- a/src/memmap.h
>>> +++ b/src/memmap.h
>>> @@ -15,7 +15,7 @@ static inline void *memremap(u32 addr, u32 len) {
>>>    }
>>>    // Return the value of a linker script symbol (see scripts/layoutrom.py)
>>> -#define SYMBOL(SYM) ({ extern char SYM; (u32)&SYM; })
>>> +#define SYMBOL(SYM) ({ extern char SYM[]; (u32)SYM; })
>>>    #define VSYMBOL(SYM) ((void*)SYMBOL(SYM))
>>>    #endif // memmap.h
>>
>> gcc (Debian 11.2.0-13) 11.2.0 still shows a lot of:
>>
>>        Compile checking out/src/fw/smm.o
>>      In file included from src/fw/smm.c:18:
>>      src/fw/smm.c: In function 'smm_save_and_copy':
>>      src/string.h:23:16: warning: '__builtin_memcpy' offset [0, 511] is out of the bounds [0, 0] [-Warray-bounds]
>>         23 | #define memcpy __builtin_memcpy
>>      src/fw/smm.c:148:5: note: in expansion of macro 'memcpy'
>>        148 |     memcpy(&smm->cpu, &initsmm->cpu, sizeof(smm->cpu));
>>            |     ^~~~~~
>>
> 
> Yes - I see that as well in smm.c.  Alas, I don't have a fix for it.
> It seems to me that gcc is producing bogus warnings here.  It looks
> like anything of the form "memcpy((void*)0x1234, p, n)" where n is
> greater than 8 produces this warning.  It's a requirement to memcpy to
> a physical memory address.  Disabling the warning would require adding
> both "-Wno-array-bounds -Wno-stringop-overflow" to the build.
> 
> Maybe someone else has an idea on how to suppress this warning.

I asked the GCC folks [1].


Kind regards,

Paul


[1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103768
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.