[RFA] [pei-386] prevent ld (auto-import) from generating broken code

[RFA] [pei-386] prevent ld (auto-import) from generating broken code

[Charles Wilson]

This patch includes a (slightly) modified version of Paul Sokolovsky's 
fix for this bug, and updates to the ld.texinfo file to explain the new 
behavior.

The bug: accessing, from client code, a complex variable imported from a 
DLL (an array or struct; possibly others) with a constant (nonzero) 
offset leads to incorrect "fixups" --- e.g. a[2] is the same as a[0] if 
array 'a' is imported from a DLL.  s.secondfield is the same as 
s.firstfield if struct 's' is imported from a DLL.

The compiled client code has a nonzero addend, but the windows loader 
can't seem to handle "additional" offsets when doing the dynamic 
loading/relocations.

The fix: Paul's patch identifies these problematic accesses (if they 
exist; seems to be rare), prints out the following message, and aborts. 
(Actually, my modification to Paul's patch merely switches the order of 
the two suggested remedies in this message):

stringtest.o: In function `main':
/usr/src/binutils/tmp/stringtest.c:9: aggregate 'hwstr1' is referenced 
in direct addressing mode with constant offset - auto-import failed. 
Workarounds: a) use a 'volatile' auxilliary variable; b) mark the symbol 
with __declspec(dllimport)
collect2: ld returned 1 exit status

ld.texinfo is updated to provide more information on this bug/message, 
and elaborate on the possible workarounds.

See this thread:

[aida_s@mx12.freecom.ne.jp: A serious bug of "ld --enable-auto-import"]
http://sources.redhat.com/ml/binutils/2001-08/msg00595.html

and this one

Re: Serious bug in auto-import
http://sources.redhat.com/ml/binutils/2001-09/msg00126.html

--chuck

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[DJ Delorie]

I wouldn't recommend using volatile, because it won't always work.  If
they need to adjust their sources anyway, they should use the
dllimport syntax.

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[Charles Wilson]

DJ Delorie wrote:

> I wouldn't recommend using volatile, because it won't always work. 


so you're saying that in some cases, gcc will ignore the 'volatile' 
marker, and replace the variable with a constant?  Why?  Isn't 
preventing that "optimization" exactly what the volatile modifier is for?

> If
> they need to adjust their sources anyway, they should use the
> dllimport syntax.



Ugh.  As I said earlier, that's almost as bad as the original situation. 
  Now you need special compile time flags to match your link time flags, 
depending on whether you will link to a static lib or dynamic lib, or 
whether you have dynamic libs installed or only static ones...

But only if your client code does some esoteric thing; these #defines 
aren't necessary for most client code...

Blech.

Are you *sure*

{ volatile type t=extern_array; t[1] = ... }
and
{ volatile int t=constant_index; extern_array[t] = ... }
and
{ volatile struct s * t = &extern_struct; t->field = ... }

won't always work (given auto-import support and thunking symbols in the 
DLL)?  Why not?

--Chuck

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[DJ Delorie]

> Are you *sure*

Suggesting a complex workaround using #defines is worse than just
having them do the right thing in the first place.  And I suspect
people would get confused and try "extern volatile struct foo f;"
which won't work.

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[Charles Wilson]

DJ Delorie wrote:

>>Are you *sure*
>>
> 
> Suggesting a complex workaround using #defines is worse than just
> having them do the right thing in the first place.  

I'm sorry, I should have been more clear.

Sure, if all you want to do is "link this client code to that DLL" -- 
then yeah, just put

"__declspec(dllimport)" in the DLL's header file which declares the 
variable you want to import.  Or skip the header files completely; just 
declare imported variables yourself in the client code (BAD idea. 
Completely unmaintainable)

Of course, if you want that header file to ALSO work on OTHER platforms, 
you'd need to do something like:

#if defined(__CYGWIN__) || defined(_WIN32)
# define STUPID_WIN32_DECORATOR __declspec(dllimport)
#else
# define STUPID_WIN32_DECORATOR
#endif
extern STUPID_WIN32_DECORATOR int foo

But, if you want the same header file to be usable both when BUILDING 
and when LINKING to the DLL, you'd need to add the following (assuming 
that you don't want to use __declspec(dllexport); just rely on 
--export-all-symbols.  If you DO want to use __declspec(dllexport) when 
building the DLL, it gets even uglier)

#if defined(__CYGWIN__) || defined(_WIN32) && !defined(BUILDING_DLL)
# define STUPID_WIN32_DECORATOR __declspec(dllimport)
#else
# define STUPID_WIN32_DECORATOR
#endif
extern STUPID_WIN32_DECORATOR int foo

But now, let's suppose you want to support both static libraries AND 
DLL's -- on cygwin/win32 -- but using the same header file to declare 
your exported (struct? array?)  Then you need to add another flag to 
your library's header file:

#if defined(__CYGWIN__) || defined(_WIN32) && \
   ! (defined(BUILDING_DLL) || defined(STATIC_LINK))
# define STUPID_WIN32_DECORATOR __declspec(dllimport)
#else
# define STUPID_WIN32_DECORATOR
#endif
extern STUPID_WIN32_DECORATOR int foo

It's these defines I'm talking about:  -DBUILDING_DLL when building a 
dll, -DSTATIC_LINK when building a static library or when building a 
client .o that will be linked to a static library, or (no special 
defines) when building a .o that will be linked to the DLL.

That's "blech".  And unfortunately, the ugliness is absolutely necessary 
to handle the __declsec junk in your library's header files, if you want 
to allow (a) compatibility with other platforms (b) building either 
static or DLL (c) linking to either static or DLL.

 > And I suspect
 > people would get confused and try "extern volatile struct foo f;"
 > which won't work.


Right.  That's why the extra documentation that I added to elaborate on 
this issue said to:

declare a local variable that was volatile
for structs, that local variable should be a pointer to struct, not a 
struct itself.
change struct.field references to local_var->field references.

Okay, so I didn't spell it out that way -- I gave examples that obeyed 
those rules.

Are you saying I need better documentation to help prevent people from 
making the mistake, or are you dead set against anything except 
__declspec?  (Keeping in mind the hell of interlinked #defines 
__declspec requires, if you want to maintain cross-platform compatiblity 
and static/DLL choice)

--Chuck

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[DJ Delorie]

I guess there's no easy way to get people to get it right.  I would
still argue for the dllimport path, though, if for no other reason
than [I'm guessing] it's what MS developers do.

And the volatile imposes a performance penalty on non-dll cases too,
trivial though it may be.

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[Christopher Faylor]

On Tue, Sep 11, 2001 at 03:37:20PM -0400, DJ Delorie wrote:
>I guess there's no easy way to get people to get it right.  I would
>still argue for the dllimport path, though, if for no other reason
>than [I'm guessing] it's what MS developers do.
>
>And the volatile imposes a performance penalty on non-dll cases too,
>trivial though it may be.

Can I ask what the possibility of a real fix for this might be?

Paul, are you actively looking into this?  Is it even possible to
fix or is this just a problem with the windows loadder?

cgf

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[DJ Delorie]

> Paul, are you actively looking into this?  Is it even possible to
> fix or is this just a problem with the windows loadder?

It's not possible to fix it to do what we want.  GCC creates
relocations against imported data objects that aren't representable in
a Win32 import table.

The only hope is to modify gcc to not allow offsets in direct
addressing, but that would (1) be a big performance penalty, and (2)
probably not be accepted by the gcc folks.

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[Charles Wilson]

DJ Delorie wrote:

> I guess there's no easy way to get people to get it right.  I would
> still argue for the dllimport path, though, if for no other reason
> than [I'm guessing] it's what MS developers do.


Yeah, but they do it because they don't have any other choice. <g> MS's 
link.exe doesn't have anything like your own function-auto-import 
capability (virtual on-the-fly import lib), either.  But we still use it.

> 
> And the volatile imposes a performance penalty on non-dll cases too,
> trivial though it may be.
> 


Okay, this may be flamebait, but at what point do you balance the 
tradeoff between ease-of-porting and performance?  In the case of 
cygwin, it's solidly on the side of ease-of-porting: a native port of 
"foo.exe" would definitely be faster than a cygwin version, but it's 
MUCH easier to port "foo" from unix to cygwin than to native-MS.

Since we're talking about libraries, there is a ripple effect. 
Requiring strange "extra" defines, coordinating compile-time options 
with link-time options....all of these things are possible, but 
extremely painful, both for developers and maintainers of these ported 
libraries.

I know, because I maintain about 12 or so "dll-ized" libraries for 
cygwin.  I've made these kinds of ugly changes to the headers, with only 
limited success:

1) many users are often confused by these interlinked requirements; a 
good number of the "bug reports" that I get are due to a 
misunderstanding of these issues.  (Yes, the requirements are all 
fully-documented in cygwin-port-specific README files, but...)

2) most of the package development teams are reluctant to accept the 
necessary patches.  E.g. the readline patch to enable combined 
DLL/static building on cygwin, while maintaining original behavior on 
other platforms, is over 200k.  The ncurses patch is over 450k.  This 
has met with understandable resistance. :-)

3) Because of (1) and (2), there are significant problems when packages 
include local copies of the source for dependent libraries 
(bash-readline, "anything"-gettext, etc.)  Since the installed versions 
of the libraries' header files differ from these included-source-code 
versions, you end up with all kinds of "cross-pollination" problems.


Anything that makes sharedlib/staticlib behavior more "unix-like" is 
good -- because the MS model just plain stinks.  Anything that 
eliminates the need for ugly header file hacks is good, IMO, even at the 
expense of a "trivial" performance penalty.

--Chuck

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[Charles Wilson]

Charles Wilson wrote:


> 
>>
>> And the volatile imposes a performance penalty on non-dll cases too,
>> trivial though it may be.
>>
> 
> 
> Okay, this may be flamebait, but at what point do you balance the 
> tradeoff between ease-of-porting and performance?  In the case of 
> cygwin, it's solidly on the side of ease-of-porting: a native port of 
> "foo.exe" would definitely be faster than a cygwin version, but it's 
> MUCH easier to port "foo" from unix to cygwin than to native-MS.
> 


Oh yeah -- and remember, we're talking about a pretty esoteric thing. 
First, this particular type of access doesn't seem to occur very often 
in real code (otherwise, we would have run into this problem *sometime* 
in the previous *year* of tests....)

Second, we're merely arguing about which of two alternatives to 
*recommend*, while still presenting BOTH alternatives -- in the 
documentation.  (Unless you're arguing for eliminating all references to 
my preferred alternative?)

--Chuck

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[DJ Delorie]

> Second, we're merely arguing about which of two alternatives to 
> *recommend*, while still presenting BOTH alternatives -- in the 
> documentation.  (Unless you're arguing for eliminating all references to 
> my preferred alternative?)

Would it be reasonable to refer to the manual in the error message?  I
think we can document all the known workarounds (and dwarbacks of
each), but we can't do it clearly if we're limited to doing it in the
error message.

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[Charles Wilson]

DJ Delorie wrote:

 >> Second, we're merely arguing about which of two alternatives to 
*recommend*,
 >>  while still presenting BOTH alternatives -- in the  documentation.
 >>   (Unless you're arguing for eliminating all references to  my
 >> preferred alternative?)
 >>
 >
 > Would it be reasonable to refer to the manual in the error message?
 >  I think we can document all the known workarounds (and dwarbacks of 
each), but we can't do it clearly if we're limited to doing it in the
 > error message.
 >
 >


stringtest.o: In function `main':
/usr/src/binutils/tmp/stringtest.c:9: aggregate 'hwstr1' is referenced 
in direct addressing mode with constant offset - auto-import failed. 
Workarounds: a) use a 'volatile' auxilliary variable; b) mark the symbol 
with __declspec(dllimport)
collect2: ld returned 1 exit status



 > > each), but we can't do it clearly if we're limited to doing it in
 >  the error message.
 >
 >

