Fix PR ld/24574

Message ID 2353376.kH97uzeIrP@polaris
State New
Headers show
Series
  • Fix PR ld/24574
Related show

Commit Message

Eric Botcazou Sept. 5, 2019, 3:56 p.m.
Hi,

when I overhauled the auto-import feature of the PE-COFF linker about one year 
ago, I dropped a line in pe_find_data_imports because it was surrounded by a 
slightly scary comment:

-	      /* We replace original name with __imp_ prefixed, this
-		 1) may trash memory 2) leads to duplicate symbol generation.
-		 Still, IMHO it's better than having name polluted.  */
-	      undef->root.string = sym->root.string;

As reported in the PR, the __imp_ prefix is needed in DLLs by GDB to recognize 
that this is an import symbol and not the real one when the symbol is extern, 
so the attached patch simply puts the line back.

Tested on i686-pc-mingw32, OK for mainline, 2.32 and 2.31 branches?


2019-09-05  Eric Botcazou  <ebotcazou@adacore.com>

ld/
	PR ld/24574
	* pe-dll.c (pe_find_data_imports): Replace again the original name of
	the undefined symbol with the __imp_ prefixed one after it is resolved.

-- 
Eric Botcazou

Comments

Nick Clifton Sept. 5, 2019, 4:01 p.m. | #1
Hi Eric,

> Tested on i686-pc-mingw32, OK for mainline, 2.32 and 2.31 branches?


> 2019-09-05  Eric Botcazou  <ebotcazou@adacore.com>

> 

> ld/

> 	PR ld/24574

> 	* pe-dll.c (pe_find_data_imports): Replace again the original name of

> 	the undefined symbol with the __imp_ prefixed one after it is resolved.


Approved for all three.  Please apply.

Cheers
  Nick
Alan Modra Sept. 6, 2019, 12:10 a.m. | #2
On Thu, Sep 05, 2019 at 05:56:57PM +0200, Eric Botcazou wrote:
> slightly scary comment:

> 

> -	      /* We replace original name with __imp_ prefixed, this

> -		 1) may trash memory 2) leads to duplicate symbol generation.

> -		 Still, IMHO it's better than having name polluted.  */

> -	      undef->root.string = sym->root.string;


"may trash memory" is alarmist.  Replacing a pointer to a string
(typically in memory containing a copy of .strtab or .dynstr) with a
pointer to another string won't "trash memory".

-- 
Alan Modra
Australia Development Lab, IBM
Eric Botcazou Sept. 6, 2019, 7:24 a.m. | #3
> "may trash memory" is alarmist.  Replacing a pointer to a string

> (typically in memory containing a copy of .strtab or .dynstr) with a

> pointer to another string won't "trash memory".


Yes, that's probably the reason why I initially removed the line.  I guess 
that ideally we would need to make this previously undefined symbol hidden
or anonymous if this is possible.  For the time being the code reads:

	    /* Let's differentiate it somehow from defined.  */
	    undef->type = bfd_link_hash_defweak;
	    undef->u.def.value = sym->u.def.value;
	    undef->u.def.section = sym->u.def.section;

	    /* We replace the original name with the __imp_ prefixed one, this
	       1) may trash memory 2) leads to duplicate symbols.  But this is
	       better than having a misleading name that can confuse GDB.  */
	    undef->root.string = sym->root.string;

-- 
Eric Botcazou

Patch

diff --git a/ld/pe-dll.c b/ld/pe-dll.c
index 81ab116c46..577b911da8 100644
--- a/ld/pe-dll.c
+++ b/ld/pe-dll.c
@@ -1445,6 +1445,11 @@  pe_find_data_imports (const char *symhead,
 	    undef->u.def.value = sym->u.def.value;
 	    undef->u.def.section = sym->u.def.section;
 
+	    /* We replace the original name with the __imp_ prefixed one, this
+	       1) may trash memory 2) leads to duplicate symbols.  But this is
+	       better than having a misleading name that can confuse GDB.  */
+	    undef->root.string = sym->root.string;
+
 	    if (link_info.pei386_auto_import == -1)
 	      {
 		static bfd_boolean warned = FALSE;