This is the mail archive of the cygwin-developers mailing list for the Cygwin 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: CFA: pseudo-reloc v2


Charles Wilson wrote:
> cgf wrote:
>> This looks good to me although I think it could stand a few more
>> comments.  I feel like a hypocrite every time I ask for that but do as I
>> say not as I do, dammit!
> 
> No problem. I'll try to add comments throughout, but I'm not sure they
> will be all that helpful in the Kai-derived parts.  I'll also update to
> use the proposed cygwin_internal calls instead of cygwin_*_process.

BTW, looking at the original mingw version again (trying to figure out
how to comment it), I see that all of the "error messages" and
assertions are effective ONLY if -DDEBUG.

The commensurate thing to do with cygwin is to hide each of these checks
inside '#if DEBUGGING' so that we only get them if cygwin was configured
with --enable-debugging.  That is, like this:

#if defined(__CYGWIN__)
# if defined(DEBUGGING)
      __print_reloc_error (..stuff..)
      cygwin_internal(..stuff..)
 # endif
#else /* MINGW */
# if defined(DEBUG) /* <<< this was already here */
      fprintf (stderr, ..stuff..);
      ...
# endif
#endif

But I don't think that's TRTTD.

There are three such places in the code:
1) check to see if the page on which we think we have to update relocs
is actually mapped into our process.

2) v2: check that the pseudo-reloc version number is one that we know about

3) v2: check that the specified 'bit width' of the reloc is one of the
ones we know how to handle (8, 16, 32 for mingw32 and cygwin; 8, 16, 32
and 64 for mingw64). (For v1, all relocs were exactly 32 bits. The new
multi-width support allows for pc-relative relocs, as required for w64).

Here's the original behavior for mingw:
1) #if test fails, assert() (e.g. terminate, error msg if !NDEBUG)
2) #if DEBUG and test fails, print error message and continue
3) #if DEBUG and test fails, print error message and continue

I think that all of these issues are failures, and should be checked and
reported regardless of whether -DDEBUG (-UNDEBUG, -DDEBUGGING). AND that
in each case, the process should terminate.  Bad addresses, un-relocated
relocs, and relocs whose bitwidths we don't recognize are all very bad
things. Better to detect early, diagnose, and die than continue with a
busted image loaded in memory.

For case 2 and 3, there is no speed impact for the non-error path,
because the check involved must be performed anyway -- so you only see a
speed impact (printing the error message) just before you die.

For case 1, I think there is a bug in the mingw case. If the mingw
pseudo-reloc.c is compiled with -DNDEBUG, then the VirtualQuery function
is NOT called -- and "b" is not initialized.  Now, granted, when Chris
builds the mingw runtime he doesn't use -DNDEBUG, so no harm, no foul.
But it's still a bug.

Which means there is no speed impact for case 1 in the non-error path:
you always have to call VirtualQuery anyway.

So, I suggest the following behavior for cygwin, for all three cases.

1) if test fails, print error msg and terminate
2) if test fails, print error msg and terminate
3) if test fails, print error msg and terminate

Other than fixing the mingw bug, I don't contemplate changing its
behavior, right now.  Leave that for the mingw folks.

Comments?

--
Chuck


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