Currently, the message reads:

stringtest.o: In function `main':
/usr/src/binutils/tmp/stringtest.c:9: aggregate 'hwstr1' is referenced 
in direct addressing mode with constant offset - auto-import failed. 
Workarounds: a) use a 'volatile' auxilliary variable; b) mark the symbol 
with __declspec(dllimport)
collect2: ld returned 1 exit status


Perhaps this:

stringtest.o: In function `main':
/usr/src/binutils/tmp/stringtest.c:9: aggregate 'hwstr1' is referenced 
in direct addressing mode with constant offset - auto-import failed. 
Workarounds: a) use a 'volatile' auxilliary variable; b) mark the symbol 
with __declspec(dllimport).  For more information, see 
ld.info::Invocation::Options::--enable-auto-import
collect2: ld returned 1 exit status

?

--Chuck

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[DJ Delorie]

Ok, I'm cynical of the average user.  How about this:

stringtest.o: In function `main':
/usr/src/binutils/tmp/stringtest.c:9: variable 'hwstr1' can't be
auto-imported.  Please read the documentation for ld's
--enable-auto-import for details.

By not hinting at workarounds in the message, we force them to read
the more complete documentation of the issue.  And teach them to read
documentation in general too ;-)

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[Charles Wilson]

DJ Delorie wrote:

> Ok, I'm cynical of the average user.  How about this:
> 
> stringtest.o: In function `main':
> /usr/src/binutils/tmp/stringtest.c:9: variable 'hwstr1' can't be
> auto-imported.  Please read the documentation for ld's
> --enable-auto-import for details.
> 
> By not hinting at workarounds in the message, we force them to read
> the more complete documentation of the issue.  And teach them to read
> documentation in general too ;-)


