This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch ld]: Enable.pei386_auto_import for i?86-*-mingw* and x86_64-*-mingw* by default


2010/5/15 Dave Korn <dave.korn.cygwin@googlemail.com>:
> On 15/05/2010 12:05, Kai Tietz wrote:
>
>> by investigating this issue in more detail, I opened a cane of worms.
>
> ?You replied this to the wrong thread! ?Anyway ...
>
>> I prepared a patch to address this issue and tested it for x86_64 and
>> i686 targets. I put the logic for initialization of
>> pe(p)_leading_underscore into a function for not having multiple times
>> code at many places.
>
> ?This is definitely nicer. ?It's kinda awkward that we have to remember to
> call is_underscoring for its side-effect of conditioning the global
> pe_leading_underscore variable, but I see you put an abort in pe-dll.c to
> protect us against it going wrong so I guess it's not so bad.
>
>> * emultempl/pe.em (is_leading): New helper function.
>> Replace internal use of pe(p)_leading_underscore by is_leading function.
>> * emultemp/pep.em: Likewise.
>> * pe-dll.c: Change default underscoring for x64 target.
>
> ?The patch is OK, but please adjust your changelog to the standard format
> before checking it in: you have 'is_leading' instead of 'is_underscoring', the
> leading tab and line wrapping needs adding, you need to make a separate entry
> for each function where you "replaced internal use ... by is_leading
> function", and the pe-dll.c changes likewise should have one named item per
> line in brackets for each of the things you changed.

I committed test after futther testings with changelog

2010-05-15  Kai Tietz

	* emultempl/pe.em (is_underscoring): New helper function.
	(gld_${EMULATION_NAME}_before_parse): Replace code
	for pe(p)_leading_underscore detection by is_underscoring.
	(U): Likewise.
	(GET_INIT_SYMBOL_NAME): Likewise.
	(U_SIZE): Likewise.
	(set_pe_name):
	(set_entry_point):
	(gld_${EMULATION_NAME}_set_symbols):
	* emultempl/pep.em: Likewise.
	* pe-dll.c (pe_detail_list): Set default
	underscoring for x64 target.
	(pe_dll_id_target): Add initialization of
	pe(p)_leading_underscore.

> ?Also, because I think I remember us being asked once before not to mix
> functional patches and white-space tidyups in the same commit, I committed the
> whitespace trimming to pe-dll.c for you. ?Apologies in advance if that gives
> you merge problems, but I thought it was probably for the best.

Thanks for doing this.

Thanks for fast review.

>> PS: I noticed an regression on 4.6 about collect2. Everything works
>> find in ld (I've debugged collect2 call of ld and checked
>> output-files), but gcc frontend reports a failure in DLL compile. Did
>> you noticed the same?
>
> ?No, but I haven't built 4.6 this week, I've been busy. ?I'll do a build over
> the weekend and let you know if I see any problems. ?If it's a mingw-only
> problem, I'd check if there have been any recent changes to the pex stuff in
> libiberty first off.

Yeah, it seems to be new to 4.6 gcc. For 4.5 I don't get this issue.

Regards,
Kai

PS: The TLS patch looks good. For backward compatibility it is better
to remain those dots.

-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]