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

Message ID 20190626185416.GA10764@redhat.com
State New
Headers show
Series
  • RFC: Add initial support for .NET Core dlls to objdump
Related show

Commit Message

Omair Majid June 26, 2019, 6:54 p.m.
Hi,

Recent versions of .NET Core ship with some dll (PE/COFF) files that
can't be parsed by objdump:

    $ objdump -x /usr/lib64/dotnet/shared/Microsoft.NETCore.App/2.1.11/System.dll 
    objdump: /usr/lib64/dotnet/shared/Microsoft.NETCore.App/2.1.11/System.dll: file format not recognized

It seems like these files have a slightly different value for the
IMAGE_FILE_HEADER.Machine field than normal dlls. In particular, the "normal"
architecture-based magic value is XOR'ed with an OS-specific value to get the
final magic value. [1] 

Allowing the new magic values lets objdump get started:

    $ ~/local/binutils/bin/objdump -x dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll
    dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll:   file format pei-x86-64
    dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll
    architecture: i386:x86-64, flags 0x0000012f:
    HAS_RELOC, EXEC_P, HAS_LINENO, HAS_DEBUG, HAS_LOCALS, D_PAGED

    Characteristics 0x2022
        executable
        large address aware
        DLL

    Time/Date               Wed Jun  5 14:49:41 2019
    Magic                   020b    (PE32+)
    ...


Some open questions:

0. Should this "non-stanard" magic field in the dll be exposed somewhere
   in the objdump UI?

1. Should I add tests for these? If so, any pointers on how to do that?

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?

3. Since this touches shared code, do I need to have this patch reviewed
   elsewhere too?

This is my first patch for binutils, so I would appreciate it someone can tell
me about any other mistakes I am making (or about to make) :)

Thanks,
Omair

[1] https://github.com/jbevain/cecil/issues/337
-- 
PGP Key: B157A9F0 (http://pgp.mit.edu/)
Fingerprint = 9DB5 2F0B FD3E C239 E108  E7BD DF99 7AF8 B157 A9F0
From 29de65f827fe804a87dda340091d1c3f7c81f5df Mon Sep 17 00:00:00 2001
From: Omair Majid <omajid@redhat.com>

Date: Tue, 25 Jun 2019 18:03:42 -0400
Subject: [PATCH] Handle some pe files generated by .NET

The System.Runtime.dll files that get shipped with .NET Core 2.1 on
different platforms demonstrate original the problem:

    $ objdump -x dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll
    objdump: dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll: file format not recognized

After this fix:

    $ ~/local/binutils/bin/objdump -x dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll
    dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll:   file format pei-x86-64
    dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll
    architecture: i386:x86-64, flags 0x0000012f:
    HAS_RELOC, EXEC_P, HAS_LINENO, HAS_DEBUG, HAS_LOCALS, D_PAGED

    Characteristics 0x2022
        executable
        large address aware
        DLL

    Time/Date               Wed Jun  5 14:49:41 2019
    Magic                   020b    (PE32+)
    ...

These PE files are regular PE/COFF files but have a different value for
IMAGE_FILE_HEADER.Machine to indicate it contains a native image that
targets a non-Windows platform. This value is OS dependent and varies
across systems (linux vs netbsd vs macos).
---
 bfd/ChangeLog         |  6 ++++++
 bfd/coffcode.h        |  4 ++++
 include/ChangeLog     | 11 +++++++++++
 include/coff/i386.h   |  6 ++++++
 include/coff/pe.h     |  3 +++
 include/coff/x86_64.h |  8 +++++++-
 6 files changed, 37 insertions(+), 1 deletion(-)

-- 
2.21.0

Comments

Nick Clifton July 1, 2019, 12:11 p.m. | #1
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.


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


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


> 3. Since this touches shared code, do I need to have this patch reviewed

>    elsewhere too?


No - the file you are touching are all considered to be part of the binutils
project, even if they are shared with others.


> This is my first patch for binutils, so I would appreciate it someone can tell

> me about any other mistakes I am making (or about to make) :)


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

A couple of very minor points on the patch itself:

> --- a/bfd/ChangeLog

> +++ b/bfd/ChangeLog


In the future please could you provide the changelog entries just as simple
text, rather than as context diffs ?  The problem is that the changelogs
change so frequently that a context diff will often fail to apply.


> +/* Used in .NET DLLs that target non-Windows platforms */


Comments should be formatted as sentences.  So they should end with a period
and be followed by two spaces before the closing-comment characters.

But basically the patch - so far - looks great.

Cheers
  Nick
Omair Majid July 4, 2019, 4:43 a.m. | #2
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
From 391a2ebbc34e9100c1c7e307b4415cb04b8b2c59 Mon Sep 17 00:00:00 2001
From: Omair Majid <omajid@redhat.com>

Date: Tue, 25 Jun 2019 18:03:42 -0400
Subject: [PATCH] Handle some PE/COFF files generated by .NET

The System.Runtime.dll files that get shipped with .NET Core 2.1 on
different platforms demonstrate an issue:

    $ objdump -x dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll
    objdump: dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll: file format not recognized

After this fix:

    $ ~/local/binutils/bin/objdump -x dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll
    dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll:   file format pei-x86-64
    dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll
    architecture: i386:x86-64, flags 0x0000012f:
    HAS_RELOC, EXEC_P, HAS_LINENO, HAS_DEBUG, HAS_LOCALS, D_PAGED

    Characteristics 0x2022
        executable
        large address aware
        DLL

    Time/Date               Wed Jun  5 14:49:41 2019
    Magic                   020b    (PE32+)
    ...

These PE files are regular PE/COFF files but have a different value for
IMAGE_FILE_HEADER.Machine to indicate it contains a native image that
targets a non-Windows platform. This value is OS dependent and varies
across systems (linux vs netbsd vs macos).

ECMA-335 does not mention these new values; it still insists that the
only valid value is 0x14c.