Sounds fine to me.  Any comments on the proposed changes to that 
particular section of ld.texinfo?  (Reproduced here for discussion:)

------------------------------------------------------
@kindex --enable-auto-import
@item --enable-auto-import
Do sophisticalted linking of @code{_symbol} to @code{__imp__symbol} for
DATA imports from DLLs, and create the necessary thunking symbols when
building the DLLs with those DATA exports.  This generally will 'just
work' -- but sometimes you may see this message:

"aggregate '<var>' is referenced in direct addressing mode with constant
offset - auto-import failed. Workarounds: a) use 'volatile' auxilliary
variable; b) mark the symbol with __declspec(dllimport)"

This message occurs when some (sub)expression accesses an address
ultimately given by the sum of two constants, such as member fields
of struct variables imported from a DLL, or using a constant index into
an array variable imported from a DLL.  The solution is to force one
'constant' to be a variable -- that is, unknown and un-optimizable
at compile time.  For arrays, there are two possibilities: a) make
the indexee (the array's address) a variable, or b) make
the 'constant' index a variable.  Thus:

@example
extern type extern_array[];
extern_array[1] -->
    @{ volatile type *t=extern_array; t[1] @}
@end example

or

@example
extern type extern_array[];
extern_array[1] -->
    @{ volatile int t=1; extern_array[t] @}
@end example

For structs, the only option is to make the struct itself variable:

@example
extern struct s extern_struct;
extern_struct.field -->
    @{ volatile struct s *t=&extern_struct; t->field @}
@end example

Of course, you can also just give up on 'auto-import' for the offending
symbol and mark it with @code{__declspec(dllimport)}.  However, that
requires using compile-time #defines to indicate whether you are
building a DLL, building client code that will link to the DLL, or
merely building/linking to a static library.   In making the choice
between the two methods of resolving the 'direct address with constant
offset' problem, you should consider typical real-world usage:

Original:
@example
--foo.h
extern int arr[];
--foo.c
#include "foo.h"
void main(int argc, char **argv)@{
   printf("%d\n",arr[1]);
@}
@end example

Solution 1:
@example
--foo.h
extern int arr[];
--foo.c
#include "foo.h"
void main(int argc, char **argv)@{
   /* This workaround is for win32 and cygwin; do not "optimize" */
   volatile int *parr = arr;
   printf("%d\n",parr[1]);
@}
@end example

Solution 2:
@example
--foo.h
/* Note: auto-export is assumed (no __declspec(dllexport)) */
#if (defined(_WIN32) || defined(__CYGWIN__)) && \
   !(defined(FOO_BUILD_DLL) || defined(FOO_STATIC))
#define FOO_IMPORT __declspec(dllimport)
#else
#define FOO_IMPORT
#endif
extern FOO_IMPORT int arr[];
--foo.c
#include "foo.h"
void main(int argc, char **argv)@{
   printf("%d\n",arr[1]);
@}
@end example

A final way to avoid this problem is to re-code your
library to use a functional interface rather than a data interface
for the offending variables (e.g. set_foo() and get_foo() accessor
functions).

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[DJ Delorie]

sophisticalted -> sophisticated

and make sure the error message example in the docs matches the
printed message ;-)

