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

[Omair Majid <omajid at redhat dot com>]

Hi Nick,

Thanks for reviewing the patch and the great feedback!

* Nick Clifton <nickc@redhat.com> [2019-07-01 08:11]:
> > 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?

> > 1. Should I add tests for these? If so, any pointers on how to do that?
> 
> If it is easy to do then yes.  But I suspect that it might be difficult
> to construct a file containing the new machine field value.  If you can
> do that however then the best place to add one or more tests would be the
> binutils/testsuite/binutils-all directory.  Possibly as an addition to the
> objdump.exp file, or else as a new stand alone test. 

Done. At least two dll files are now generated "by hand" when tests are
run.

> > 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? Even if I don't have
> >    access to binary dlls that demonstrate this?
> 
> Yes please.  In cases like this it is best to try to cover as many variants
> as possible, even if you cannot test them all.  If there are any errors then
> at some point in the future someone will file a bug report pointing out the
> problem.  But if you get the values right first time then everyone will be
> happy, and no one will have to be nailed to a tree for doing it.  (Sorry - 
> slipped into misquoting the Hitch Hikers' Guide to the Galaxy there).

If it's okay with you, can I do that as a separate patch? I would like
to take care of take care of aarch64 (and others platforms and
architectures) separately instead of making this patch much larger.

> One thing missing from the patch, which is probably not your fault at all,
> is comment containing a URL to the official documentation for these new
> values.  I am of course assuming that there is some official documentation,
> and that it is publicly accessible...

I added some links to some sources I have been looking at, but even
ECMA-335 says that only one valid value is possible, which we know is
no longer true. The best and the most concise source I can find is bug
report for a project:
https://github.com/jbevain/cecil/issues/337

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"?

I have attached an updated patch.

bfd/ChangeLog:

2019-07-03  Omair Majid  <omajid@redhat.com>

	* coffcode.h (coff_set_arch_mach_hook): Handle
	I386_NATIVE_LINUX_MAGIC, I386_NATIVE_APPLE_MAGIC,
	AMD64_NATIVE_LINUX_MAGIC and AMD64_NATIVE_LINUX_MAGIC.
	* peXXigen.c: Add references to ECMA-335.

binutils/ChangeLog:

2019-07-03  Omair Majid  <omajid@redhat.com>

	* binutils/Makefile.amh (GENTESTDLLS_PROG): Define.
	(TEST_PROGS): Add GENTESTDLLS_PROG.
	(gentestdlls_DEPENDENCIES): Define.
	* gentestdlls.c: New file.
	* testsuite/binutils-all/objdump.exp
	(test_objdump_dotnet_assemblies): Define.

include/ChangeLog:

2019-07-03  Omair Majid <omajid@redhat.com>

       * coff/pe.h (IMAGE_FILE_MACHINE_NATIVE_LINUX_OVERRIDE),
       (IMAGE_FILE_MACHINE_NATIVE_APPLE_OVERRIDE): Define.
       * coff/i386.h (I386_NATIVE_LINUX_MAGIC),
       (I386_NATIVE_APPLE_MAGIC): Define.
       (I386BADMAG): Extend to include the above.
       * coff/x86_64.h (AMD64_NATIVE_LINUX_MAGIC),
       (AMD64_NATIVE_APPLE_MAGIC): Define.
       (AMD64BADMAG): Extend to include the above.

Omair

-- 
PGP Key: B157A9F0 (http://pgp.mit.edu/)
Fingerprint = 9DB5 2F0B FD3E C239 E108  E7BD DF99 7AF8 B157 A9F0

Attachment: 0001-Handle-some-PE-COFF-files-generated-by-.NET.patch
Description: Text document