This commit also introduces a tool to generate handcrafted dll files
that can be used in tests to verify objdump parsing.
---
 bfd/coffcode.h                              |   4 +
 bfd/peXXigen.c                              |   7 +
 binutils/Makefile.am                        |   4 +-
 binutils/gentestdlls.c                      | 167 ++++++++++++++++++++
 binutils/testsuite/binutils-all/objdump.exp |  29 ++++
 include/coff/i386.h                         |   6 +
 include/coff/pe.h                           |   3 +
 include/coff/x86_64.h                       |   8 +-
 8 files changed, 226 insertions(+), 2 deletions(-)
 create mode 100644 binutils/gentestdlls.c

diff --git a/bfd/coffcode.h b/bfd/coffcode.h
index c67bfbb0e6..94f0bacc4a 100644
--- a/bfd/coffcode.h
+++ b/bfd/coffcode.h
@@ -2104,6 +2104,8 @@ coff_set_arch_mach_hook (bfd *abfd, void * filehdr)
 #endif
 #ifdef I386MAGIC
     case I386MAGIC:
+    case I386_NATIVE_LINUX_MAGIC:
+    case I386_NATIVE_APPLE_MAGIC:
     case I386PTXMAGIC:
     case I386AIXMAGIC:		/* Danbury PS/2 AIX C Compiler.  */
     case LYNXCOFFMAGIC:
@@ -2112,6 +2114,8 @@ coff_set_arch_mach_hook (bfd *abfd, void * filehdr)
 #endif
 #ifdef AMD64MAGIC
     case AMD64MAGIC:
+    case AMD64_NATIVE_LINUX_MAGIC:
+    case AMD64_NATIVE_APPLE_MAGIC:
       arch = bfd_arch_i386;
       machine = bfd_mach_x86_64;
       break;
diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
index 77fb4933e7..ee6da6480d 100644
--- a/bfd/peXXigen.c
+++ b/bfd/peXXigen.c
@@ -35,6 +35,13 @@
    "Peering Inside the PE: A Tour of the Win32 Portable Executable
    File Format", MSJ 1994, Volume 9.
 
+   The PE/PEI format is also used by .NET. ECMA-335 describes this:
+
+   "Standard ECMA-335 Common Language Infrastructure (CLI)", 6th Edition, June 2012.
+
+   This is also available at
+   https://www.ecma-international.org/publications/files/ECMA-ST/ECMA-335.pdf.
+
    The *sole* difference between the pe format and the pei format is that the
    latter has an MSDOS 2.0 .exe header on the front that prints the message
    "This app must be run under Windows." (or some such).
diff --git a/binutils/Makefile.am b/binutils/Makefile.am
index 128494ca9e..625a3fd7e6 100644
--- a/binutils/Makefile.am
+++ b/binutils/Makefile.am
@@ -93,8 +93,9 @@ EXTRA_SCRIPTS = embedspu
 ## Test programs.
 BFDTEST1_PROG = bfdtest1
 BFDTEST2_PROG = bfdtest2
+GENTESTDLLS_PROG = gentestdlls
 
-TEST_PROGS = $(BFDTEST1_PROG) $(BFDTEST2_PROG)
+TEST_PROGS = $(BFDTEST1_PROG) $(BFDTEST2_PROG) $(GENTESTDLLS_PROG)
 
 ## We need a special rule to install the programs which are built with
 ## -new, and to rename cxxfilt to c++filt.
@@ -233,6 +234,7 @@ elfedit_DEPENDENCIES = $(LIBINTL_DEP) $(LIBIBERTY)
 dllwrap_DEPENDENCIES =   $(LIBINTL_DEP) $(LIBIBERTY)
 bfdtest1_DEPENDENCIES =  $(LIBINTL_DEP) $(LIBIBERTY) $(BFDLIB)
 bfdtest2_DEPENDENCIES =  $(LIBINTL_DEP) $(LIBIBERTY) $(BFDLIB)
+gentestdlls_DEPENDENCIES =
 
 LDADD = $(BFDLIB) $(LIBIBERTY) $(LIBINTL)
 