" the sum of two constants," how about " the sum of two constants
(Win32 import tables only allow one)," so that it's clear this is a
fundamental limitation, not just a lazy GNU coder.

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[Richard Henderson]

On Tue, Sep 11, 2001 at 02:27:57PM -0400, Charles Wilson wrote:
> so you're saying that in some cases, gcc will ignore the 'volatile' 
> marker, and replace the variable with a constant?  Why?  Isn't 
> preventing that "optimization" exactly what the volatile modifier is for?

No.  The volatile marker applies to the *memory* not the address.
You're having problems with the computations of the address.  Ergo,
adding volatile is useless.

GCC's previous actions in not generally optimizing the addresses
of volatile memories is irrelevant.


r~

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[Richard Henderson]

On Tue, Sep 11, 2001 at 03:45:09PM -0400, DJ Delorie wrote:
> The only hope is to modify gcc to not allow offsets in direct
> addressing, but that would (1) be a big performance penalty,
> and (2) probably not be accepted by the gcc folks.

(1) is probably not true -- offsets don't happen *that* often.
(2) is definitely not true, assuming the restriction is limited
    to the affected targets.

r~

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[DJ Delorie]

> (1) is probably not true -- offsets don't happen *that* often.

I guess we'd have to benchmark something to know for sure.

