From e1ad57e7ef6e17fb6f55bf70fff61bfd93703b78 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 18 Jul 2023 07:24:01 -0600 Subject: [PATCH] binman: Correct handling of zero bss size Fix the check for the __bss_size symbol, since it may be 0. Unfortunately there was no test coverage for this. Signed-off-by: Simon Glass --- tools/binman/elf_test.py | 5 +++++ tools/binman/etype/u_boot_spl_bss_pad.py | 2 +- tools/binman/etype/u_boot_tpl_bss_pad.py | 2 +- tools/binman/etype/u_boot_vpl_bss_pad.py | 2 +- tools/binman/ftest.py | 12 ++++++++++++ tools/binman/test/285_spl_expand.dts | 13 +++++++++++++ tools/binman/test/Makefile | 5 ++++- tools/binman/test/bss_data_zero.c | 16 ++++++++++++++++ tools/binman/test/bss_data_zero.lds | 15 +++++++++++++++ tools/binman/test/embed_data.lds | 1 + 10 files changed, 69 insertions(+), 4 deletions(-) create mode 100644 tools/binman/test/285_spl_expand.dts create mode 100644 tools/binman/test/bss_data_zero.c create mode 100644 tools/binman/test/bss_data_zero.lds diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index 2fb3f6f28f..cc95b424b3 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -371,6 +371,11 @@ class TestElf(unittest.TestCase): elf.GetSymbolOffset(fname, 'embed') self.assertIn('__image_copy_start', str(e.exception)) + def test_get_symbol_address(self): + fname = self.ElfTestFile('embed_data') + addr = elf.GetSymbolAddress(fname, 'region_size') + self.assertEqual(0, addr) + if __name__ == '__main__': unittest.main() diff --git a/tools/binman/etype/u_boot_spl_bss_pad.py b/tools/binman/etype/u_boot_spl_bss_pad.py index 1ffeb3911f..4af4045d37 100644 --- a/tools/binman/etype/u_boot_spl_bss_pad.py +++ b/tools/binman/etype/u_boot_spl_bss_pad.py @@ -38,7 +38,7 @@ class Entry_u_boot_spl_bss_pad(Entry_blob): def ObtainContents(self): fname = tools.get_input_filename('spl/u-boot-spl') bss_size = elf.GetSymbolAddress(fname, '__bss_size') - if not bss_size: + if bss_size is None: self.Raise('Expected __bss_size symbol in spl/u-boot-spl') self.SetContents(tools.get_bytes(0, bss_size)) return True diff --git a/tools/binman/etype/u_boot_tpl_bss_pad.py b/tools/binman/etype/u_boot_tpl_bss_pad.py index 29c6a95412..46d2cd58f7 100644 --- a/tools/binman/etype/u_boot_tpl_bss_pad.py +++ b/tools/binman/etype/u_boot_tpl_bss_pad.py @@ -38,7 +38,7 @@ class Entry_u_boot_tpl_bss_pad(Entry_blob): def ObtainContents(self): fname = tools.get_input_filename('tpl/u-boot-tpl') bss_size = elf.GetSymbolAddress(fname, '__bss_size') - if not bss_size: + if bss_size is None: self.Raise('Expected __bss_size symbol in tpl/u-boot-tpl') self.SetContents(tools.get_bytes(0, bss_size)) return True diff --git a/tools/binman/etype/u_boot_vpl_bss_pad.py b/tools/binman/etype/u_boot_vpl_bss_pad.py index bba38ccf9e..12b286a719 100644 --- a/tools/binman/etype/u_boot_vpl_bss_pad.py +++ b/tools/binman/etype/u_boot_vpl_bss_pad.py @@ -38,7 +38,7 @@ class Entry_u_boot_vpl_bss_pad(Entry_blob): def ObtainContents(self): fname = tools.get_input_filename('vpl/u-boot-vpl') bss_size = elf.GetSymbolAddress(fname, '__bss_size') - if not bss_size: + if bss_size is None: self.Raise('Expected __bss_size symbol in vpl/u-boot-vpl') self.SetContents(tools.get_bytes(0, bss_size)) return True diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index ed1940ed9f..dc9a95d341 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6773,6 +6773,18 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertEqual(U_BOOT_NODTB_DATA, data[-len(U_BOOT_NODTB_DATA):]) fit_data = data[len(U_BOOT_DATA):-len(U_BOOT_NODTB_DATA)] + def testSplEmptyBss(self): + """Test an expanded SPL with a zero-size BSS""" + # ELF file with a '__bss_size' symbol + self._SetupSplElf(src_fname='bss_data_zero') + + entry_args = { + 'spl-bss-pad': 'y', + 'spl-dtb': 'y', + } + data = self._DoReadFileDtb('285_spl_expand.dts', + use_expanded=True, entry_args=entry_args)[0] + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/285_spl_expand.dts b/tools/binman/test/285_spl_expand.dts new file mode 100644 index 0000000000..9c88ccb287 --- /dev/null +++ b/tools/binman/test/285_spl_expand.dts @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot-spl { + }; + }; +}; diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile index cd66a3038b..4d152eee9c 100644 --- a/tools/binman/test/Makefile +++ b/tools/binman/test/Makefile @@ -32,7 +32,7 @@ LDS_BINMAN_EMBED := -T $(SRC)u_boot_binman_embed.lds LDS_EFL_SECTIONS := -T $(SRC)elf_sections.lds LDS_BLOB := -T $(SRC)blob_syms.lds -TARGETS = u_boot_ucode_ptr u_boot_no_ucode_ptr bss_data \ +TARGETS = u_boot_ucode_ptr u_boot_no_ucode_ptr bss_data bss_data_zero \ u_boot_binman_syms u_boot_binman_syms.bin u_boot_binman_syms_bad \ u_boot_binman_syms_size u_boot_binman_syms_x86 embed_data \ u_boot_binman_embed u_boot_binman_embed_sm elf_sections blob_syms.bin @@ -48,6 +48,9 @@ u_boot_ucode_ptr: u_boot_ucode_ptr.c bss_data: CFLAGS += $(SRC)bss_data.lds bss_data: bss_data.c +bss_data_zero: CFLAGS += $(SRC)bss_data_zero.lds +bss_data_zero: bss_data_zero.c + embed_data: CFLAGS += $(SRC)embed_data.lds embed_data: embed_data.c diff --git a/tools/binman/test/bss_data_zero.c b/tools/binman/test/bss_data_zero.c new file mode 100644 index 0000000000..7047a3bb01 --- /dev/null +++ b/tools/binman/test/bss_data_zero.c @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2016 Google, Inc + * + * Simple program to create a bss_data region so the symbol can be read + * by binutils. This is used by binman tests. + */ + +int bss_data[10]; + +int main(void) +{ + bss_data[2] = 2; + + return 0; +} diff --git a/tools/binman/test/bss_data_zero.lds b/tools/binman/test/bss_data_zero.lds new file mode 100644 index 0000000000..8fa0210a8f --- /dev/null +++ b/tools/binman/test/bss_data_zero.lds @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (c) 2016 Google, Inc + */ + +OUTPUT_FORMAT("elf32-i386", "elf32-i386", "elf32-i386") +OUTPUT_ARCH(i386) +ENTRY(_start) + +SECTIONS +{ + . = 0xfffffdf0; + _start = .; + __bss_size = 0; +} diff --git a/tools/binman/test/embed_data.lds b/tools/binman/test/embed_data.lds index 908bf66c29..d416cb2111 100644 --- a/tools/binman/test/embed_data.lds +++ b/tools/binman/test/embed_data.lds @@ -17,6 +17,7 @@ SECTIONS embed_start = .; *(.embed*) embed_end = .; + region_size = 0; . = ALIGN(32); *(.data*) } -- 2.39.5