diff --git a/binutils/gentestdlls.c b/binutils/gentestdlls.c
new file mode 100644
index 0000000000..ff2d6add7e
--- /dev/null
+++ b/binutils/gentestdlls.c
@@ -0,0 +1,167 @@
+/* Copyright (C) 2007-2019 Free Software Foundation, Inc.
+
+   This file is part of GNU Binutils.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA
+   02110-1301, USA.  */
+
+
+/*
+   This file generates a number of DLL (PE/COFF binaries traditionally
+   used on Windows) that we can then utilize in various tests to
+   ensure objdump can parse these file correctly.
+
+   See:
+   https://www.ecma-international.org/publications/files/ECMA-ST/ECMA-335.pdf
+   https://docs.microsoft.com/en-us/windows/desktop/debug/pe-format
+   https://en.wikibooks.org/wiki/X86_Disassembly/Windows_Executable_Files
+
+ */
+
+#include <memory.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+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);
+
+static int
+write_dos_header_and_stub(FILE* file)
+{
+  /*
+   * 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] = {
+    0x4d, 0x5a, 0x90, 0x00, 0x03, 0x00, 0x00, 0x00,
+    0x04, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0x00, 0x00,
+    0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, 0x00, 0x00, /* Last 4 bytes are precomputed lfanew.  */
+    0x0e, 0x1f, 0xba, 0x0e, 0x00, 0xb4, 0x09, 0xcd,
+    0x21, 0xb8, 0x01, 0x4c, 0xcd, 0x21, 0x54, 0x68,
+    0x69, 0x73, 0x20, 0x70, 0x72, 0x6f, 0x67, 0x72,
+    0x61, 0x6d, 0x20, 0x63, 0x61, 0x6e, 0x6e, 0x6f,
+    0x74, 0x20, 0x62, 0x65, 0x20, 0x72, 0x75, 0x6e,
+    0x20, 0x69, 0x6e, 0x20, 0x44, 0x4f, 0x53, 0x20,
+    0x6d, 0x6f, 0x64, 0x65, 0x2e, 0x0d, 0x0d, 0x0a,
+    0x24, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+  };
+
+  fwrite(buffer, 1, 128, file);
+
+  return 0;
+}
+
+static int
+write_pe_signature(FILE* file)
+{
+  char buffer[4];
+  buffer[0] = 'P';
+  buffer[1] = 'E';
+  buffer[2] = 0;
+  buffer[3] = 0;
+  fwrite(buffer, 1, 4, file);
+  return 0;
+}
+
+static int
+write_coff_header(FILE* file, uint16_t machine)
+{
+  char buffer[128];
+  memset(buffer, 0, sizeof(buffer));
+
+  /* Machine.  ECMA-335 says this must be 0x14c but that's not true anymore.  */
+  buffer[0] = machine & 0xff;
+  buffer[1] = machine >> 0x8;
+  fwrite(buffer, 2, 1, file);
+  memset(buffer, 0, sizeof(buffer));
+  /* NumberOfSections = 0 */
+  fwrite(buffer, 2, 1, file);
+  /* TimeDateStamp = 0 */
+  fwrite(buffer, 4, 1, file);
+  /* PointerToSymbolTable = 0 */
+  fwrite(buffer, 4, 1, file);
+  /* NumberOfSymbols = 0 */
+  fwrite(buffer, 4, 1, file);
+  /* OptionalHeaderSize = 0 */
+  fwrite(buffer, 2, 1, file);
+  /* Characteristics = 0x2000 */
+  buffer[0] = 0x00;
+  buffer[1] = 0x20;
+  fwrite(buffer, 2, 1, file);
+  memset(buffer, 0 , sizeof(buffer));
+
+  return 0;
+}
+
+int
+main(int argc, char** argv)
+{
+  if (argc < 2)
+    {
+      fprintf(stderr, "usage: %s output-directory\n", argv[0]);
+      exit(2);
+    }
+  if (chdir(argv[1]) != 0)
+    {
+      fprintf(stderr, "error: unable to change directory to %s\n", argv[0]);
+      exit(2);
+    }
+
+  FILE* file;
+
+  /* Generate a simple DLL file.  */
+  file = fopen("simple-i386.dll", "w");
+  if (file == NULL)
+    {
+      fprintf(stderr, "error: unable to open file for writing\n");
+      exit(1);
+    }
+
+  write_dos_header_and_stub(file);
+  write_pe_signature(file);
+  write_coff_header(file, 0x14c);
+  fclose(file);
+  printf("wrote simple-i386.dll\n");
+
+  /* Generate a sample .NET Core on Linux dll file.  As opposed to the
+     more common DLLs that contain bytecode (CIL/MSIL), many .NET Core
+     DLLs are pre-compiled for specific architectures and platforms.
+     See https://github.com/jbevain/cecil/issues/337 for an example of
+     this value being used in practice.  */
+  file = fopen("dotnet-linux-x86-64.dll", "w");
+  if (file == NULL)
+    {
+      fprintf(stderr, "error: unable to open file for writing\n");
+      exit(1);
+    }
+
+  write_dos_header_and_stub(file);
+  write_pe_signature(file);
+  write_coff_header(file, 0xfd1d /* x86-64 + Linux */);
+  fclose(file);
+  printf("wrote dotnet-linux-x86-64.dll\n");
+
+  return 0;
+}
diff --git a/binutils/testsuite/binutils-all/objdump.exp b/binutils/testsuite/binutils-all/objdump.exp
index beaf44f954..657b4541c8 100644
--- a/binutils/testsuite/binutils-all/objdump.exp
+++ b/binutils/testsuite/binutils-all/objdump.exp
@@ -732,6 +732,35 @@ if {[is_elf_format]} then {
     remote_file host delete $testfile3
 }
 
+# Test objdump on .NET assemblies (PE files)
+
+proc test_objdump_dotnet_assemblies {} {
+    global OBJDUMP
+    global base_dir
+
+    set test "dotnet-assemblies"
+
+    set got [binutils_run "$base_dir/gentestdlls" "tmpdir"]
+    set want "wrote dotnet-linux-x86-64.dll"
+    if ![regexp $want $got] then {
+	fail "$test"
+    }
+
+    set want "file format pei-i386"
+    set got [binutils_run $OBJDUMP "-x tmpdir/simple-i386.dll"]
+    if ![regexp $want $got] then {
+	fail "$test"
+    }
+
+    set want "file format pei-x86-64"
+    set got [binutils_run $OBJDUMP "-x tmpdir/dotnet-linux-x86-64.dll"]
+    if ![regexp $want $got] then {
+	fail "$test"
+    }
+}
+
+test_objdump_dotnet_assemblies
+
 # Options which are not tested: -a -D -R -T -x -l --stabs
 # I don't see any generic way to test any of these other than -a.
 # Tests could be written for specific targets, and that should be done
diff --git a/include/coff/i386.h b/include/coff/i386.h
index 1d2ccff6f7..55b67572bf 100644
--- a/include/coff/i386.h
+++ b/include/coff/i386.h
@@ -43,7 +43,13 @@
 
 #define LYNXCOFFMAGIC	0415
 
+/* Used in .NET DLLs that target a specific platform.  */
+#define I386_NATIVE_LINUX_MAGIC (I386MAGIC ^ IMAGE_FILE_MACHINE_NATIVE_LINUX_OVERRIDE)
+#define I386_NATIVE_APPLE_MAGIC (I386MAGIC ^ IMAGE_FILE_MACHINE_NATIVE_APPLE_OVERRIDE)
+
 #define I386BADMAG(x) (  ((x).f_magic != I386MAGIC) \
+		       && (x).f_magic != I386_NATIVE_LINUX_MAGIC \
+		       && (x).f_magic != I386_NATIVE_APPLE_MAGIC \
 		       && (x).f_magic != I386AIXMAGIC \
 		       && (x).f_magic != I386PTXMAGIC \
 		       && (x).f_magic != LYNXCOFFMAGIC)
diff --git a/include/coff/pe.h b/include/coff/pe.h
index 85cc331831..c401586b2e 100644
--- a/include/coff/pe.h
+++ b/include/coff/pe.h
@@ -158,6 +158,9 @@
 #define IMAGE_FILE_MACHINE_WCEMIPSV2         0x0169
 #define IMAGE_FILE_MACHINE_AMD64             0x8664
 
