public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [patch ld]: Testsuite fix of vers-script-3 and vers-script-4 tests
@ 2011-02-15 12:42 Kai Tietz
  2011-02-15 15:30 ` Kai Tietz
  0 siblings, 1 reply; 7+ messages in thread
From: Kai Tietz @ 2011-02-15 12:42 UTC (permalink / raw)
  To: Binutils; +Cc: Dave Korn

[-- Attachment #1: Type: text/plain, Size: 271 bytes --]

Hello,

ChangeLog

2011-02-15  Kai Tietz

        * ld-pe/vers-script-3.ver: Add _Z* to global and use
        wildcard for personality symbols.
        * ld-pe/vers-script-4.ver: Likewise.

Tested for x86_64-w64-mingw32 and i686-w64-mingw32. Ok for apply?

Regards,
Kai

[-- Attachment #2: ld_i386_testsuite.txt --]
[-- Type: text/plain, Size: 1153 bytes --]

Index: testsuite/ld-pe/vers-script-3.ver
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-pe/vers-script-3.ver,v
retrieving revision 1.1
diff -u -3 -r1.1 vers-script-3.ver
--- testsuite/ld-pe/vers-script-3.ver	4 May 2009 12:09:30 -0000	1.1
+++ testsuite/ld-pe/vers-script-3.ver	15 Feb 2011 12:37:35 -0000
@@ -2,6 +2,6 @@
 # symbols in libgcj.so.
 
 {
-  global: Jv*; _Jv_*; __gcj_personality_v0; __gcj_personality_sj0; _Z*;
+  global: Jv*; _Jv*; *gcj_personality_v0; *gcj_personality_sj0; Z*; _Z*;
   local: *;
 };
Index: testsuite/ld-pe/vers-script-4.ver
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-pe/vers-script-4.ver,v
retrieving revision 1.1
diff -u -3 -r1.1 vers-script-4.ver
--- testsuite/ld-pe/vers-script-4.ver	4 May 2009 12:09:30 -0000	1.1
+++ testsuite/ld-pe/vers-script-4.ver	15 Feb 2011 12:37:35 -0000
@@ -6,5 +6,5 @@
 };
 
 TEST_1_1 {
-  global: Jv*; _Jv_*; __gcj_personality_v0; __gcj_personality_sj0; _Z*;
-};
\ No newline at end of file
+  global: Jv*; _Jv*; *gcj_personality_v0; *gcj_personality_sj0; Z*; _Z*;
+};

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch ld]: Testsuite fix of vers-script-3 and vers-script-4 tests
  2011-02-15 12:42 [patch ld]: Testsuite fix of vers-script-3 and vers-script-4 tests Kai Tietz
@ 2011-02-15 15:30 ` Kai Tietz
  2011-02-15 15:55   ` Dave Korn
  0 siblings, 1 reply; 7+ messages in thread
From: Kai Tietz @ 2011-02-15 15:30 UTC (permalink / raw)
  To: Binutils; +Cc: Dave Korn

[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]

2011/2/15 Kai Tietz <ktietz70@googlemail.com>:
> Hello,
>
> ChangeLog
>
> 2011-02-15  Kai Tietz
>
>        * ld-pe/vers-script-3.ver: Add _Z* to global and use
>        wildcard for personality symbols.
>        * ld-pe/vers-script-4.ver: Likewise.
>
> Tested for x86_64-w64-mingw32 and i686-w64-mingw32. Ok for apply?
>
> Regards,
> Kai
>

Ok, I withdraw recent patch. It is just fixing the symptoms but not
the underlying issue.
For 32-bit COFF the real issue is in pe-dll.c file in function
process_def_file_and_drectve. Here the underscore of a symbol is
stripped, and the same time it is done in bfd_demangle, which is used
by bfd_find_version_for_sym. By this in 32-bit case with leading
underscores, they were stripped twice.

ChangeLog

2011-02-15  Kai Tietz

          * pe-dll.c (process_def_file_and_drectve): Don't strip
          leading underscore from symbol by calling bfd_find_version_for_sym.

Tested for x86_64-w64-mingw32, i686-pc-cygwin, and i686-w64-mingw32.
Ok for apply?

Regards,
Kai

[-- Attachment #2: pe_dll.txt --]
[-- Type: text/plain, Size: 622 bytes --]

Index: pe-dll.c
===================================================================
RCS file: /cvs/src/src/ld/pe-dll.c,v
retrieving revision 1.133
diff -u -3 -r1.133 pe-dll.c
--- pe-dll.c	27 Jun 2010 04:07:54 -0000	1.133
+++ pe-dll.c	15 Feb 2011 15:23:17 -0000
@@ -720,9 +720,8 @@
 	      if (lang_elf_version_info && would_export)
 		{
 		  bfd_boolean hide = 0;
-		  char ofs = pe_details->underscored && symbols[j]->name[0] == '_';
 		  (void) bfd_find_version_for_sym (lang_elf_version_info,
-				symbols[j]->name + ofs, &hide);
+				symbols[j]->name, &hide);
 		  would_export = !hide;
 		}
 	      if (would_export)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch ld]: Testsuite fix of vers-script-3 and vers-script-4 tests
  2011-02-15 15:30 ` Kai Tietz
