btxldr: process all PT_LOAD segments, not just the first two

[emaste]

Summary

With default settings GNU ld generates two PT_LOADs for loader.sym while LLD generates three, because it creates a rodata segment. Previously btxldr terminated phdr processing after two PT_LOADs. Instead, loop over all phdr entries and process all PT_LOADs.

Test Plan

Build and boot the i386 loader with GNU ld and with lld, verifying that the intermediate loader.sym has 2 and 3 PT_LOAD segments respectively.

[tsoome]

sys/boot/i386/btx/btxldr/btxldr.S

238 ↗

(On Diff #23311)

perhaps we should read e_phentsize and use it here instead of 0x20?

[kib]

sys/boot/i386/btx/btxldr/btxldr.S

238 ↗

(On Diff #23311)

We do not know what to do if e_phentsize != 0x20.

239 ↗

(On Diff #23311)

The loop instruction decrements %ecx and compares it with 0. Old code read e_phnum into %ecx and used it as loop counter, i.e. there were two counters: one in %rdi (AKA 2) and one in %ecx (AKA e_phnum). You moved e_phnum into %edi, and left %ecx uninitialized.

IMO you should use %ecx instead of %edi, removing decl %edi/je start.7 part and remove unneded pushl/popl of %edi around rep movsb.

[tsoome]

sys/boot/i386/btx/btxldr/btxldr.S

239 ↗

(On Diff #23311)

you are correct with e_phentsize...

but yep, %edi game there is not needed at all:)

[jhb]

Also, this same bug may be in sys/kern/link_elf.c (for kernel modules on !amd64). It also assumes only 2 PT_LOAD segments. Hmm, seems it now supports 4 as of 151430, but there are older comments that say it assumes exactly two.

I think the kernel at least emits a sensible error if there are too many segments.

[jhb]

Also, still curious about the disparate command_set's and how that actually works (are there commands missing, etc.)

[emaste]

As pointed out by @kib the e_phnum loop count is already handled, so just remove the unnecessary 2 PT_LOAD safety belt.

[tsoome]

LGTM

[emaste]

In D8929#184808, @jhb wrote:

Also, still curious about the disparate command_set's and how that actually works (are there commands missing, etc.)

I'm sure it can't work; I'll look at that next.