+#define IMAGE_FILE_MACHINE_NATIVE_LINUX_OVERRIDE 0x7b79
+#define IMAGE_FILE_MACHINE_NATIVE_APPLE_OVERRIDE 0x4644
+
 #define IMAGE_SUBSYSTEM_UNKNOWN			 0
 #define IMAGE_SUBSYSTEM_NATIVE			 1
 #define IMAGE_SUBSYSTEM_WINDOWS_GUI		 2
diff --git a/include/coff/x86_64.h b/include/coff/x86_64.h
index 3d0e6f085d..d91845bd1b 100644
--- a/include/coff/x86_64.h
+++ b/include/coff/x86_64.h
@@ -28,8 +28,14 @@
 #define COFF_PAGE_SIZE	0x1000
 
 #define AMD64MAGIC	0x8664
+/* Used in .NET DLLs that target a specific platform.  */
+#define AMD64_NATIVE_LINUX_MAGIC (AMD64MAGIC ^ IMAGE_FILE_MACHINE_NATIVE_LINUX_OVERRIDE)
+#define AMD64_NATIVE_APPLE_MAGIC (AMD64MAGIC ^ IMAGE_FILE_MACHINE_NATIVE_APPLE_OVERRIDE)
+
+#define AMD64BADMAG(x)	(((x).f_magic != AMD64MAGIC) \
+                         && ((x).f_magic != AMD64_NATIVE_LINUX_MAGIC) \
+                         && ((x).f_magic != AMD64_NATIVE_APPLE_MAGIC))
 
-#define AMD64BADMAG(x)	((x).f_magic != AMD64MAGIC)
 #define IMAGE_NT_OPTIONAL_HDR64_MAGIC      0x20b
 
 #define OMAGIC          0404    /* Object files, eg as output.  */
-- 
2.21.0
Nick Clifton July 5, 2019, 2:39 p.m. | #3
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...
Omair Majid July 18, 2019, 5:43 p.m. | #4
Hi Nick,

Thanks for all your comments! They have been really helpful.

Nick Clifton <nickc@redhat.com> writes:

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


I see that some other programs - bfdtest1 and bfdtest2 - also work
similarly. I have left them in binutils instead of moving them into
binutils/testsuite.

> Also you need a mechanism to build the gentestdlls

> executable, and to bail out from the test if the executable cannot be created.


The changes to Makefile.am take care of this already, no? I did run
autoreconf locally in both binutils-gdb and in binutils-gdb/binutils. I
did not include the changes in my patch because it causes a large diff,
and I also seem to have a different version of automake (1.16.1 vs 1.15.1).

As I understand it, if gentestdlls fails to build, it fails the entire
binutils build. Is that okay?

Do I have to handle other architectures in the test specially? Will
objdump be able to handle dumping i386 PE/Coff on aarch64 platforms, for
example?

> PS.  I am off on PTO for two weeks, so I will not be able to respond to emails

> until later this month...


Hope you are enjoying your time off!

Cheers,
Omair