> (2) is definitely not true, assuming the restriction is limited
>     to the affected targets.

It would need to be in the ix86 common code, since it affects
GO_IF_LEGITIMATE_ADDRESS.

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[Richard Henderson]

On Tue, Sep 11, 2001 at 07:47:01PM -0400, DJ Delorie wrote:
> It would need to be in the ix86 common code, since it affects
> GO_IF_LEGITIMATE_ADDRESS.

But controlled by TARGET_CAN_OFFSET_ADDRESSES.


r~

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[Charles Wilson]

DJ Delorie wrote:

> sophisticalted -> sophisticated
> 
> and make sure the error message example in the docs matches the
> printed message ;-)
> 
> " the sum of two constants," how about " the sum of two constants
> (Win32 import tables only allow one)," so that it's clear this is a
> fundamental limitation, not just a lazy GNU coder.


Okay, how's this (I've reworded it a little so that it flows better; the 
actual error message matches what is quoted below):


Do sophisticated linking of @code{_symbol} to @code{__imp__symbol} for
DATA imports from DLLs, and create the necessary thunking symbols when
building the DLLs with those DATA exports.  This generally will 'just
work' -- but sometimes you may see this message:

"variable '<var>' can't be auto-imported. Please read the
documentation for ld's @code{--enable-auto-import} for details."

This message occurs when some (sub)expression accesses an address
ultimately given by the sum of two constants (Win32 import tables only
allow one).  Instances where this may occur include accesses to member
fields of struct variables imported from a DLL, as well as using a
constant index into an array variable imported from a DLL.  There are
several ways to address this difficulty.

One solution is to force one of the 'constants' to be a variable --
that is, unknown and un-optimizable at compile time.  For arrays,
there are two possibilities: a) make the indexee (the array's address)
a variable, or b) make the 'constant' index a variable.  Thus:

@example
extern type extern_array[];
extern_array[1] -->
   @{ volatile type *t=extern_array; t[1] @}
@end example

or

@example
extern type extern_array[];
extern_array[1] -->
   @{ volatile int t=1; extern_array[t] @}
@end example

For structs, the only option is to make the struct itself variable:

@example
extern struct s extern_struct;
extern_struct.field -->
   @{ volatile struct s *t=&extern_struct; t->field @}
@end example

A second method of dealing with this difficulty is to abandon
'auto-import' for the offending symbol and mark it with
@code{__declspec(dllimport)}.  However, in practice that
requires using compile-time #defines to indicate whether you are
building a DLL, building client code that will link to the DLL, or
merely building/linking to a static library.   In making the choice
between the various methods of resolving the 'direct address with
constant offset' problem, you should consider typical real-world usage:

Original:
@example
--foo.h
extern int arr[];
--foo.c
#include "foo.h"
void main(int argc, char **argv)@{
  printf("%d\n",arr[1]);
@}
@end example

Solution 1:
@example
--foo.h
extern int arr[];
--foo.c
#include "foo.h"
void main(int argc, char **argv)@{
  /* This workaround is for win32 and cygwin; do not "optimize" */
  volatile int *parr = arr;
  printf("%d\n",parr[1]);
@}
@end example

Solution 2:
@example
--foo.h
/* Note: auto-export is assumed (no __declspec(dllexport)) */
#if (defined(_WIN32) || defined(__CYGWIN__)) && \
  !(defined(FOO_BUILD_DLL) || defined(FOO_STATIC))
