CFA: pseudo-reloc v2

[Dave Korn]

[ redirected from cygwin-developers. ]
On 04/10/2009 05:11, Charles Wilson wrote:
[ thread seriously necro'd! ]
> Dave Korn wrote:
>> Charles Wilson wrote:
>>>   120 void
>>>   121 _pei386_runtime_relocator ()
>>>   122 {
>>>   123   static int was_init = 0;
>>>   124   if (was_init)
>>>   125     return;
>>>   126   ++was_init;
>>>   127   do_pseudo_reloc (&__RUNTIME_PSEUDO_RELOC_LIST__,&__RUNTIME_PSEUDO_RELOC_LIST_END__,&_image_base__);
>>>   128 }
>>   Maybe that static should be NO_COPY?  If everything gets remapped in the
>> forkee, do the relocs need rerunning?  (I'm not sure about the behaviour of
>> NtCreateProcess w.r.t modified .text section pages.)
> 
> Good guess!  With the following patch, all of these fork tests perform
> as expected.

  Aha, not so good as all that after all!  We need to re-apply relocs in the
forkee - but only if they *don't* point to regions covered by the .data/.bss
section copying at startup.  Argh!

>  One oddity; it turns out that __INSIDE_CYGWIN__ is not
> defined inside pseudo-reloc.c, so I used __CYGWIN__ as a guard.

  Dunno if we ever went into that, but it's right; pseudo-reloc.o is part of
the CRT in winsup/cygwin/lib/, and is linked statically into every exe and
(user) dll, but is not part of the cygwin1 dll.  Hence not "inside Cygwin".

  So, the attached is my proposed fix.  It resolves the problem reported on
the main list the other day(*) and the supplied testcases all work once it's
applied.  There are two things that people might want to change: the minor one
is that I let a couple of the lines get a bit long, but no longer than we
already have in the definition of NO_COPY at the top of the file, so I didn't
wrap them for the sake of one trailing word.  The slightly bigger one is that,
if I remember, the reason for having non-#if-CYGWIN code in the file at all is
to make any potential future merges from upstream MinGW sources theoretically
easier, but now that I've had to diverge the internal interfaces anyway, is
there any reason not to just delete the whole lot?

winsup/cygwin/ChangeLog:

	lib/pseudo-reloc.c (memskip_t): New struct and typedef.
	(__write_memory): Accept an optional memskip_t argument and avoid
	writing to any memory ranges mentioned in the linked list.
	(do_pseudo_reloc): Accept an optional memskip_t argument and pass
	it through in all calls to __write_memory.
	(_pei386_runtime_relocator): When reapplying relocs in a forked
	child process, avoid doubly-relocating the .data and .bss sections
	that were copied from the parent.

    cheers,
      DaveK
-- 
(*) - http://cygwin.com/ml/cygwin/2010-04/msg00957.html

-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-double-relocs.diff
Type: text/x-c
Size: 6665 bytes
Desc: not available
URL: <http://cygwin.com/pipermail/cygwin-patches/attachments/20100505/da3c5305/attachment.bin>