--
PGP Key: B157A9F0 (http://pgp.mit.edu/)
Fingerprint = 9DB5 2F0B FD3E C239 E108  E7BD DF99 7AF8 B157 A9F0
From 47f5117e29679216f35d9e104ddb2fdbdfeed940 Mon Sep 17 00:00:00 2001
From: Omair Majid <omajid@redhat.com>

Date: Tue, 25 Jun 2019 18:03:42 -0400
Subject: [PATCH] Handle some PE/COFF files generated by .NET

The System.Runtime.dll files that get shipped with .NET Core 2.1 on
different platforms demonstrate an issue:

    $ objdump -x dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll
    objdump: dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll: file format not recognized

After this fix:

    $ ~/local/binutils/bin/objdump -x dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll
    dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll:   file format pei-x86-64
    dotnet/shared/Microsoft.NETCore.App/2.1.11/System.Runtime.dll
    architecture: i386:x86-64, flags 0x0000012f:
    HAS_RELOC, EXEC_P, HAS_LINENO, HAS_DEBUG, HAS_LOCALS, D_PAGED

    Characteristics 0x2022
        executable
        large address aware
        DLL

    Time/Date               Wed Jun  5 14:49:41 2019
    Magic                   020b    (PE32+)
    ...

These PE files are regular PE/COFF files but have a different value for
IMAGE_FILE_HEADER.Machine to indicate it contains a native image that
targets a non-Windows platform. This value is OS dependent and varies
across systems (linux vs netbsd vs macos).

ECMA-335 does not mention these new values; it still insists that the
only valid value is 0x14c.

This commit also introduces a tool to generate handcrafted dll files
that can be used in tests to verify objdump parsing.
---
 bfd/coffcode.h                              |   8 ++
 bfd/peXXigen.c                              |   7 +
 binutils/Makefile.am                        |   5 +-
 binutils/testsuite/binutils-all/objdump.exp |  29 ++++
 binutils/testsuite/gentestdlls.c            | 151 ++++++++++++++++++++
 include/coff/i386.h                         |  10 ++
 include/coff/pe.h                           |   9 ++
 include/coff/x86_64.h                       |  12 +-
 8 files changed, 228 insertions(+), 3 deletions(-)
 create mode 100644 binutils/testsuite/gentestdlls.c

diff --git a/bfd/coffcode.h b/bfd/coffcode.h
index c67bfbb0e6..6ec6baa9b3 100644
--- a/bfd/coffcode.h
+++ b/bfd/coffcode.h
@@ -2104,6 +2104,10 @@ coff_set_arch_mach_hook (bfd *abfd, void * filehdr)
 #endif
 #ifdef I386MAGIC
     case I386MAGIC:
+    case I386_APPLE_MAGIC:
+    case I386_FREEBSD_MAGIC:
+    case I386_LINUX_MAGIC:
+    case I386_NETBSD_MAGIC:
     case I386PTXMAGIC:
     case I386AIXMAGIC:		/* Danbury PS/2 AIX C Compiler.  */
     case LYNXCOFFMAGIC:
@@ -2112,6 +2116,10 @@ coff_set_arch_mach_hook (bfd *abfd, void * filehdr)
 #endif
 #ifdef AMD64MAGIC
     case AMD64MAGIC:
+    case AMD64_APPLE_MAGIC:
+    case AMD64_FREEBSD_MAGIC:
+    case AMD64_LINUX_MAGIC:
+    case AMD64_NETBSD_MAGIC:
       arch = bfd_arch_i386;
       machine = bfd_mach_x86_64;
       break;
diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
index 77fb4933e7..ee6da6480d 100644
--- a/bfd/peXXigen.c
+++ b/bfd/peXXigen.c
@@ -35,6 +35,13 @@
    "Peering Inside the PE: A Tour of the Win32 Portable Executable
    File Format", MSJ 1994, Volume 9.
 
+   The PE/PEI format is also used by .NET. ECMA-335 describes this:
+
+   "Standard ECMA-335 Common Language Infrastructure (CLI)", 6th Edition, June 2012.
+
+   This is also available at
+   https://www.ecma-international.org/publications/files/ECMA-ST/ECMA-335.pdf.
+
    The *sole* difference between the pe format and the pei format is that the
    latter has an MSDOS 2.0 .exe header on the front that prints the message
    "This app must be run under Windows." (or some such).
diff --git a/binutils/Makefile.am b/binutils/Makefile.am
index 128494ca9e..de93ffa040 100644
--- a/binutils/Makefile.am
+++ b/binutils/Makefile.am
@@ -17,7 +17,7 @@
 # <http://www.gnu.org/licenses/>.
 #
 
-AUTOMAKE_OPTIONS = dejagnu no-dist foreign
+AUTOMAKE_OPTIONS = dejagnu no-dist foreign subdir-objects
 ACLOCAL_AMFLAGS = -I .. -I ../config -I ../bfd
 
 SUBDIRS = doc po
@@ -93,8 +93,9 @@ EXTRA_SCRIPTS = embedspu
 ## Test programs.
 BFDTEST1_PROG = bfdtest1
 BFDTEST2_PROG = bfdtest2
+GENTESTDLLS_PROG = testsuite/gentestdlls
 
-TEST_PROGS = $(BFDTEST1_PROG) $(BFDTEST2_PROG)
+TEST_PROGS = $(BFDTEST1_PROG) $(BFDTEST2_PROG) $(GENTESTDLLS_PROG)
 
 ## We need a special rule to install the programs which are built with
 ## -new, and to rename cxxfilt to c++filt.
diff --git a/binutils/testsuite/binutils-all/objdump.exp b/binutils/testsuite/binutils-all/objdump.exp
index beaf44f954..48098f5c69 100644
--- a/binutils/testsuite/binutils-all/objdump.exp
+++ b/binutils/testsuite/binutils-all/objdump.exp
@@ -732,6 +732,35 @@ if {[is_elf_format]} then {
     remote_file host delete $testfile3
 }
 
+# Test objdump on .NET assemblies (PE files)
+
+proc test_objdump_dotnet_assemblies {} {
+    global OBJDUMP
+    global base_dir
+
+    set test "dotnet-assemblies"
+
+    set got [binutils_run "$base_dir/testsuite/gentestdlls" "tmpdir"]
+    set want "wrote dotnet-linux-x86-64.dll"
+    if ![regexp $want $got] then {
+	fail "$test"
+    }
+
+    set want "file format pei-i386"
+    set got [binutils_run $OBJDUMP "-x tmpdir/simple-i386.dll"]
+    if ![regexp $want $got] then {
+	fail "$test"
+    }
+
+    set want "file format pei-x86-64"
+    set got [binutils_run $OBJDUMP "-x tmpdir/dotnet-linux-x86-64.dll"]
+    if ![regexp $want $got] then {
+	fail "$test"
+    }
+}
+
+test_objdump_dotnet_assemblies
+
 # Options which are not tested: -a -D -R -T -x -l --stabs
 # I don't see any generic way to test any of these other than -a.
 # Tests could be written for specific targets, and that should be done
diff --git a/binutils/testsuite/gentestdlls.c b/binutils/testsuite/gentestdlls.c
new file mode 100644
index 0000000000..bb7d94cdc6
--- /dev/null
+++ b/binutils/testsuite/gentestdlls.c
@@ -0,0 +1,151 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GNU Binutils.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA
+   02110-1301, USA.  */
+
+
+/* This file generates a number of DLL (PE/COFF binaries traditionally
+   used on Windows) that we can then utilize in various tests to
+   ensure objdump can parse these file correctly.
+
+   See:
+   https://www.ecma-international.org/publications/files/ECMA-ST/ECMA-335.pdf  */
+
+#include <memory.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+static void
+write_dos_header_and_stub (FILE* file)
+{
+  /* 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] =
+    {
+     0x4d, 0x5a, 0x90, 0x00, 0x03, 0x00, 0x00, 0x00,
+     0x04, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0x00, 0x00,
+     0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+     0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+     0x00, 0x00, 0x00, 0x00, 0x80, 0x00, 0x00, 0x00, /* Last 4 bytes are precomputed lfanew.  */
+     0x0e, 0x1f, 0xba, 0x0e, 0x00, 0xb4, 0x09, 0xcd,
+     0x21, 0xb8, 0x01, 0x4c, 0xcd, 0x21, 0x54, 0x68,
+     0x69, 0x73, 0x20, 0x70, 0x72, 0x6f, 0x67, 0x72,
+     0x61, 0x6d, 0x20, 0x63, 0x61, 0x6e, 0x6e, 0x6f,
+     0x74, 0x20, 0x62, 0x65, 0x20, 0x72, 0x75, 0x6e,
+     0x20, 0x69, 0x6e, 0x20, 0x44, 0x4f, 0x53, 0x20,
+     0x6d, 0x6f, 0x64, 0x65, 0x2e, 0x0d, 0x0d, 0x0a,
+     0x24, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+    };
+
+  fwrite (buffer, 1, 128, file);
+}
+
+static void
+write_pe_signature (FILE* file)
+{
+  char buffer[4];
+  buffer[0] = 'P';
+  buffer[1] = 'E';
+  buffer[2] = 0;
+  buffer[3] = 0;
+  fwrite (buffer, 1, 4, file);
+}
+
+static void
+write_coff_header (FILE* file, uint16_t machine)
+{
+  char buffer[128];
+  memset (buffer, 0, sizeof (buffer));
+
+  /* Machine.  ECMA-335 says this must be 0x14c but that's not true anymore.  */
+  buffer[0] = machine & 0xff;
+  buffer[1] = machine >> 0x8;
+  fwrite (buffer, 2, 1, file);
+  memset (buffer, 0, sizeof (buffer));
+  /* NumberOfSections = 0 */
+  fwrite (buffer, 2, 1, file);
+  /* TimeDateStamp = 0 */
+  fwrite (buffer, 4, 1, file);
+  /* PointerToSymbolTable = 0 */
+  fwrite (buffer, 4, 1, file);
+  /* NumberOfSymbols = 0 */
+  fwrite (buffer, 4, 1, file);
+  /* OptionalHeaderSize = 0 */
+  fwrite (buffer, 2, 1, file);
+  /* Characteristics = 0x2000 */
+  buffer[0] = 0x00;
+  buffer[1] = 0x20;
+  fwrite (buffer, 2, 1, file);
+  memset (buffer, 0 , sizeof (buffer));
+}
+
+int
+main(int argc, char** argv)
+{
+  if (argc < 2)
+    {
+      fprintf (stderr, "usage: %s output-directory\n", argv[0]);
+      exit (2);
+    }
+  if (chdir (argv[1]) != 0)
+    {
+      fprintf (stderr, "error: unable to change directory to %s\n", argv[0]);
+      exit (2);
+    }
+
+  FILE* file;
+
+  /* Generate a simple DLL file.  */
+  file = fopen ("simple-i386.dll", "w");
+  if (file == NULL)
+    {
+      fprintf (stderr, "error: unable to open file for writing\n");
+      exit (1);
+    }
+
+  write_dos_header_and_stub (file);
+  write_pe_signature (file);
+  write_coff_header (file, 0x14c);
+  fclose (file);
+  printf ("wrote simple-i386.dll\n");
+
+  /* Generate a sample .NET Core on Linux dll file.  As opposed to the
+     more common DLLs that contain bytecode (CIL/MSIL), many .NET Core
+     DLLs are pre-compiled for specific architectures and platforms.
+     See https://github.com/jbevain/cecil/issues/337 for an example of
+     this value being used in practice.  */
+  file = fopen ("dotnet-linux-x86-64.dll", "w");
+  if (file == NULL)
+    {
+      fprintf (stderr, "error: unable to open file for writing\n");
+      exit (1);
+    }
+
+  write_dos_header_and_stub (file);
+  write_pe_signature (file);
+  write_coff_header (file, 0xfd1d /* x86-64 + Linux */);
+  fclose (file);
+  printf ("wrote dotnet-linux-x86-64.dll\n");
+
+  return 0;
+}
diff --git a/include/coff/i386.h b/include/coff/i386.h
index 1d2ccff6f7..9ea7ddfa8a 100644
--- a/include/coff/i386.h
+++ b/include/coff/i386.h
@@ -43,7 +43,17 @@
 
 #define LYNXCOFFMAGIC	0415
 
