Skip to content

Revert "Generate segment padding from ELF-internal offsets"#72

Merged
bradjc merged 3 commits intotock:masterfrom
lschuermann:otdev/elf-offset-padding
Jul 17, 2023
Merged

Revert "Generate segment padding from ELF-internal offsets"#72
bradjc merged 3 commits intotock:masterfrom
lschuermann:otdev/elf-offset-padding

Conversation

@lschuermann
Copy link
Copy Markdown
Member

This reverts commit d89f703. Ultimately, the underlying issue motivating these changes seems to be a linker bug, as documented in libtock-rs PR tock/libtock-rs#477 1. Indeed, we should follow the physical addresses as specified in the ELF file to generate padding, meaning that the previous code was correct.

However, there's two invariants which elf2tab should ensure still:

  • the libtock-rs bug seems to indicate that the section placement in ELF files does not necessarily have to correlate to their physical (load) address. It appears to be valid for ELF files to list sections in an arbitrary order, and thus elf2tab should presumably sort sections by their physical address (intended load address), and then walk them and insert padding as necessary.

  • the confusing behavior which lead to this incorrect bugfix was that elf2tab generated a very large (> 256MB) TBF. To ensure that this does not happen, we can introduce sanity checks.

    elf2tab acts as the Tock application loader, by placing apps into memory such that they can be executed in place. This means that the physical address of all LOADed segments must be within the board's FLASH address space. We can have elf2tab issue a warning if there is an excessive amount of space (>= 4kB) between the physical (load) end address of one segment, and the start address of the next. This prevents linker script bugs from creeping into elf2tab undetected, and cause it to blindly generate nonsensical binaries.

This reverts commit d89f703.
Ultimately, the underlying issue motivating these changes seems to be
a linker bug, as documented in libtock-rs PR tock/libtock-rs#477 [1].
Indeed, we should follow the physical addresses as specified in the
ELF file to generate padding, meaning that the previous code was
correct.

However, there's two invariants which elf2tab should ensure still:

- the libtock-rs bug seems to indicate that the section placement in
  ELF files does not necessarily have to correlate to their physical
  (load) address. It appears to be valid for ELF files to list
  sections in an arbitrary order, and thus elf2tab should presumably
  sort sections by their physical address (intended load address),
  and then walk them and insert padding as necessary.

- the confusing behavior which lead to this incorrect bugfix was
  that elf2tab generated a very large (> 256MB) TBF. To ensure that
  this does not happen, we can introduce sanity checks.

  elf2tab acts as the Tock application loader, by placing apps into
  memory such that they can be executed in place. This means that
  the physical address of all LOADed segments must be within the
  board's FLASH address space. We can have elf2tab issue a warning
  if there is an excessive amount of space (>= 4kB) between the
  physical (load) end address of one segment, and the start address
  of the next. This prevents linker script bugs from creeping into
  elf2tab undetected, and cause it to blindly generate nonsensical
  binaries.

[1]: tock/libtock-rs#477
@lschuermann
Copy link
Copy Markdown
Member Author

In addition to reverting the incorrect change, I've inserted two assertions that generate warnings when certain invariants are violated:

  • if elf2tab's calculations indicate that negative padding is required, segments are likely not ordered by their PhysAddr in the ELF file. We don't handle this case yet by reordering segments, as I'm not 100% sure that this won't break things.

  • if elf2tab would generate >= 4096 bytes of padding (where 4096 bytes is the default alignment of ELF segments), we warn the user that we're inserting a large amount of padding. This may happen when the PhysAddr of a segment in the ELF file is incorrect, and points (for instance) to the RAM address instead of the flash memory section. This is not supported by elf2tab, as it is a flash loader and Tock does not perform dynamic loading.

Thus, running elf2tab on the broken ELF files of tock/libtock-rs#477 (generating a > 256MB TBF) yields the following output:

  Warning! Inserting a large amount of padding.

Running elf2tab on a libtock-rs ELF without the changes of tock/libtock-rs#478 yields the following output:

  Warning! Expecting ELF sections to be in physical (load) address order.
           Not inserting padding, the resulting TBF may be broken.

elf2tab keeps quiet on innocent & correct libtock-rs and libtock-c ELFs.

@lschuermann lschuermann force-pushed the otdev/elf-offset-padding branch from 94b4a26 to 7e33b20 Compare July 13, 2023 17:58
Leon Schuermann added 2 commits July 13, 2023 13:59
This can happen when iterating over sections which are not placed in
physical address order in the ELF file. When this occurs, elf2tab has
a chance of producing an incorrect TBF file, which we should warn
users about.

It can be an indication of an incorrect / broken ELF file, although
placing sections in an arbitrary order within an ELF file may well be
allowed. This change is deliberately conservative by issuing a warning
instead of exiting with an error, to avoid breaking anyone's
toolchain. It may be reasonable to sort sections by their physical
address in the future, if this is confirmed to be correct behavior.
elf2tab generates TBFs which place ELF sections into flash according
to their physical address, thus acting as a flash loader. To ensure
that segments end up at their correct flash offset, it may need to
insert some padding in between two sections.

However, this padding insertion can also mask errors in the ELFs
placement constraints. For instance, when the ELF file includes a
non-zero sized segment which is specified to be loaded at a non-flash
address (e.g. a RAM segment having its PhysAddr == VirtAddr), elf2tab
may insert an excessive amount of padding to meet those placement
constraints. This generates incorrect TBFs.

We issue a warning when encountering such a situation. 4096 bytes is
larger than the default ELF segment padding, and thus we assume that
applications will likely not require more padding in between their
sections. A warning will ensure that developers are made aware of
potential issues, and can be directed to this comment, this commit, or
the accompanying PR.
@lschuermann lschuermann force-pushed the otdev/elf-offset-padding branch from 7e33b20 to 161bbb2 Compare July 13, 2023 17:59
@bradjc bradjc merged commit eaf833e into tock:master Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants