[Bug ld/19541] Format reader for ILF format (used by MSVC-generated import libraries) does not properly handle data imports

2016-02-05 Thread njs at pobox dot com
https://sourceware.org/bugzilla/show_bug.cgi?id=19541

--- Comment #5 from Nathaniel J. Smith  ---
Can the test suite include binary files? Two tests I can think of would be:

1) Include the ILF file; run objdump on it; confirm that the appropriate
symbols appear.

2) Include the ILF file + a regular .o that references a data member in it; use
ld to link them together and then use objdump to confirm that the resulting dll
has a correct import table.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
bug-binutils mailing list
bug-binutils@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-binutils


[Bug ld/19541] Format reader for ILF format (used by MSVC-generated import libraries) does not properly handle data imports

2016-01-31 Thread njs at pobox dot com
https://sourceware.org/bugzilla/show_bug.cgi?id=19541

--- Comment #3 from Nathaniel J. Smith  ---
Created attachment 8946
  --> https://sourceware.org/bugzilla/attachment.cgi?id=8946&action=edit
Tiny ILF file with DATA symbol for testing

Attaching an example ILF-format import member, extracted from a 32-bit
python27.lib (as created by MSVC, and distributed in the python.org official
python 2.7 windows downloads).

Without the patch, nm on this file shows no symbols; after the patch, it
correctly shows a reference to __imp___Py_NoneStruct. Also, here's a simple
test case:

 nonestruct-test.c 

__declspec(dllimport) struct PyObject_Struct _Py_NoneStruct;
struct PyObject_Struct *f()
{
return &_Py_NoneStruct;
}

 end nonestruct-test.c 

Without the patch, linking this program to export-_Py_NoneStruct.o fails with:

  undefined reference to `_imp___Py_NoneStruct'

but with the patch, linking succeeds and correctly creates an import table
referencing _Py_NoneStruct.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
bug-binutils mailing list
bug-binutils@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-binutils


[Bug ld/19541] Format reader for ILF format (used by MSVC-generated import libraries) does not properly handle data imports

2016-01-31 Thread njs at pobox dot com
https://sourceware.org/bugzilla/show_bug.cgi?id=19541

--- Comment #2 from Nathaniel J. Smith  ---
Update: on further investigation my helpful tester reports that their segfault
problem was an unrelated configuration error that they made; once they sorted
that out then all was well.

So, I'm now fairly confident that the attached patch does in fact solve the
problem.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
bug-binutils mailing list
bug-binutils@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-binutils


[Bug ld/19541] Format reader for ILF format (used by MSVC-generated import libraries) does not properly handle data imports

2016-01-30 Thread njs at pobox dot com
https://sourceware.org/bugzilla/show_bug.cgi?id=19541

--- Comment #1 from Nathaniel J. Smith  ---
Created attachment 8943
  --> https://sourceware.org/bugzilla/attachment.cgi?id=8943&action=edit
First attempt at a patch

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
bug-binutils mailing list
bug-binutils@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-binutils


[Bug ld/19541] New: Format reader for ILF format (used by MSVC-generated import libraries) does not properly handle data imports

2016-01-30 Thread njs at pobox dot com
https://sourceware.org/bugzilla/show_bug.cgi?id=19541

Bug ID: 19541
   Summary: Format reader for ILF format (used by MSVC-generated
import libraries) does not properly handle data
imports
   Product: binutils
   Version: unspecified
Status: NEW
  Severity: normal
  Priority: P2
 Component: ld
  Assignee: unassigned at sourceware dot org
  Reporter: njs at pobox dot com
  Target Milestone: ---

On Windows, there are two formats for "import libraries" commonly in use: the
format generated by dlltool and commonly used with mingw-w64 toolchains, which
is an ar archive of COFF objects (conventionally named like "libpython27.a"),
and the format generated and commonly used by MSVC toolchains, which is an ar
archive of "ILF" objects (and conventionally named like "python27.lib").

Either way, the purpose of such files is to provide DLL imports. DLL imports
come in two main flavors. There are "code" symbols, which refer to functions,
and which require (a) a __imp___foo relocation symbol, and (b) _foo trampoline
that just jumps to __imp___foo. And then there are "data" symbols, which refer
to things like global variables exported from a DLL, and only require a
__imp___foo symbol, with no trampoline symbol. In the ILF format these are
represented identically, except that there is a flag value labeling each import
symbol as either IMPORT_CODE or IMPORT_DATA.

Unfortunately, binutils's ILF-reading code currently only supports IMPORT_CODE
symbols. Any symbols with the IMPORT_DATA flag set are completely skipped over,
as if they didn't exist.

One place where this causes problems is when attempting to use binutils to link
programs against the python C API, because python27.dll makes extensive use of
DATA symbols. But really it would be nice to support these in general, because
it should be simple to do and it would be nice if we didn't have to jump
through hoops to use import libraries on Windows. In addition, mingw-w64 is
looking to transition to a new 'genlib' tool for generating import libraries,
and genlib also generates ILF-format libraries.

I've attached a patch that in theory ought to fix this. Basically, right now
all the code for emitting import symbol information from ILF files is hidden
behind switch { case IMPORT_CODE: ...}; my patch just takes the parts that are
common to IMPORT_CODE and IMPORT_DATA and moves them out of the switch
statement so that they are executed unconditionally. The function-specific
trampoline generation code remains inside the switch statement, and if we see
anything besides IMPORT_CODE or IMPORT_DATA then we abort(), so this should be
safe.

The patch compiles, and using it to manually link a simple test program against
python27.lib produces correct-looking output (i.e., a binary is successfully
produced, and running objdump on it shows that it has a correct-looking import
table that successfully references DATA symbols in python27.dll). Unfortunately
though I don't have a great way to test this directly (no handy Windows system,
and no convenient way to build a whole toolchain including my patched ld), and
someone else reported that when they did all that then the compile seemed to
work but the resulting binary segfaulted. (Possibly due to unrelated issues
though, who knows.)

So, I'm posting this here in hopes that either someone who knows what they're
doing will look at it and think "well, this is obviously correct", or else
think "well, this is obviously broken, you need to do _" :-)

Some discussion where we found this bug:
https://github.com/mingwpy/mingwpy.github.io/issues/31#issuecomment-175334085

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
bug-binutils mailing list
bug-binutils@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-binutils