+/* Used in some .NET DLLs that target a specific OS.  */
+#define I386_APPLE_MAGIC (I386MAGIC ^ IMAGE_FILE_MACHINE_NATIVE_APPLE_OVERRIDE)
+#define I386_FREEBSD_MAGIC (I386MAGIC ^ IMAGE_FILE_MACHINE_NATIVE_FREEBSD_OVERRIDE)
+#define I386_LINUX_MAGIC (I386MAGIC ^ IMAGE_FILE_MACHINE_NATIVE_LINUX_OVERRIDE)
+#define I386_NETBSD_MAGIC (I386MAGIC ^ IMAGE_FILE_MACHINE_NATIVE_NETBSD_OVERRIDE)
+
 #define I386BADMAG(x) (  ((x).f_magic != I386MAGIC) \
+		       && (x).f_magic != I386_APPLE_MAGIC \
+		       && (x).f_magic != I386_FREEBSD_MAGIC \
+		       && (x).f_magic != I386_LINUX_MAGIC \
+		       && (x).f_magic != I386_NETBSD_MAGIC \
 		       && (x).f_magic != I386AIXMAGIC \
 		       && (x).f_magic != I386PTXMAGIC \
 		       && (x).f_magic != LYNXCOFFMAGIC)
diff --git a/include/coff/pe.h b/include/coff/pe.h
index 85cc331831..8b3251e587 100644
--- a/include/coff/pe.h
+++ b/include/coff/pe.h
@@ -158,6 +158,15 @@
 #define IMAGE_FILE_MACHINE_WCEMIPSV2         0x0169
 #define IMAGE_FILE_MACHINE_AMD64             0x8664
 
+/* .NET DLLs XOR the Machine number (above) with an override to
+    indicate that the DLL contains OS-specific machine code rather
+    than just IL or bytecode. See
+    https://github.com/dotnet/coreclr/blob/6f7aa7967c607b8c667518314ab937c0d7547025/src/inc/pedecoder.h#L94-L107. */
+#define IMAGE_FILE_MACHINE_NATIVE_APPLE_OVERRIDE 0x4644
+#define IMAGE_FILE_MACHINE_NATIVE_FREEBSD_OVERRIDE 0xadc4
+#define IMAGE_FILE_MACHINE_NATIVE_LINUX_OVERRIDE 0x7b79
+#define IMAGE_FILE_MACHINE_NATIVE_NETBSD_OVERRIDE 0x1993
+
 #define IMAGE_SUBSYSTEM_UNKNOWN			 0
 #define IMAGE_SUBSYSTEM_NATIVE			 1
 #define IMAGE_SUBSYSTEM_WINDOWS_GUI		 2
diff --git a/include/coff/x86_64.h b/include/coff/x86_64.h
index 3d0e6f085d..5dde1337d5 100644
--- a/include/coff/x86_64.h
+++ b/include/coff/x86_64.h
@@ -28,8 +28,18 @@
 #define COFF_PAGE_SIZE	0x1000
 
 #define AMD64MAGIC	0x8664