@ 2011-02-15 15:55   ` Dave Korn
  2011-02-15 16:03     ` Dave Korn
  2011-02-15 16:08     ` Kai Tietz
  0 siblings, 2 replies; 7+ messages in thread
From: Dave Korn @ 2011-02-15 15:55 UTC (permalink / raw)
  To: Kai Tietz; +Cc: Binutils

On 15/02/2011 15:30, Kai Tietz wrote:

> Ok, I withdraw recent patch. It is just fixing the symptoms but not
> the underlying issue.
> For 32-bit COFF the real issue is in pe-dll.c file in function
> process_def_file_and_drectve. Here the underscore of a symbol is
> stripped, and the same time it is done in bfd_demangle, which is used
> by bfd_find_version_for_sym. By this in 32-bit case with leading
> underscores, they were stripped twice.
> 
> ChangeLog
> 
> 2011-02-15  Kai Tietz
> 
>           * pe-dll.c (process_def_file_and_drectve): Don't strip
>           leading underscore from symbol by calling bfd_find_version_for_sym.
> 
> Tested for x86_64-w64-mingw32, i686-pc-cygwin, and i686-w64-mingw32.
> Ok for apply?

  Nope.  This causes two regressions on i686-pc-cygwin:

> FAIL: vers-script-3
> FAIL: vers-script-4

  They pass currently.  They fail with your patch.  I'm reasonably sure that I
only added that underscore-stripping code when I discovered it to be
necessary, but I may not have tested it on a non-prefixing host.

  I don't understand why you want to remove the underscore stripping code to
solve a problem on w64, where it should never be active in any case?

    cheers,
      DaveK

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch ld]: Testsuite fix of vers-script-3 and vers-script-4 tests
  2011-02-15 15:55   ` Dave Korn
@ 2011-02-15 16:03     ` Dave Korn
  2011-02-15 16:07       ` Dave Korn
  2011-02-15 16:08     ` Kai Tietz
  1 sibling, 1 reply; 7+ messages in thread
From: Dave Korn @ 2011-02-15 16:03 UTC (permalink / raw)
  To: Kai Tietz; +Cc: Binutils

On 15/02/2011 15:55, Dave Korn wrote:

>   Nope.  This causes two regressions on i686-pc-cygwin:
> 
>> FAIL: vers-script-3
>> FAIL: vers-script-4
> 
>   They pass currently.  They fail with your patch.  


Ah, hang on: I just updated my sandbox, and I have found a suspect change:

> 2011-02-14  Mike Frysinger  <...
> 
> 	* ldlang.c (lang_vers_match): Declare a new c_sym, assign it to
> 	the bfd_demangle of sym, change users of sym to c_sym when not
> 	already demangling, and free when done.  Change callers of
> 	cplus_demangle to bfd_demangle.


  This is likely to have changed the behaviour I guess.

    cheers,
      DaveK

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch ld]: Testsuite fix of vers-script-3 and vers-script-4 tests
  2011-02-15 16:03     ` Dave Korn
