[PATCH v3] bfd: fill in PE load config directory entry.

[Jeremy Drake]

I might not be able to get right on this right now, I've got some other
stuff going on, but I want to memorialize my understanding at least.

On Fri, 21 Mar 2025, Jan Beulich wrote:

> On 20.03.2025 19:22, Jeremy Drake wrote:
> > On Thu, 20 Mar 2025, Jan Beulich wrote:
> >> Still assuming bfd_get_symbol_leading_char() can only ever return nil or '_'?
> >
> > Yes:
> >> Nevertheless, because of the pre-existing similar code, I'm not going to
> >> insist that this be corrected right here.
>
> Oh, right, I said that earlier on. A remark towards the limitation in the
> commit message would maybe help.
>
> >> I wonder if we can indeed blindly use that value, with no sanity checking
> >> whatsoever.
> >>
> >> Furthermore, having checked two random executables each from two random VC
> >> installations, I find that the size in the data directory and the value at
> >> the start of the referenced RVA aren't the same: In all four cases the data
> >> directory says 0x40 while the 32-bit item at the indicated RVA is 0x48.
> >
> > That's odd.  This matches what LLD does (though they do indeed do some
> > sanity checking):
> > https://github.com/llvm/llvm-project/blob/49f06075a6d298bd13564c9bffcf51281bed4962/lld/COFF/SymbolTable.cpp#L576
>
> As one more data point, a quote from MS doc: "For Windows XP, the size must
> be specified as 64 for x86 images." It looks odd to me though that the exact
> same size is stated for both the 32- and 64-bit struct.
>
> Since hopefully hardly anyone still cares about XP, doing what you do (and
> what llvm does) is likely fine, but imo wants to come with a code comment
> then.


* Add note to commit message about bfd_get_symbol_leading_char actually
returning the char in question, and code shouldn't be assuming it's '_'.
(I've only dealt with this on Windows, and other code I've dealt with
generally has a boolean as to whether to add an underscore or not (and
then only sets it to true on i386)).

* Add sanity checks (symbol + size is still contained in section,
required alignment is satisfied)

* Add code comment about possible issues on Windows XP where it might be
necessary to lie about the size in the directory entry?

Feel free to correct me if any of that sounds wrong.  Also, I wouldn't be
offended if somebody else picks this up in the meantime and did a
Co-authored-by or something.  I do still have another patch series in
https://github.com/jeremyd2019/binutils-gdb/pulls fixing delay-load
crashes introduced in 2.44.  That's probably more important than this, but
I was hoping to figure out the process and pitfalls of contributing to
binutils before tackling that more complicated change.  Unfortunately life
has intervened and I may not be able to get back to that soon either, so
again, feel free to pick it up and run with that too.