This is the mail archive of the binutils@sources.redhat.com 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]

Re: Serious bug in auto-import


Charles Wilson wrote:


> 
> Therefore, I *much* prefer recommending the second of your alternatives 
> -- I wasn't aware that volatile would "fix" it for us. 


To clarify, 'volatile' is not a magic bullet.  It just prevents gcc from 
optimizing our workaround out of existence.  In the absence of 
optimization, this works just as well:
   extern_array[1] --> { type *t=extern_array; t[1] }
   extern_struct.field --> { struct s *t=&extern_struct; t->field }

> (And you can use 
> volatile for ALL THREE cases on cygwin; static, DLL(build), and 
> DLL(linkto) -- I think. I will test this tonight.).  


Yep.  Works fine.  (I know, I know, should be obvious -- but at this 
point, I'm in the "trust nothing unless you've tested it" mode.)

> One question, 
> though:  you say:
> 
>  > /* This stupid workaround is for win32 - do not "optimize" */
>  > volatile int *parr = arr;
> 
> Does your comment mean
>   "Future programmer editing this client code, do not replace references 
> to 'parr' with 'arr'."


After testing with various optimization settings, it is obvious you mean 
this ^^^^

> or
>   "When compiling this source code, do not specify -O{n}"


and not this ^^^^^^^^^

> Therefore, ld's error message should at least HINT at the "use a local 
> variable" possibility.  Your original wording does that.  Then, more 
> detailed stuff would go in the .info file (I'll base my ld.texinfo patch 
> on your explanation below).


In the patch I will soon be posting, I've reversed the order of the two 
solutions, to present the 'auxilliary variable' first, the __declspec() 
second.

 
> 
> Actually, given the "volatile" fix, I like it.  This usage seems to be 
> pretty uncommon, so the bug (and error message) will rarely be hit. 
> Recommending "volatile" markers means when only need to #ifdef on 
> __CYGWIN__ , not a bunch of FOO_STATIC/FOO_DLL_BUILD/FOO_DLL_LINKTO 
> variables.
> 
> Getting rid of THAT garbage was my primary goal in advocating your 
> auto-import stuff in the first place!
> 
> So: we have a rarely triggered bug.  Your patch warns the user and 
> prevents buggy code from being generated. My forthcoming documentation 
> patch (based on your stuff, above) will explain how a user hit by this 
> bug can work around it.  And we still don't need 
> FOO_STATIC/FOO_DLL_BUILD/etc.
> 
> I'm happy.


Okay, combined patch (with ld.texinfo stuff) coming up...

--Chuck



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