process: unify load on NOMMU #611
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
badochov
force-pushed
the
badochov/process_load_remove_ifdef
branch
from
ff1ed8d
to
2a6dd63
Compare
proc/process.c
Outdated
| sym = (Elf32_Sym *)(symTab + (ELF32_R_SYM(rela->r_info) * sizeof(Elf32_Sym))); | ||
|
|
||
| /* Write addend to the address */ | ||
| *(char **)relptr = (char *)(sym->st_value + rela->r_addend); |
There was a problem hiding this comment.
This is a pretty much the only difference between SPARC and ARM stuff. Perhaps we could store strcmp result and use it here to do this operation conditionally?
There was a problem hiding this comment.
I thought about it and it was a bunny hole.
Each architecture has a separate set of Relocation Types, however relocation types are just numbers and there maybe a clash eg. R_SPARC_32 on SPARC and R_ARM_REL32(unused now) on ARM have the same value of 3. Thus, we should compare relocation type separately on each architecture. This can be done either via ifdefs or via checking if in the ELF header the machine type(e_machine field). Now this clash will not happen, as SPARC uses RELA and ARM uses REL. If we unify the code there is high potential for a bug in the future if more relocation types would be needed for some reason.
In my opinion the code should be modified so that:
- Getting
relasection on an architecture usingrelwould result inENOEXECand vice versa - We should have separate relocations handler for each architecture
What do you think about it? Also, Should we check the architecture via ifdef or the ELF HEADER?
There was a problem hiding this comment.
Well, first question that has to be asked is if we'll ever need to handle any more relocation types in the kernel? Aren't dynamic linker solely in the userspace?
Regarding collisions of values of relocation types between archs - this can be solved by e.g. defining our own, abstract, machine independent relocation types on per arch basis, and handle these instead of e.g. R_SPARC_32 directly (that might be represented by e.g. HAL_R_REL32 or something along these lines).
There was a problem hiding this comment.
- To support static FDPIC program some 2 more relocations are needed, unless we want to move relocation process to the userspace.
Additionally if we ever want to handle C++ on NOMMU then to have proper exceptions handling there may be a need to add few more relocations. However that's a harder topic as currently exception handling causes relocations in text segment ☠️
- Nice idea!
Also my bad, both ARM and SPARC support in ABI both rela and rel, however GCC by default uses only one of those types if possible.
There was a problem hiding this comment.
My perspective is as follows - better for this code to do the thing it has to do nice and simple and to not prepare for the future that might never come (kinda a part of the top-down philosophy).
From my side - the proposed version is way better and status quo as is, so I have no objection to merge if you wish to not make any further changes.
There was a problem hiding this comment.
I checked and compilation with -static-pie fixes issue of relocation in text segment in exceptions handling section
There was a problem hiding this comment.
I have adapted code to you suggestions, and added feature to handle both Rel and Rela relocations on both targets
badochov
force-pushed
the
badochov/process_load_remove_ifdef
branch
2 times, most recently
from
172480e
to
58dceed
Compare
badochov
force-pushed
the
badochov/process_load_remove_ifdef
branch
from
58dceed
to
ff882dc
Compare
badochov
force-pushed
the
badochov/process_load_remove_ifdef
branch
from
ff882dc
to
e448443
Compare
JIRA: RTOS-958
badochov
force-pushed
the
badochov/process_load_remove_ifdef
branch
from
e448443
to
55f8566
Compare
JIRA: RTOS-958
Description
Motivation and Context
Types of changes
How Has This Been Tested?
Checklist:
Special treatment