[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