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[2]: Serious bug in auto-import


Hello Charles,

Charles Wilson wrote on Sunday, September 09, 2001:

[]

CW> Yep.  Tested.  ld with Paul's patch accurately detects, prints out a
CW> warning message, and exits-with-error whenever "direct addressing" is

    Yep, if I didn't confused it and used right English term (I
learned that long ago in Russian). Well, for sure it must right. In
Intel's terms, it is "direct addressing with [non-zero] displacement"
(vs for example with indirect addressing when [part of] address is held
in register). I used term "offset" instead of "displacement" though
(Intel used to mark offset within 8086 64k segements).

[]

CW> Now, with structs, it's a little tricky.  Assume the following definition:
CW> typedef struct {
CW>    int a;
CW>    int b;
CW> } ST_;

CW> This is bad:
CW> extern ST_ st;
CW>    st.a = ...

       Well, in fact *this* exact code is good. It's possible to
access the very first member of struct ;-)

CW> This is also bad (but is an analogous form to the "array" version that
CW> works)...
CW> extern ST_ st;
CW> ST_ p;
CW>    p = st;
CW>    p.a = ...

       Here's the trick: it should be working in general (get the
address of st, memcpy()  it), but gcc optimizes 8-byte copy, into two
consecutive stores, hence problem.

       Nevermind. It will tell when it's bad, and what is needed is to
know how to workaround such case.

[]

CW> Of course, that's almost identical to what happens when you do this
CW> (which is also good, of course):
CW> extern __declspec(dllimport) ST_ st;
CW>    st.a = ...

CW> The good news is, Paul's patch will always detect, warn, and fail when
CW> the "bad" forms are used -- because it's triggered by the "bad" assember
CW> instructions directly, regardless of what C code produced it.  (That is,
CW> no weird list of special cases.)  The only question is, when a user gets
CW> Paul's error message, how will the user know HOW to fix the problem (in
CW> their client code)?  *That* requires a long list of special cases in
CW> some documentation somewhere...

    As you see, I tried to hammer in howto into error message ;-)
(yes, it hardly will make sense for outsider). I think, there should
be standard recommendation: mark offending symbol with
__declspec(dllimport) (and error message should state only this
alternative). If you care, in ld you can add more elaborated
discussion, along the lines of:

=====
The problem happens when some (sub)expression accessing address
ultimately given by the sum of two constants. The solution is to make
it use one variable instead. In case of array, we have two
possibilities: a) make indexee (array) variable, b) make index
variable, i.e.:

extent type extern_array[];

extern_array[1] => { volatile type *t=extern_array; t[1] }

or

extern_array[1] => { volatile int t=1; extern_array[t] }

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

extent struct s extern_struct;

extern_struct.field => { volatile struct s *t=extern_struct; t->field }

Which workaround (__declspec(dllimport) marking or rewriting rules
above) you should use? Obviously, __declspec(dllimport) marking,
unless you have the aim not to introduce system-dependent features to
your code (but note that alternative is to introduce obscure
constructs into your code, so you will need to comment why you did
this anyway). In short, consider typical real-world usage of both
methods and decide yourself:

Original :
--foo.h
extern int arr[];
--foo.c
#include "foo.h"
main()
{
  printf(arr[1]);
}
--

1:
--foo.h
/* Note: auto-export is assumed (no __declspec(dllexport)) */
#if defined(_WIN32) && !defined(FOO_BUILD)
#define FOO_IMPORT __declspec(dllimport)
#else
#define FOO_IMPORT
#endif
extern FOO_IMPORT int arr[];
--foo.c
#include "foo.h"
main()
{
  printf(arr[1]);
}
--

2:
--foo.h
extern int arr[];
--foo.c
#include "foo.h"
main()
{
  /* This stupid workaround is for win32 - do not "optimize" */
  volatile int *parr = arr;
  printf(parr[1]);
}
--


=====

[]

CW> Unless of course:

>> THEN we can think about a "real" fix for this array-index problem (if
>> possible; it may not be). :-)

   Well, my guess is that occurrences of the issue will be so rare,
that "real fix" will never pay for itself...


[]

CW> --Chuck

--
Paul Sokolovsky, IT Specialist
http://www.brainbench.com/transcript.jsp?pid=11135



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