@ 2011-02-15 16:07       ` Dave Korn
  0 siblings, 0 replies; 7+ messages in thread
From: Dave Korn @ 2011-02-15 16:07 UTC (permalink / raw)
  To: Kai Tietz; +Cc: Binutils

On 15/02/2011 16:03, Dave Korn wrote:
> On 15/02/2011 15:55, Dave Korn wrote:
> 
>>   Nope.  This causes two regressions on i686-pc-cygwin:
>>
>>> FAIL: vers-script-3
>>> FAIL: vers-script-4
>>   They pass currently.  They fail with your patch.  
> 
> 
> Ah, hang on: I just updated my sandbox, and I have found a suspect change:
> 
>> 2011-02-14  Mike Frysinger  <...
>>
>> 	* ldlang.c (lang_vers_match): Declare a new c_sym, assign it to
>> 	the bfd_demangle of sym, change users of sym to c_sym when not
>> 	already demangling, and free when done.  Change callers of
>> 	cplus_demangle to bfd_demangle.
> 
> 
>   This is likely to have changed the behaviour I guess.

  Now reading the recent "version scripts and default/C language mangling" and
"[PATCH] bfd/ld: handle ABI prefixes in version scripts" threads.. looks like
the core handling was made language independent by adding demangling, so
removing the pseudo-demangling code in binutils is the correct solution.
(I've been a couple of days behind HEAD trying to debug a patchset for the ld
plugin api.)

  Patch is OK after all, thanks!

    cheers,
      DaveK

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch ld]: Testsuite fix of vers-script-3 and vers-script-4 tests
  2011-02-15 15:55   ` Dave Korn
  2011-02-15 16:03     ` Dave Korn
@ 2011-02-15 16:08     ` Kai Tietz
  2011-02-15 16:16       ` Kai Tietz
  1 sibling, 1 reply; 7+ messages in thread
From: Kai Tietz @ 2011-02-15 16:08 UTC (permalink / raw)
  To: Dave Korn; +Cc: Binutils

2011/2/15 Dave Korn <dave.korn.cygwin@gmail.com>:
> On 15/02/2011 15:30, Kai Tietz wrote:
>
>> Ok, I withdraw recent patch. It is just fixing the symptoms but not
>> the underlying issue.
>> For 32-bit COFF the real issue is in pe-dll.c file in function
>> process_def_file_and_drectve. Here the underscore of a symbol is
>> stripped, and the same time it is done in bfd_demangle, which is used
>> by bfd_find_version_for_sym. By this in 32-bit case with leading
>> underscores, they were stripped twice.
>>
>> ChangeLog
>>
>> 2011-02-15  Kai Tietz
>>
>>           * pe-dll.c (process_def_file_and_drectve): Don't strip
>>           leading underscore from symbol by calling bfd_find_version_for_sym.
>>
>> Tested for x86_64-w64-mingw32, i686-pc-cygwin, and i686-w64-mingw32.
>> Ok for apply?
>
>  Nope.  This causes two regressions on i686-pc-cygwin:
>
>> FAIL: vers-script-3
>> FAIL: vers-script-4
>
>  They pass currently.  They fail with your patch.  I'm reasonably sure that I
> only added that underscore-stripping code when I discovered it to be
> necessary, but I may not have tested it on a non-prefixing host.
>
>  I don't understand why you want to remove the underscore stripping code to
> solve a problem on w64, where it should never be active in any case?
>
>    cheers,
>      DaveK
>
>

Well, for none-underscored code I see no regressions. I see it for
32-bit mingw and cygwin. Have you updated your tree? I assume it is
related to some indirect changes.

For me the vers-script-3 and vers-script-4 tests in pe-ld are failing
at the moment in an unpatched variant.

Kai

PS: See
2011-02-14  Mike Frysinger  <vapier@gentoo.org>

        * ldlang.c (lang_vers_match): Declare a new c_sym, assign it to
        the bfd_demangle of sym, change users of sym to c_sym when not
        already demangling, and free when done.  Change callers of
        cplus_demangle to bfd_demangle.

This could be the culprit here.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch ld]: Testsuite fix of vers-script-3 and vers-script-4 tests
  2011-02-15 16:08     ` Kai Tietz
