On 08/02/2022 21:49, Simon Glass wrote:
> Add a function which reads the segments and the entry address.
>
> Also fix a comment nit in the tests while we are here.
>
> Signed-off-by: Simon Glass <s...@chromium.org>
> ---
>
> tools/binman/elf.py | 37 +++++++++++++++++++++++++++++++++++++
> tools/binman/elf_test.py | 31 +++++++++++++++++++++++++++++--
> 2 files changed, 66 insertions(+), 2 deletions(-)
>
> diff --git a/tools/binman/elf.py b/tools/binman/elf.py
> index de2bb4651f..2b83ac1876 100644
> --- a/tools/binman/elf.py
> +++ b/tools/binman/elf.py
> @@ -20,6 +20,7 @@ from patman import tout
> ELF_TOOLS = True
> try:
> from elftools.elf.elffile import ELFFile
> + from elftools.elf.elffile import ELFError
> from elftools.elf.sections import SymbolTableSection
> except: # pragma: no cover
> ELF_TOOLS = False
> @@ -369,3 +370,39 @@ def UpdateFile(infile, outfile, start_sym, end_sym,
> insert):
> newdata += data[syms[end_sym].offset:]
> tools.WriteFile(outfile, newdata)
> tout.Info('Written to offset %#x' % syms[start_sym].offset)
> +
> +def read_segments(data):
> + """Read segments from an ELF file
> +
> + Args:
> + data (bytes): Contents of file
> +
> + Returns:
> + tuple:
> + list of segments, each:
> + int: Segment number (0 = first)
> + int: Start address of segment in memory
> + bytes: Contents of segment
> + int: entry address for image
> +
> + Raises:
> + ValueError: elftools is not available
... or input data is not a correct ELF file?
> + """
> + if not ELF_TOOLS:
> + raise ValueError('Python elftools package is not available')
I see something like ModuleNotFoundError("No module named 'elftools'")
when I try to import an unavailable module, so maybe this could match that.
> + with io.BytesIO(data) as inf:
> + try:
> + elf = ELFFile(inf)
> + except ELFError as err:
> + raise ValueError(err)
Could also be: raise ValueError("Not an ELF file") from err
But I guess you want err's message here to match on it in tests. (It's
also possible but slightly inconvenient with __cause__ when using
raise-from)
> + entry = elf.header['e_entry']
> + segments = []
> + for i in range(elf.num_segments()):
> + segment = elf.get_segment(i)
> + if segment['p_type'] != 'PT_LOAD' or not segment['p_memsz']:
I can't say I fully understand ELF details, is it obvious in context
that a function named read_segments() would only return these segments,
or should the name be explicit about it e.g. read_loadable_segments()?
> + skipped = 1 # To make code-coverage see this line
> + continue
> + start = segment['p_offset']
> + rend = start + segment['p_filesz']
> + segments.append((i, segment['p_paddr'], data[start:rend]))
> + return segments, entry
> diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py
> index f727258487..369260c17a 100644
> --- a/tools/binman/elf_test.py
> +++ b/tools/binman/elf_test.py
> @@ -56,8 +56,8 @@ class FakeSection:
> def BuildElfTestFiles(target_dir):
> """Build ELF files used for testing in binman
>
> - This compiles and links the test files into the specified directory. It
> the
> - Makefile and source files in the binman test/ directory.
> + This compiles and links the test files into the specified directory. It
> uses
> + the Makefile and source files in the binman test/ directory.
>
> Args:
> target_dir: Directory to put the files into
> @@ -258,6 +258,33 @@ class TestElf(unittest.TestCase):
> offset = elf.GetSymbolFileOffset(fname, ['missing_sym'])
> self.assertEqual({}, offset)
>
> + def test_read_segments(self):
> + """Test for read_segments()"""
> + if not elf.ELF_TOOLS:
> + self.skipTest('Python elftools not available')
> + fname = self.ElfTestFile('embed_data')
> + segments, entry = elf.read_segments(tools.ReadFile(fname))
> +
> + def test_read_segments_fail(self):
> + """Test for read_segments() without elftools"""
> + try:
> + old_val = elf.ELF_TOOLS
> + elf.ELF_TOOLS = False
> + fname = self.ElfTestFile('embed_data')
> + with self.assertRaises(ValueError) as e:
> + elf.read_segments(tools.ReadFile(fname))
> + self.assertIn('Python elftools package is not available',
> + str(e.exception))
> + finally:
> + elf.ELF_TOOLS = old_val
> +
> + def test_read_segments_bad_data(self):
> + """Test for read_segments() with an invalid ELF file"""
> + fname = self.ElfTestFile('embed_data')
> + with self.assertRaises(ValueError) as e:
> + elf.read_segments(tools.GetBytes(100, 100))
> + self.assertIn('Magic number does not match', str(e.exception))
> +
>
> if __name__ == '__main__':
> unittest.main()
