This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC: Add initial support for .NET Core dlls to objdump


Hi Omair,

>>> 0. Should this "non-stanard" magic field in the dll be exposed somewhere
>>>    in the objdump UI?
>>
>> Yes.  Probably in the output for -p/--private-headers would be best.  Have
>> a look at the _bfd_XX_print_private_bfd_data_common function in bfd/peXXigen.c.
> 
> I am running into a bit of a wall here. I need to access the original
> value of internal_filehdr->f_magic here. But that's not accessible at
> this point.
>
> I took a look at dump_bfd_header in objdump.c and that works with
> processed bdf entries too.
> 
> Any pointers on how I can get a hold of the original f_magic field?

In theory you could call coff_set_flags() and that you give you the
f_magic value as a return value.  But in practice this will give you
the f_magic value that the BFD library *thinks* that the output file
should have, and not the f_magic value that was read in.  

Incidentally this raises another issue - it looks like the BFD library
will not generate PE files with these new magic values, so if you run
say "objcopy a.exe b.exe" then b.exe will not have the same f_magic as
a.exe.  Assuming that a.exe had the new f_magic value.

Anyway, so the short answer to your question is that you cannot access
the f_magic value of the input bfd.  The long answer is that the way to
solve this is to create new bfd architecture structures to handle the
new f_magic values.  Then you can have coff_set_arch_mach_hook() return 
the new architectures and coff_set_flags() return new f_magic values for
those architectures.  If you would like to have a go at doing this then
please do... 
 
>>> 2. I added the new flags for architecture/OS combination for the binaries I
>>>    could find. Should I try and find out what the magic flags for other
>>>    architecture/OS combinations (bsds? arm64?) are? 

> If it's okay with you, can I do that as a separate patch?

Sure - that is not a problem.


> The .NET Core sources are available under the MIT license:
> https://github.com/dotnet/coreclr/blob/6f7aa7967c607b8c667518314ab937c0d7547025/src/inc/pedecoder.h#L94-L107
> 
> Is that something I can use/link to as "a source of
> truth/documentation"?

Sure, it does not hurt.

> I have attached an updated patch.

Thanks - there are a few issues with the patch that I will mention here:

> +static int write_dos_header_and_stub(FILE* file);
> +static int write_pe_signature(FILE* file);
> +static int write_coff_header(FILE* file, uint16_t machine);

You do not need prototypes for static functions unless they are
used before they are defined in the source file.  (And generally
speaking it is better to define them first if possible, as this
does help some compilers produce better code).

Also - I note that all three of these functions return an integer
value which is always 0, and which is always ignored.  It would
be better to just have them be void functions...

Also please include a single white space between the end of the
function name and the opening parenthesis of its arguments.  eg:

  write_dos_header_and_stub (FILE* file)

This applies both when declaring the function and when using it.


> +  /*
> +   * See ECMA-335 II.25.2.1.
> +   *
> +   * Instead of lfanew, lets just hardcode the offset of the next byte
> +   * after this header (0x80).
> +   */

Multiline comments should not have line prefixes, should be treated as a 
normal paragraph and should end on the last line of the comment.  ie:

   /* See ECMA-335 II.25.2.1.
      Instead of lfanew, lets just hardcode the offset of the next byte
      after this header (0x80).  */


> +  char buffer[128] = {

Curly braces should be moved to the line below the declaration, rather than
ending a line.  ie:

    char buffer[128] = 
    {

I also think that since the gentestdlls.c program is part of the binutils
testsuite it ought to live in the binutils/testsuite directory and not the
binutils/ directory.  Also you need a mechanism to build the gentestdlls
executable, and to bail out from the test if the executable cannot be created.

Thanks for persisting with this patch.

Cheers
  Nick

PS.  I am off on PTO for two weeks, so I will not be able to respond to emails
until later this month...


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]