#define FOO_IMPORT __declspec(dllimport)
#else
#define FOO_IMPORT
#endif
extern FOO_IMPORT int arr[];
--foo.c
#include "foo.h"
void main(int argc, char **argv)@{
  printf("%d\n",arr[1]);
@}
@end example
 
A third way to avoid this problem is to re-code your
library to use a functional interface rather than a data interface
for the offending variables (e.g. set_foo() and get_foo() accessor
functions).

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[DJ Delorie]

Yeah, that reads smoothly.

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[Charles Wilson]

DJ Delorie wrote:

> Yeah, that reads smoothly.
> 
> 


Okay to commit revised patch (as attached) ?

--Chuck

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[DJ Delorie]

Why is this one needed?  That's what the ATTRIBUTE_UNUSED is for...

The rest of it looks OK to me.  I assume you're done the usual
build/test cycle, yes?

> @@ -931,6 +947,8 @@
>    struct bfd_hash_entry *h;
>    PTR string ATTRIBUTE_UNUSED;
>  {
> +  (void)string;
> +
>    if (pe_dll_extra_pe_debug)

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[Charles Wilson]

DJ Delorie wrote:

> Why is this one needed?  That's what the ATTRIBUTE_UNUSED is for...
> 
> The rest of it looks OK to me.  I assume you're done the usual
> build/test cycle, yes?
> 


Yes.  Well, I have rebuilt & and tested with those revised patches, but 
it wasn't a full "clean" test; I didn't wipe out my build dir and start 
over from scratch.  I'll do that now.

> 
>>@@ -931,6 +947,8 @@
>>   struct bfd_hash_entry *h;
>>   PTR string ATTRIBUTE_UNUSED;
>> {
>>+  (void)string;
>>+
>>   if (pe_dll_extra_pe_debug)


Ah -- I think that's because Paul was using non-current-cvs sources when 
he developed the original patch. (However, I have been, so *my* version 
of the patch has both the ATTRIBUTE_UNUSED thing and the (void)string 
thing).  The PTR string ATTRIBUTE_UNUSED thing was added sometime after 
the late July/early August round of testing on auto-import.

I'm removing the (void)string thing, and then I'll do a complete (clean) 
rebuild/test cycle.

--Chuck

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[Charles Wilson]

Charles Wilson wrote:

> DJ Delorie wrote:
> 
>> Why is this one needed?  That's what the ATTRIBUTE_UNUSED is for...
>>
>> The rest of it looks OK to me.  I assume you're done the usual
>> build/test cycle, yes?
>>
> 
> 
> Yes.  Well, I have rebuilt & and tested with those revised patches, but 
> it wasn't a full "clean" test; I didn't wipe out my build dir and start 
> over from scratch.  I'll do that now.
> 
>>
>>> @@ -931,6 +947,8 @@
>>>   struct bfd_hash_entry *h;
>>>   PTR string ATTRIBUTE_UNUSED;
>>> {
>>> +  (void)string;
>>> +
>>>   if (pe_dll_extra_pe_debug)
>>
> 
> 
> Ah -- I think that's because Paul was using non-current-cvs sources when 
> he developed the original patch. (However, I have been, so *my* version 
> of the patch has both the ATTRIBUTE_UNUSED thing and the (void)string 
> thing).  The PTR string ATTRIBUTE_UNUSED thing was added sometime after 
> the late July/early August round of testing on auto-import.
> 
> I'm removing the (void)string thing, and then I'll do a complete (clean) 
> rebuild/test cycle.


Okay, with the (void)string part of the patch removed, I did a complete, 
clear-out-the-build-dir-and-rebuild cycle.  The rebuilt ld works as 
expected.

The re-revised patch, as tested, is attached.  Okay to apply?

--Chuck

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[DJ Delorie]

> The re-revised patch, as tested, is attached.  Okay to apply?

With a suitable ChangeLog entry, yes.  Thanks!

Re: [RFA] [pei-386] prevent ld (auto-import) from generating broken code

[Charles Wilson]

DJ Delorie wrote:

>>The re-revised patch, as tested, is attached.  Okay to apply?
>>
> 
> With a suitable ChangeLog entry, yes.  Thanks!


Done.

--Chuck