Bug 26588 - enable-reloc-section on .exe with no exported symbols causes segfault if requested to create implib
Summary: enable-reloc-section on .exe with no exported symbols causes segfault if requ...
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.36
: P2 critical
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks: 19011 17321
  Show dependency treegraph
 
Reported: 2020-09-08 19:00 UTC by sourceware-bugzilla
Modified: 2020-09-30 09:53 UTC (History)
1 user (show)

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


Attachments
proposed patch (359 bytes, patch)
2020-09-08 19:00 UTC, sourceware-bugzilla
Details | Diff
proposed patch (729 bytes, patch)
2020-09-10 18:42 UTC, sourceware-bugzilla
Details | Diff
patch allowing empty import library for dll (741 bytes, patch)
2020-09-24 19:32 UTC, sourceware-bugzilla
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description sourceware-bugzilla 2020-09-08 19:00:15 UTC
Created attachment 12824 [details]
proposed patch

PR #17321 added --enable-reloc-section, and PR #19011 made it default.  However, some (misguided) tools try to create import libraries for executables, even if they don't export any symbols.  This didn't cause a failure previously, but now segfaults if --enable-reloc-section is enabled.

0:000> kb
 # RetAddr           : Args to Child                                                           : Call Site
00 00007ff6`f8aa4907 : 00000000`00000000 00000001`40023760 0000003c`5dbff9ec 00000000`00000044 : msvcrt!strlen+0x31
01 00007ff6`f898f53b : 00000000`00000000 00007ff6`f897618a 0000003c`5dbff9f0 00007ff6`f89680f0 : ld!xstrdup+0x17 [..\..\binutils-2.35\libiberty\xstrdup.c @ 33] 
02 00007ff6`f8980a08 : 000001a9`af6758a0 000001a9`ad6d88b0 00007ff6`f8c618c0 00007ff6`f89680f0 : ld!pep_dll_generate_implib+0x53 [..\..\binutils-2.35\ld\pe-dll.c @ 2822] 
03 00007ff6`f8975c27 : 0000003c`5dbffabc 0000000a`af68bd50 00000000`00000025 00000001`00000044 : ld!gld_i386pep_finish+0xb1 [ei386pep.c @ 1823] 
04 00007ff6`f896ab7d : 00000000`00000004 00007ff6`00000001 00000000`00000000 00007ff6`f8976050 : ld!ldemul_finish+0x15 [..\..\binutils-2.35\ld\ldemul.c @ 102] 
05 00007ff6`f896f22e : 00007ff6`f8b14608 00007ff6`f8b067c0 000001a9`00000001 00007ffc`50000061 : ld!lang_process+0x6c6 [..\..\binutils-2.35\ld\ldlang.c @ 8126] 
06 00007ff6`f89513c1 : 00000000`00000025 000001a9`ad6d1b40 00007ff6`f8c67398 00000000`00000000 : ld!main+0x795 [..\..\binutils-2.35\ld\ldmain.c @ 498] 
07 00007ff6`f89514f6 : 00000000`00000000 00000000`00000000 00000000`00000000 00000000`00000000 : ld!__tmainCRTStartup+0x231 [D:\mingwbuild\mingw-w64-crt-git\src\mingw-w64\mingw-w64-crt\crt\crtexe.c @ 337] 
08 00007ffc`a86b6fd4 : 00000000`00000000 00000000`00000000 00000000`00000000 00000000`00000000 : ld!mainCRTStartup+0x16 [D:\mingwbuild\mingw-w64-crt-git\src\mingw-w64\mingw-w64-crt\crt\crtexe.c @ 221] 
09 00007ffc`a9b1cec1 : 00000000`00000000 00000000`00000000 00000000`00000000 00000000`00000000 : KERNEL32!BaseThreadInitThunk+0x14
0a 00000000`00000000 : 00000000`00000000 00000000`00000000 00000000`00000000 00000000`00000000 : ntdll!RtlUserThreadStart+0x21


This is

  dll_filename = (def->name) ? def->name : dll_name;
  dll_symname = xstrdup (dll_filename);


the global dll_name is filled in if def->name is not set in generate_edata, however 

  if (pe_def_file->num_exports == 0 && !bfd_link_pic (info))
    {
      if (pe_dll_enable_reloc_section)
	{
	  build_filler_bfd (0);
	  pe_output_file_set_long_section_names (filler_bfd);
	}
      return;
    }

  generate_edata (abfd, info);
  build_filler_bfd (1);
  pe_output_file_set_long_section_names (filler_bfd);


I'm not sure whether it would be better to call generate_edata in the no exports but enable reloc section case, or my proposed patch (attached)
Comment 1 sourceware-bugzilla 2020-09-09 23:27:09 UTC
(In reply to sourceware-bugzilla from comment #0)
> I'm not sure whether it would be better to call generate_edata in the no
> exports but enable reloc section case, or my proposed patch (attached)

It seems that with --disable-reloc-section ld simply does not create the requested implib, so I believe my proposed patch would solve this regression.
Comment 2 sourceware-bugzilla 2020-09-10 18:42:57 UTC
Created attachment 12835 [details]
proposed patch

Updated patch, against master (instead of 2.35), git-formatted rather than bare, and fixing a copy-paste-o (pep vs pe in pe.em).
Comment 3 Sourceware Commits 2020-09-11 16:52:31 UTC
The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=9cdc5bacddc3776a455cc918e034b01dfb31c75b

commit 9cdc5bacddc3776a455cc918e034b01dfb31c75b
Author: Jeremy Drake <sourceware-bugzilla@jdrake.com>
Date:   Fri Sep 11 17:51:16 2020 +0100

    Fix a segfault when creating an import library with 0 exports.
    
            PR 26588
            * emultempl/pe.em (_finish): Only generate a import library if not
            exporting relocs.
            * emultempl/pep.em: Likewise.
Comment 4 Nick Clifton 2020-09-11 16:54:11 UTC
Hi Jeremy,

  Thanks for reporting this problem, and providing a patch to fix.
  (Also please accept my apologies for not repsonding sooner).

  Anyway I have now checked in your patch, so this issue should be resolved.

Cheers
  Nick
Comment 5 sourceware-bugzilla 2020-09-11 17:12:18 UTC
Thanks.

I am confused by your ChangeLog text: "Only generate a import library if not exporting relocs."  Perhaps should be more like "Only generate an import library if exporting symbols."
Comment 6 sourceware-bugzilla 2020-09-14 21:45:04 UTC
I've gotten a report that a DLL that exports no symbols no longer generates an import library, as previously expected.

I hesitate to add more conditions to that inner if, though that's the most obvious solution.
Comment 7 sourceware-bugzilla 2020-09-24 19:32:56 UTC
Created attachment 12864 [details]
patch allowing empty import library for dll

After preventing creating an import library for an exe when there are no exports, to avoid a crash, it turned out that some projects expected to be able to create an import library for a dll with no exports, so more closely match the condition to the condition around initializing the dll name.
Comment 8 Sourceware Commits 2020-09-30 09:51:19 UTC
The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=51dee44b37c4254e6b6a016b062c23b8e1f9ca33

commit 51dee44b37c4254e6b6a016b062c23b8e1f9ca33
Author: Jeremy Drake <sourceware-bugzilla@jdrake.com>
Date:   Wed Sep 30 10:50:46 2020 +0100

    After preventing creating an import library for an exe when there are no exports, to avoid a crash, it turned out that some projects expected to be able to create an import library for a dll with no exports, so more closely match the condition to the condition around initializing the dll name.
    
            PR 26588
            * emultempl/pe.em (_finish): Generate an import library for DLLs,
            even if they have no exports.
            * emultempl/pep.em (_finish): Likewise.
Comment 9 Nick Clifton 2020-09-30 09:53:30 UTC
Patch applied.  (Sorry about the delay).