+/* Used in some .NET DLLs that target a specific OS.  */
+#define AMD64_APPLE_MAGIC (AMD64MAGIC ^ IMAGE_FILE_MACHINE_NATIVE_APPLE_OVERRIDE)
+#define AMD64_FREEBSD_MAGIC (AMD64MAGIC ^ IMAGE_FILE_MACHINE_NATIVE_FREEBSD_OVERRIDE)
+#define AMD64_LINUX_MAGIC (AMD64MAGIC ^ IMAGE_FILE_MACHINE_NATIVE_LINUX_OVERRIDE)
+#define AMD64_NETBSD_MAGIC (AMD64MAGIC ^ IMAGE_FILE_MACHINE_NATIVE_NETBSD_OVERRIDE)
+
+#define AMD64BADMAG(x) (   ((x).f_magic != AMD64MAGIC) \
+                        && ((x).f_magic != AMD64_APPLE_MAGIC) \
+                        && ((x).f_magic != AMD64_FREEBSD_MAGIC) \
+                        && ((x).f_magic != AMD64_LINUX_MAGIC) \
+                        && ((x).f_magic != AMD64_NETBSD_MAGIC))
 
-#define AMD64BADMAG(x)	((x).f_magic != AMD64MAGIC)
 #define IMAGE_NT_OPTIONAL_HDR64_MAGIC      0x20b
 
 #define OMAGIC          0404    /* Object files, eg as output.  */
-- 
2.21.0
Nick Clifton July 23, 2019, 8:50 a.m. | #5
Hi Omair,

  Thanks for the revised patch.  I have now checked it in, although
  I did have to make a couple of changes:  The binutils test now copes
  with targets which do not recognise the PE file format, and the
  additions to the header files will now work on targets which only
  support 32-bit PE format files.

Cheers
  Nick

PS.  I also created some changelog entries for you:

include/ChangeLog
2019-07-23  Omar Majid  <omajid@redhat.com>

	* coff/i386.h (IMAGE_FILE_MACHINE_NATIVE_APPLE_OVERRIDE): Define.
	(IMAGE_FILE_MACHINE_NATIVE_FREEBSD_OVERRIDE): Define.
	(IMAGE_FILE_MACHINE_NATIVE_LINUX_OVERRIDE): Define.
	(IMAGE_FILE_MACHINE_NATIVE_NETBSD_OVERRIDE): Define.
	(I386_APPLE_MAGIC): Define.
	(I386_FREEBSD_MAGIC): Define.
	(I386_LINUX_MAGIC): Define.
	(I386_NETBSD_MAGIC): Define.
	(I386BADMAG): Extend macro to allow new magic numbers.
	* coff/x86_64.h (IMAGE_FILE_MACHINE_NATIVE_APPLE_OVERRIDE): Define.
	(IMAGE_FILE_MACHINE_NATIVE_FREEBSD_OVERRIDE): Define.
	(IMAGE_FILE_MACHINE_NATIVE_LINUX_OVERRIDE): Define.
	(IMAGE_FILE_MACHINE_NATIVE_NETBSD_OVERRIDE): Define.
	(AMD64_APPLE_MAGIC): Define.
	(AMD64_FREEBSD_MAGIC): Define.
	(AMD64_LINUX_MAGIC): Define.
	(AMD64_NETBSD_MAGIC): Define.
	(AMD64BADMAG): Extend macro to allow new magic numbers.

bfd/Changelog
2019-07-23  Omar Majid  <omajid@redhat.com>

	* coffcode.h (coff_set_arch_mach_hook): Handle I386_APPLE_MAGIC,
	I386_FREEBSD_MAGIC, I386_LINUX_MAGIC, I386_NETBSD_MAGIC,
	AMD64_APPLE_MAGIC, AMD64_FREEBSD_MAGIC, AMD64_LINUX_MAGIC,
	AMD64_NETBSD_MAGIC.
	* peXXigen.c: Add comment about source of .NET magic numbers.

binutils/ChangeLog
2019-07-23  Omar Majid  <omajid@redhat.com>

	* Makefile.am (AUTOMAKE_OPTIONS): Add subdir-objects
	(GENTESTDLLSPROG): Define.
	(TEST_PROGS): Add GENTESTDLLSPROG.
	* Makefile.in: Regenerate.
	* testsuite/binutils-all/objdump.exp
	(test_objdump_dotnet_assemblies): New proc.
	Run the new proc.
	* testsuite/gentestdlls.c: New source file.
Omair Majid July 23, 2019, 2:23 p.m. | #6
Hi Nick,

Nick Clifton <nickc@redhat.com> writes:

>   Thanks for the revised patch.  I have now checked it in


Thanks for checking it in! I was expecting a few more rounds of patches,
so this was a *very* nice surprise!

>   although

>   I did have to make a couple of changes:  The binutils test now copes

>   with targets which do not recognise the PE file format


This might be a stupid question, but how does the test deal with
regressions in dll parsing? It seems to me like the test would pass even
if objudmp can't parse System.Runtime.dll. The "file format not
recognized" is the same error that a objdump built without the fix
prints out:

$ objdump -x /usr/lib64/dotnet/shared/Microsoft.NETCore.App/2.1.12/System.Runtime.dll
objdump: /usr/lib64/dotnet/shared/Microsoft.NETCore.App/2.1.12/System.Runtime.dll: file format not recognized

>   and the

>   additions to the header files will now work on targets which only

>   support 32-bit PE format files.


Ah, thank you. Is there a way I can discover this myself (for next
time)? Would ./configure --enable-targets=all flag this for me?

> PS.  I also created some changelog entries for you:


Apologies. I was expecting a few more rounds of patches so I figured I
could be a bit sloppy and skip this. Sorry.

Regards,
Omair

