Re: libelf gelf_newehdr and gelf_newphdr return types

[Ali Bahrami <Ali dot Bahrami at Oracle dot COM>]

On 12/ 2/16 01:25 PM, Ali Bahrami wrote:
>
>    I'm currently searching code to see what the impacts are,
> but I suspect that there are none, and that we should follow
> FreeBSD's lead. They've done us a service by already testing
> out the viability.
>
> The reason for using unsigned long was undoubtedly so that
> pre-K&R C compilers can compile the code. gelf was invented
> at a time in history where ansi C was still pretty new, and
> not everyone was using it. 'unsigned long' was the pre-ansi
> idiom for "generic pointer" that could be used in either mode.
>
> All of the code I've found in Solaris using these functions so
> far, and there is very little, treat the return value as a
> pointer, and even compare it to NULL, so switching to 'void *'
> would be transparent and binary compatible.
>
> I'll follow up to this as soon as I'm done digging. In the meantime,
> might I suggest that you make this change in a test environment,
> do a 'make world', and see if you hit any issues?


Hi Mark,

    I'm done with my experiments, and I think that we should
adopt 'void *' as the return type for these functions. That
might take awhile, but I see no reason why the folks doing the
Windows port shouldn't move ahead immediately, using a few
ifdefs, which can be later removed when we all catch up.

I already described the machine-level reasons why I think this
change is safe. 'void *' and 'unsigned long' generate the same
machine code for systems where

     sizeof(long) == sizeof(pointer)

which generally holds for unix, and on any systems that are
sill likely to be running. For these systems, these are equivalent
alternative syntaxes that generate the same machine code, and and
here should be no binary level incompatibilities. The FreeBSD experiment
provides real world evidence of this.

That's the theory, here are my experiments...

The first thing to report is that use of these 2 functions
is very rare. In Solaris, they are called once each in a small
set of programs:

     - The mcs/strip/elfcompress programs, which are really
       one program, hardlinked together, using the argv[0]
       trick to figure out what they are at runtime.

     - A demo program we ship under /usr/demo/ELF

     - A couple of we use to build Solaris, but which are not
       shipped.

In all of these cases, our code calls them as such:

     if (gelf_newehdr(elf, class) == NULL)
             bail();

and then accesses them using gelf get/set routines. We know that
the return value is a valid pointer, but we never access it via
that pointer, probably because that requires the programmer to
care about which ELFCLASS they're dealing with. To do that defeats
the only real reason to use GElf rather than just calling the
"real" class-specific libelf routines. For that reason, it would
have made sense for these functions to return a boolean error/success
indication rather than a pointer. I don't think we should make that
change at this late date --- it's just an observation in passing.

I also googled for these APIs, and found very little of note,
other than manpages.

I made the ['void *' change, to the <gelf.h> header, and to libelf,
and then without modifying any calling code, did a full build of our
core OS workspace. There were no errors. I then installed those
bits on my build machine, and used it to do a second full build,
and that one was also completely clean. I therefore have no concerns
about this regarding any code that we produce.

I also grepped the sources for a majority of the open source that
we deliver (including gcc and binutils), and found that none of it
calls these functions. If they did, I'm confident that they would
"just work", with the possible exception of some easily fixed C++.
However, as nothing calls it, there nothing to be concerned about.

In your environment, you may have some different consumers, so I'll
wait to hear what you find, but 'void *' seems OK from here. In the
final analysis, our main concern is to keep the various libelf's
unified, so we'll go along with the group consensus.

- Ali