Subject: Re: libsa/loadfile.c
To: None <thorpej@zembu.com>
From: None <nigel@ind.tansu.com.au>
List: tech-kern
Date: 07/16/2001 12:00:43
I postulated:
>
> 2) The symbol space marker returned by elf_exec(), marks[MARK_SYM],
>    is set to elfp, which is only set if the LOAD_HDR flag is set.
> 
>    Shouldn't MARK_SYM should point to the start of the symbol space?
>    It does in aout_exec().


Jason stated:
>
> I wrote the ELF symbol handling stuff in elf_exec() and in DDB.  There is
> a very specific reason that elf_exec() sets MARK_SYM to the ELF header:

	Aah. Always helpful to talk to the original author
(whom I thought was Christos)

> 
> 	Chasing the ELF headers from the beginning is the only way
> 	to get to the string table, and to handle the multiple symbol
> 	and string talbes that ELF images may have.


* That is certainly true for complex elf executables.
  (e.g. > 2 symbol sections, with a string section in between)
  Not all ELF files would do that, though, and in those cases
  my simple additions will work.


* I now realise that my proposed changes would break any callers
  which parse the Elf Header. I should have had something like this:

	if ( flags & (LOAD_HDR|COUNT_HDR) )
		marks[MARK_SYM] = LOADADDR(elfp); /* Caller parses Elf_Ehdr */
	else
		marks[MARK_SYM] = LOADADDR(symp); /* Caller parses Elf_Sym */

  Sorry about that.


* The kernel I was parsing had the section header string table,
  then the single symbol table, and then the string table.
  My 10 line hack counted, and pointed to all this, just fine.


* Thanks for the pointer to db_elf.c - if I had found that code
  three months ago I may have taken a very different path.


* Doing a full parse of all the sections was the way I started out,
  but a few people advised me just to use loadfile & elf_exec().
  Sadly, unless it passes just a little bit more information back,
  using elf_exec() gains me little over a full parser.


>  > 3) Similarly, elf_exec() does not count symbols loaded:
>  >    marks[MARK_NSYM] = 1;	/* XXX: Kernel needs >= 0 */
> 
> That's because the count is sort of meaningless -- you get all the
> information from the ELF headers.

	True, but there is some code size and processing overhead
involved in parsing these sections again, when load_elf has already
done it. (Probably twice - once for counting, once for loading)

	As long as any multiple symbol sections are loaded adjacent,
(i.e. no string section in the middle), then such a count, with a
pointer to the first section, is quite meaningful to me!

	The count could be more meaningful if only the size of the
first symbol section was stored, but that loses the possibility of
counting multiple adjacent sections.





	As I see it, I can go three ways with the Booter.

1) Commit safe changes to load_elf() into the source tree,
   so that it returns simple symbol section information

2) Add extra ELF parsing on top of load_elf(), like ddb_elf.c

3) Write my own ELF parser, which may be more efficient


	Option 1 is my preference. With the extra code check above,
I think my proposed changes would be safe for all other users of
elf_exec().

	Option 3 is the next in line. While I do support code reuse,
if I need to write an ELF parser anyway, why not combine elf_exec's
stuff into it too.

-- 
| Nigel Pearson, nigel@ind.tansu.com.au | "Reality is that which,   |
|   Telstra NW-D, Sydney, Australia.    |  when you stop believing  |
| Office: 9206 3468    Fax:  9212 6329  |  in it, doesn't go away." |
| Mobile: 0408 664435  Home: 9792 6998  | Philip K. Dick - 'Valis.' |