On Mon, Jul 16, 2001 at 12:00:43PM +1000, nigel@ind.tansu.com.au wrote:

 > * 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.

For *YOUR* case only, however.

 > * 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.

How do you find the string table, then?  What do you need to do to the
symbol table?  Relocate every symbol?  If you're not just touching every
symbol, then you need access to the string table, as well, and assuming
that it will be loaded in some kind of order is simply wrong.

 > * 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.

For that specific case.  And then when we upgrade the linker, or someone
modifies the linker script for some reason, and it writes sections in a
different order, you will lose.

Since ELF is a very flexible image format, you need code to parse it
in order to manipulate it.

 > * 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.

Well, I stronly encourage you to take that path now.

 > * 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.

No, using elf_exec() gains you a lot:

	(1) It saves you from having to re-implement the functionality
	    it already provides.

	(2) It is guaranteed to load symbols in the way the kernel
	    expects for DDB.

It already passes back all the information you need, and you don't need
a whole lot of extra code to locate the symbol table once elf_exec() has
loaded it.

 > 	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)

The code size and overhead is ... tiny.  This is a boot program,
for goodness sake, so blinding speed isn't exactly a high priority.
And code size -- I suspect the very small amount of code required
hunt down and cache pointers to the symbols and strings is negligible.

 > 	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!

Except you can't make this assumption with ELF.

 > 	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

They're not really "safe" changes -- the boot program is expressly
defined to load ELF symbols in a specific way, and you break that,
even if you only break it when LOAD_HDR isn't set, you still break it.

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

This is the correct way to accomplish what you want to do.  It's
a tiny amount of extra work, and you have the added bonus of having
DDB symbols work with the ELF kernel (because they're loaded in the
format that the kernel expects).

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

...but would be wrong to do so.  Re-use existing code, add additional
code for your (corner case!) as necessary.

 > 	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.

...not a full ELF parser to find the symbols/strings afterwards.  You
have a parsing loop that is 14 lines long to do it the way I suggest.
That is far from a full ELF parser.

-- 
        -- Jason R. Thorpe <thorpej@zembu.com>