casaroli commented on PR #19584: URL: https://github.com/apache/nuttx/pull/19584#issuecomment-5160160982
Thanks for the review — it changed the shape of this substantially. Summary of where it's going, then the individual threads below. The loader merges into ELF. binfmt/fdpic.c goes away and FDPIC becomes a mode of the existing loader. The one structural obstacle is the ET_DYN single load bias, which elf_load.c:580 states outright: "For Dynamic shared objects the relative positions between text and data must be maintained due to references to the GOT. Therefore we cannot do two different allocations." That is exactly what FDPIC repeals. The ET_REL PIC path is already the reference implementation for everything needed — XIP text via xipbase without copying (elf_load.c:590-594 plus the goto skipload at :430), separate data allocation (:615), per-section relocation bases rather than a load bias (elf_bind.c:336-349), read-only sections never patched (:355-360), GOT-based import binding (:346-351), and a fresh dspace_s per exec (binfmt/elf.c:272-285). So this is "make ET_DYN do what ET_REL already does", which is what you meant by "just like how the origin ELF add the support of PIC(XIP)". DT_NEEDED is dropped entirely. You're right that shared libraries belong to dlopen rather than to a loader-private dependency walker. A module carrying a DT_NEEDED entry will be refused with a clear error rather than loaded and left to fault. That removes fdpic_loaddepends(), the cross-object symbol search, the multi-object list and the owner-GOT rule — about a quarter of the loader, and the quarter with the weakest test coverage. It also removes the only part of the merge that would have changed behaviour for existing ELF and dlopen users, since DT_NEEDED loading does not exist in the tree today; include/elf.h:257 is the tag's only occurrence. r9 becomes the PIC base register for all of PIC, as a separate PR ahead of this one. That one turned out to be more interesting than expected — details in the register thread. I'm opening the loader work as a fresh PR rather than force-pushing here, so the two implementations can be compared side by side. I'll close this one once the new one has been looked at. Two questions still open, in the threads below: the PLT shim, and whether constructors move to the module's context in this series or a follow-up. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
