[U-Boot] [PATCH 2/2] armv8: add asserts of sizes to u-boot-spl.lds

[Masahiro Yamada]

2016-12-26 23:20 GMT+09:00 Oded Gabbay <oded.gabbay at gmail.com>:
> This patch copies from arm u-boot-spl.lds some asserts that check that the
> size of the SPL image and BSS doesn't violate their max size.
>
> Signed-off-by: Oded Gabbay <oded.gabbay at gmail.com>
> Cc: Albert Aribaud <albert.u.boot at aribaud.net>
> ---
>  arch/arm/cpu/armv8/u-boot-spl.lds | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot-spl.lds
> index e7799cc..0876a94 100644
> --- a/arch/arm/cpu/armv8/u-boot-spl.lds
> +++ b/arch/arm/cpu/armv8/u-boot-spl.lds
> @@ -78,3 +78,22 @@ SECTIONS
>         /DISCARD/ : { *(.interp*) }
>         /DISCARD/ : { *(.gnu*) }
>  }
> +
> +#if defined(CONFIG_SPL_MAX_SIZE)
> +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
> +       "SPL image too big");
> +#endif
> +
> +#if defined(CONFIG_SPL_BSS_MAX_SIZE) && defined(CONFIG_SPL_MAX_FOOTPRINT)
> +ASSERT(0, "CONFIG_SPL_BSS_MAX_SIZE and CONFIG_SPL_MAX_FOOTPRINT must not be defined together");
> +#endif
> +
> +#if defined(CONFIG_SPL_BSS_MAX_SIZE)
> +ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS_MAX_SIZE), \
> +       "SPL image BSS too big");
> +#endif
> +
> +#if defined(CONFIG_SPL_MAX_FOOTPRINT)
> +ASSERT(__bss_end - _start < (CONFIG_SPL_MAX_FOOTPRINT), \
> +       "SPL image plus BSS too big");
> +#endif
> --
> 2.7.4
>

This patch is wrong in multiple ways.



See the top lines of this file.

MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,
         LENGTH = CONFIG_SPL_MAX_SIZE }
MEMORY { .sdram : ORIGIN = CONFIG_SPL_BSS_START_ADDR,
         LENGTH = CONFIG_SPL_BSS_MAX_SIZE }

This means CONFIG_SPL_BSS_MAX_SIZE must always be defined.




Your code

> +#if defined(CONFIG_SPL_BSS_MAX_SIZE) && defined(CONFIG_SPL_MAX_FOOTPRINT)
> +ASSERT(0, "CONFIG_SPL_BSS_MAX_SIZE and CONFIG_SPL_MAX_FOOTPRINT must not be defined together");
> +#endif

... means CONFIG_SPL_BSS_MAX_FOOTPRINT can never be defined.


As a result,

> +#if defined(CONFIG_SPL_BSS_MAX_SIZE)
> +ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS_MAX_SIZE), \
> +       "SPL image BSS too big");
> +#endif

the ifdef is always true, so meaningless.

> +ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS_MAX_SIZE), \
> +       "SPL image BSS too big");

is already size-checked by the following:

MEMORY { .sdram : ORIGIN = CONFIG_SPL_BSS_START_ADDR,
         LENGTH = CONFIG_SPL_BSS_MAX_SIZE }


so, meaningless.


> +#if defined(CONFIG_SPL_MAX_FOOTPRINT)
> +ASSERT(__bss_end - _start < (CONFIG_SPL_MAX_FOOTPRINT), \
> +       "SPL image plus BSS too big");
> +#endif

This code is never enabled.



-- 
Best Regards
Masahiro Yamada