--
PGP Key: B157A9F0 (http://pgp.mit.edu/)
Fingerprint = 9DB5 2F0B FD3E C239 E108  E7BD DF99 7AF8 B157 A9F0
Nick Clifton July 23, 2019, 2:28 p.m. | #7
Hi Omair,

> This might be a stupid question,


Actually it is not stupid at all...

> but how does the test deal with

> regressions in dll parsing? It seems to me like the test would pass even

> if objudmp can't parse System.Runtime.dll. The "file format not

> recognized" is the same error that a objdump built without the fix

> prints out:

> 

> $ objdump -x /usr/lib64/dotnet/shared/Microsoft.NETCore.App/2.1.12/System.Runtime.dll

> objdump: /usr/lib64/dotnet/shared/Microsoft.NETCore.App/2.1.12/System.Runtime.dll: file format not recognized


Well bananas.  I had not considered that.

I think that what we need to do is to extend gentestdlls.c so that
it generates a normal x86 PE dll file as well.  Then the test can
be extended to check this file first.  If the target objdump is
able to parse this file, then it should be able to parse the other
two files as well.  If it cannot then there is an error.  (Actually
32-bit PE toolchains may only be able to parse one of the new .NET
PE dlls).


>>   additions to the header files will now work on targets which only

>>   support 32-bit PE format files.

> 

> Ah, thank you. Is there a way I can discover this myself (for next

> time)? Would ./configure --enable-targets=all flag this for me?


Nope.  You need a target that includes the 32-bit PE header but not
the 64-bit PE header.  I used "./configure --target=i686-pc-cygwin".

Cheers
  Nick

Patch

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 0783242758..a0710531ac 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,9 @@ 
+2019-06-26  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.
+
 2019-06-25  Jan Beulich  <jbeulich@suse.com>
 
 	* elf-properties.c (elf_find_and_remove_property): Rename last
diff --git a/bfd/coffcode.h b/bfd/coffcode.h
index c67bfbb0e6..94f0bacc4a 100644
--- a/bfd/coffcode.h
+++ b/bfd/coffcode.h
@@ -2104,6 +2104,8 @@  coff_set_arch_mach_hook (bfd *abfd, void * filehdr)
 #endif
 #ifdef I386MAGIC
     case I386MAGIC:
+    case I386_NATIVE_LINUX_MAGIC:
+    case I386_NATIVE_APPLE_MAGIC:
     case I386PTXMAGIC:
     case I386AIXMAGIC:		/* Danbury PS/2 AIX C Compiler.  */
     case LYNXCOFFMAGIC:
@@ -2112,6 +2114,8 @@  coff_set_arch_mach_hook (bfd *abfd, void * filehdr)
 #endif
 #ifdef AMD64MAGIC
     case AMD64MAGIC:
+    case AMD64_NATIVE_LINUX_MAGIC:
+    case AMD64_NATIVE_APPLE_MAGIC:
       arch = bfd_arch_i386;
       machine = bfd_mach_x86_64;
       break;
diff --git a/include/ChangeLog b/include/ChangeLog
index 81b6670668..cf841f74f5 100644
--- a/include/ChangeLog
+++ b/include/ChangeLog
@@ -1,3 +1,14 @@ 
+2019-06-26  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.
+
 2019-06-19  Nick Alcock <nick.alcock@oracle.com>
 
 	* ctf.h (ctf_slice_t): Make cts_offset and cts_bits unsigned
diff --git a/include/coff/i386.h b/include/coff/i386.h
index 1d2ccff6f7..2879072b3d 100644
--- a/include/coff/i386.h
+++ b/include/coff/i386.h
@@ -43,7 +43,13 @@ 
 
 #define LYNXCOFFMAGIC	0415
 
+/* Used in .NET DLLs that target non-Windows platforms */
+#define I386_NATIVE_LINUX_MAGIC (I386MAGIC ^ IMAGE_FILE_MACHINE_NATIVE_LINUX_OVERRIDE)
+#define I386_NATIVE_APPLE_MAGIC (I386MAGIC ^ IMAGE_FILE_MACHINE_NATIVE_APPLE_OVERRIDE)
+
 #define I386BADMAG(x) (  ((x).f_magic != I386MAGIC) \
+		       && (x).f_magic != I386_NATIVE_LINUX_MAGIC \
+		       && (x).f_magic != I386_NATIVE_APPLE_MAGIC \
 		       && (x).f_magic != I386AIXMAGIC \
 		       && (x).f_magic != I386PTXMAGIC \
 		       && (x).f_magic != LYNXCOFFMAGIC)
diff --git a/include/coff/pe.h b/include/coff/pe.h
index 85cc331831..c401586b2e 100644
--- a/include/coff/pe.h
+++ b/include/coff/pe.h
@@ -158,6 +158,9 @@ 
 #define IMAGE_FILE_MACHINE_WCEMIPSV2         0x0169
 #define IMAGE_FILE_MACHINE_AMD64             0x8664
 
+#define IMAGE_FILE_MACHINE_NATIVE_LINUX_OVERRIDE 0x7b79
+#define IMAGE_FILE_MACHINE_NATIVE_APPLE_OVERRIDE 0x4644
+
 #define IMAGE_SUBSYSTEM_UNKNOWN			 0
 #define IMAGE_SUBSYSTEM_NATIVE			 1
 #define IMAGE_SUBSYSTEM_WINDOWS_GUI		 2
diff --git a/include/coff/x86_64.h b/include/coff/x86_64.h
index 3d0e6f085d..74947dc5c5 100644
--- a/include/coff/x86_64.h
+++ b/include/coff/x86_64.h
@@ -28,8 +28,14 @@ 
 #define COFF_PAGE_SIZE	0x1000
 
 #define AMD64MAGIC	0x8664
+/* Used in .NET DLLs that target non-Windows platforms */
+#define AMD64_NATIVE_LINUX_MAGIC (AMD64MAGIC ^ IMAGE_FILE_MACHINE_NATIVE_LINUX_OVERRIDE)
+#define AMD64_NATIVE_APPLE_MAGIC (AMD64MAGIC ^ IMAGE_FILE_MACHINE_NATIVE_APPLE_OVERRIDE)
+
+#define AMD64BADMAG(x)	(((x).f_magic != AMD64MAGIC) \
+                         && ((x).f_magic != AMD64_NATIVE_LINUX_MAGIC) \
+                         && ((x).f_magic != AMD64_NATIVE_APPLE_MAGIC))
 
-#define AMD64BADMAG(x)	((x).f_magic != AMD64MAGIC)
 #define IMAGE_NT_OPTIONAL_HDR64_MAGIC      0x20b
 
 #define OMAGIC          0404    /* Object files, eg as output.  */