[Skiboot] [PATCH v3] Fix array-bound compilation warnings
Abhishek SIngh Tomar
abhishek at linux.ibm.com
Tue Jan 25 17:11:17 AEDT 2022
hello Nick
I got your concern
> The volatile just seems a bit clunky but I'm not sure what a better
> option is. Declaring the addresses in the linker script maybe gives us
> a better result including bounds checking for the memory? Will take a
> little more work though, e.g.:
We can check with declaring address in linker script.
I also like to share one of workaround used in seabios for similar issue.
https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/HLK3BHP2T3FN6FZ46BIPIK3VD5FOU74Z/
Can we go with something like this.
Thanks,
Regards
Abhishek Singh Tomar
On Tue, Jan 25, 2022 at 02:21:53PM +1000, Nicholas Piggin wrote:
> Excerpts from Abhishek Singh Tomar's message of January 24, 2022 11:56 pm:
> > Resolves : the warray bounds warning during compilation
> >
> > /build/libc/include/string.h:34:16: warning: '__builtin_memset' offset [0, 2097151] is out of the bounds [0, 0] [-Warray-bounds]
> > 34 | #define memset __builtin_memset
> > hw/fsp/fsp.c:1855:9: note: in expansion of macro 'memset'
> > 1855 | memset(fsp_tce_table, 0, PSI_TCE_TABLE_SIZE);
> >
> > use volatile pointer to avoid optimization introduced with gcc-11 on constant
> > address assignment to pointer.
>
> Thanks for persisting with this. I'm not sure it's quite the right way
> to fix it. It might be something to ask on the gcc at gcc.gnu.org mailing
> list.
>
> C is supposed to be usable for embedded systems where you expect to be
> working with a lot of data that's outside the visibility of the
> compiler. Yet we don't want to disable array bounds checking entirely.
>
> The volatile just seems a bit clunky but I'm not sure what a better
> option is. Declaring the addresses in the linker script maybe gives us
> a better result including bounds checking for the memory? Will take a
> little more work though, e.g.:
>
> Thanks,
> Nick
>
> ---
>
> diff --git a/core/fast-reboot.c b/core/fast-reboot.c
> index fedfa88cc..3fec6ab8a 100644
> --- a/core/fast-reboot.c
> +++ b/core/fast-reboot.c
> @@ -304,6 +304,9 @@ static void cleanup_cpu_state(void)
> /* Entry from asm after a fast reset */
> void __noreturn fast_reboot_entry(void);
>
> +extern char kernel_load_base[KERNEL_LOAD_SIZE];
> +extern char initramfs_load_base[INITRAMFS_LOAD_SIZE];
> +
> void __noreturn fast_reboot_entry(void)
> {
> struct cpu_thread *cpu = this_cpu();
> @@ -425,8 +428,8 @@ void __noreturn fast_reboot_entry(void)
> * Mambo may have embedded payload here, so don't clear
> * it at all.
> */
> - memset(KERNEL_LOAD_BASE, 0, KERNEL_LOAD_SIZE);
> - memset(INITRAMFS_LOAD_BASE, 0, INITRAMFS_LOAD_SIZE);
> + memset(kernel_load_base, 0, KERNEL_LOAD_SIZE);
> + memset(initramfs_load_base, 0, INITRAMFS_LOAD_SIZE);
> }
>
> /* Start preloading kernel and ramdisk */
> diff --git a/skiboot.lds.S b/skiboot.lds.S
> index 058848fa9..92726d7e1 100644
> --- a/skiboot.lds.S
> +++ b/skiboot.lds.S
> @@ -251,6 +251,9 @@ SECTIONS
>
> DEBUG_SECTIONS
>
> + kernel_load_base = 0x20000000;
> + initramfs_load_base = 0x20000000 + 0x08000000;
> +
> /* Discards */
> /DISCARD/ : {
> *(.note.GNU-stack)
>
More information about the Skiboot
mailing list