@ 2011-02-15 16:16       ` Kai Tietz
  0 siblings, 0 replies; 7+ messages in thread
From: Kai Tietz @ 2011-02-15 16:16 UTC (permalink / raw)
  To: Dave Korn; +Cc: Binutils

2011/2/15 Kai Tietz <ktietz70@googlemail.com>:
> 2011/2/15 Dave Korn <dave.korn.cygwin@gmail.com>:
>> On 15/02/2011 15:30, Kai Tietz wrote:
>>
>>> Ok, I withdraw recent patch. It is just fixing the symptoms but not
>>> the underlying issue.
>>> For 32-bit COFF the real issue is in pe-dll.c file in function
>>> process_def_file_and_drectve. Here the underscore of a symbol is
>>> stripped, and the same time it is done in bfd_demangle, which is used
>>> by bfd_find_version_for_sym. By this in 32-bit case with leading
>>> underscores, they were stripped twice.
>>>
>>> ChangeLog
>>>
>>> 2011-02-15  Kai Tietz
>>>
>>>           * pe-dll.c (process_def_file_and_drectve): Don't strip
>>>           leading underscore from symbol by calling bfd_find_version_for_sym.
>>>
>>> Tested for x86_64-w64-mingw32, i686-pc-cygwin, and i686-w64-mingw32.
>>> Ok for apply?
>>
>>  Nope.  This causes two regressions on i686-pc-cygwin:
>>
>>> FAIL: vers-script-3
>>> FAIL: vers-script-4
>>
>>  They pass currently.  They fail with your patch.  I'm reasonably sure that I
>> only added that underscore-stripping code when I discovered it to be
>> necessary, but I may not have tested it on a non-prefixing host.
>>
>>  I don't understand why you want to remove the underscore stripping code to
>> solve a problem on w64, where it should never be active in any case?
>>
>>    cheers,
>>      DaveK
>>
>>
>
> Well, for none-underscored code I see no regressions. I see it for
> 32-bit mingw and cygwin. Have you updated your tree? I assume it is
> related to some indirect changes.
>
> For me the vers-script-3 and vers-script-4 tests in pe-ld are failing
> at the moment in an unpatched variant.
>
> Kai
>
> PS: See
> 2011-02-14  Mike Frysinger  <vapier@gentoo.org>
>
>        * ldlang.c (lang_vers_match): Declare a new c_sym, assign it to
>        the bfd_demangle of sym, change users of sym to c_sym when not
>        already demangling, and free when done.  Change callers of
>        cplus_demangle to bfd_demangle.
>
> This could be the culprit here.
>

Applied.

Thanks,
Kai

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2011-02-15 16:16 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-15 12:42 [patch ld]: Testsuite fix of vers-script-3 and vers-script-4 tests Kai Tietz
2011-02-15 15:30 ` Kai Tietz
2011-02-15 15:55   ` Dave Korn
2011-02-15 16:03     ` Dave Korn
2011-02-15 16:07       ` Dave Korn
2011-02-15 16:08     ` Kai Tietz
2011-02-15 16:16       ` Kai Tietz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).