Bug 30079 - Mingw ld : linking against an import lib causes ld to abort() at compare_section in ldlang.c
Summary: Mingw ld : linking against an import lib causes ld to abort() at compare_sect...
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.39
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-02-04 15:19 UTC by Didier Smets
Modified: 2023-02-13 15:13 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
archive containing the four files needed to reproduce the bug (17.91 KB, application/gzip)
2023-02-04 15:19 UTC, Didier Smets
Details
object file + lib (10.30 KB, application/x-tar)
2023-02-07 18:12 UTC, Christoph Reiter
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Didier Smets 2023-02-04 15:19:51 UTC
Created attachment 14653 [details]
archive containing the four files needed to reproduce the bug

After an upgrade of my Debian machine (which for binutils means 2.35 -> 2.39), I have encountered the following issue while building a project. Debian packager has asked me to report upstream.

I have been able to trace the issue so that it can reproduced in a very simple test
case (all needed files included as attachments - the datarefs.c is a tiny test one, the XPLM_64.lib is a third party import lib).

Test:

Untar the attached archive (one C source + one import lib and corresponding two header files) into a directory of your choice and issue :

/usr/bin/x86_64-w64-mingw32-gcc -shared -o datarefs.dll  datarefs.c -L . -l :XPLM_64.lib

Yields :

/usr/bin/x86_64-w64-mingw32-ld: internal error: aborting at ./upstream/ld/ldlang.c:527 in compare_section
/usr/bin/x86_64-w64-mingw32-ld: please report this bug
collect2: error: ld returned 1 exit status

Notes after testing:

1) It works fine in Debian Bullseye (binutils-mingw 2.35).
2) It works fine in the non mingw build  (I mean with .so rather than .dll output, but of course no import lib is needed in that case).
3) It works fine in mingw binutils 2.39 if in datarefs.c I comment out _all but one_ (any of them) function.

Exact Debian package causing the issue:
Debian unstable package : binutils-mingw-w64-x86-64 2.39.90.20230110-1+10.3
Comment 1 Christoph Reiter 2023-02-07 11:41:42 UTC
Using the reproducer bisected to:

I can also confirm that reverting it on top of 2.40 makes things work again (assuming I resolved the conflicts correctly) .

---

af31506c31a59a6edbb13498d6075fa704b801cd is the first bad commit
commit af31506c31a59a6edbb13498d6075fa704b801cd
Author: Michael Matz <matz@suse.de>
Date:   Thu Nov 10 16:06:20 2022 +0100

    Only use wild_sort_fast

    there's no reason why the tree-based variant can't always be used
    when sorting is required, it merely needs to also support filename
    sorting and have a fast path for insertion at end (aka rightmost tree
    leaf).

    The filename sorting isn't tested anywhere and the only scripttempl
    that uses it is avr (for 'SORT(*)(.ctors)'), and I believe even there it
    was a mistake.  Either way, this adds a testcase for filename sorting as
    well.

    Then the non-BST based sorting can be simplified to only support
    the fast case of no sorting required at all (at the same time renaming
    the two variants to _sort and _nosort).

 ld/ldlang.c                          | 302 +++++++++++++++--------------------
 ld/ldlang.h                          |   3 +-
 ld/testsuite/ld-scripts/sort-file.d  |  18 +++
 ld/testsuite/ld-scripts/sort-file.t  |   6 +
 ld/testsuite/ld-scripts/sort-file1.s |   6 +
 ld/testsuite/ld-scripts/sort-file2.s |   6 +
 6 files changed, 163 insertions(+), 178 deletions(-)
 create mode 100644 ld/testsuite/ld-scripts/sort-file.d
 create mode 100644 ld/testsuite/ld-scripts/sort-file.t
 create mode 100644 ld/testsuite/ld-scripts/sort-file1.s
 create mode 100644 ld/testsuite/ld-scripts/sort-file2.s
Comment 2 Christoph Reiter 2023-02-07 16:48:22 UTC
It looks like the guards before calling compare_section() were dropped in the new sorting code compared to the old one: https://github.com/bminor/binutils-gdb/commit/af31506c31a59a6edbb13498d6075fa704b801cd#diff-a914d0473272b436f9df11b7baced0716e595c73daca00d118a733bf3d915b02L2866-L2870

For extra context, here is a downstream bug report: https://github.com/msys2/MINGW-packages/issues/15469
Comment 3 Michael Matz 2023-02-07 17:10:33 UTC
The guards are supposed to be moved only, not removed:

  if (!wild->filenames_sorted
      && (sec == NULL || sec->spec.sorted == none
	  || sec->spec.sorted == by_none))
    {
      return wild->rightmost;
    }
  ...
  while (*tree) {
    if (wild->filenames_sorted) {
      ...
      if (i > 0) ... continue; else if (i < 0) ... continue;
      if (fa || la) {
        ...
        if (i > 0) ... continue else if (i < 0) ... continue;
      }
    }
    ...
    compare_section (sec->spec.sorted, section, (*tree)->section) < 0)
  ...

So it could only get to calling compare_section() (with sec->spec.sorted being none or by_none) when ->filename_sorted _and_ it falls through the compare
based on these names (i.e. if filenames are equal and containing archives (if
they exist) are equal).

I probably convinced myself that this situation is not supposed to happen,
and as testcases were totally missing I didn't notice otherwise.

If you can tar up the object files I can take a look myself (I don't have 
cross compilers here, but cross binutils).  Or you add the guard
back again and see how that goes.
Comment 4 Christoph Reiter 2023-02-07 18:12:29 UTC
Created attachment 14658 [details]
object file + lib
Comment 5 Christoph Reiter 2023-02-07 18:15:37 UTC
(In reply to Michael Matz from comment #3)
> If you can tar up the object files I can take a look myself (I don't have 
> cross compilers here, but cross binutils).

See above, if you want :)

> Or you add the guard
> back again and see how that goes.

I did that in https://github.com/msys2/MINGW-packages/pull/15540 now which fixed it for this example. Once it's in the repo I can ask the affected users to try again.

thanks!
Comment 6 Christoph Reiter 2023-02-12 14:51:51 UTC
(In reply to Christoph Reiter from comment #5)
> I did that in https://github.com/msys2/MINGW-packages/pull/15540 now which
> fixed it for this example. Once it's in the repo I can ask the affected
> users to try again.

All affected downstream users reported that it's working again for them.
Comment 7 Michael Matz 2023-02-13 13:06:58 UTC
(In reply to Christoph Reiter from comment #6)
> (In reply to Christoph Reiter from comment #5)
> > I did that in https://github.com/msys2/MINGW-packages/pull/15540 now which
> > fixed it for this example. Once it's in the repo I can ask the affected
> > users to try again.
> 
> All affected downstream users reported that it's working again for them.

Thanks.  After pondering I think it's the right thing to do, the input is sane and it's (of course) fine that sections are only sorted by filename and not otherwise, so that case needs dealing with.  I've committed a patch for this.