Thread: [MinGW-dvlpr] sync mingw pseudo-reloc.c with cygwin

SourceForge Headquarters 1320 Columbia Street Suite 310 San Diego, CA 92101 +1 (858) 422-6466

After a lot of discussion and testing, we've managed to get the new v2
pseudo-reloc support from Kai Tietz integrated into cygwin:
http://cygwin.com/ml/cygwin-patches/2009-q4/msg00074.html

Discussion here:
CFA: pseudo-reloc v2
http://cygwin.com/ml/cygwin-developers/2009-10/msg00052.html

Add wrappers for ExitProcess, TerminateProcess
http://cygwin.com/ml/cygwin-patches/2009-q4/msg00028.html

Merge pseudo-reloc-v2 support from mingw/pseudo-reloc.c
http://cygwin.com/ml/cygwin-patches/2009-q4/msg00065.html

However, we needed some changes:
1) to ensure that relocation works across fork() in the child.  Since
mingw doesn't have fork(), there was no reason to expect that Kai's
initial mingw version would have worked unchanged in the cygwin context.

2) Some of the error handling used printf, and assert, which
is...unwise...during this early stage of a cygwin process's life-cycle.

3) We chose to make all errors during relocation fatal, rather than just
emitting an error message and continuing.

4) Comments.  Lots of comments.

5) There was also a possible bug in the mingw code:
      assert(VirtualQuery(addr, &b, sizeof(b));
   goes completely away if -DNDEBUG -- but later on, __write_memory()
   assumes that 'b' has been initialized.

   This "bug" has not occurred in practice, because pseudo-reloc is
   /not/ compiled using -DNDEBUG.  But, it still ought to be fixed.

6) There are a few places with extraneous spaces.

For all except #5 -- and #6 -- the changes are guarded by #if
defined(__CYGWIN__), and so have no impact on mingw.  However, it'd sure
be nice if both
   winsup/mingw/pseudo-reloc.c and
   winsup/cygwin/lib/pseudo-reloc.c
were the same.

The attached patch does that.  For warm fuzzies, I've pasted inline the
difference between the results of pre-processing both the old and new
versions of the pseudo-reloc code with mingw's gcc (and then stripping
out blank lines and lines starting with '#' generated by the
preprocessor).  E.g.

$ cat pseudo-reloc.i-old | sed -e '/^$/d' -e '/^#/d' >\
      pseudo-reloc.i-old-noblanks
$ cat pseudo-reloc.i-new | sed -e '/^$/d' -e '/^#/d' >\
      pseudo-reloc.i-new-noblanks
$ diff -up pseudo-reloc.i-old-noblanks pseudo-reloc.i-new-noblanks

I've added comments intermixed with this warm-fuzzy diff, below:

--- pseudo-reloc.i-old-noblanks	2009-10-07 13:59:08.707505800 -0400
+++ pseudo-reloc.i-new-noblanks	2009-10-07 13:59:17.949505800 -0400
@@ -13506,9 +13506,9 @@ int __attribute__((__cdecl__)) __attribu
  int __attribute__((__cdecl__)) __attribute__ ((__nothrow__)) getw (FILE*);
  int __attribute__((__cdecl__)) __attribute__ ((__nothrow__)) putw
(int, FILE*);
  void __attribute__((__cdecl__)) __attribute__ ((__nothrow__)) _assert
(const char*, const char*, int) __attribute__ ((__noreturn__));
- extern char __RUNTIME_PSEUDO_RELOC_LIST__;
- extern char __RUNTIME_PSEUDO_RELOC_LIST_END__;
- extern char _image_base__;
+extern char __RUNTIME_PSEUDO_RELOC_LIST__;
+extern char __RUNTIME_PSEUDO_RELOC_LIST_END__;
+extern char _image_base__;


[[[[ remove extra leading spaces ]]]]


 typedef struct {
   DWORD addend;
   DWORD target;
@@ -13528,9 +13528,11 @@ __write_memory (void *addr,const void *s
 {
   MEMORY_BASIC_INFORMATION b;
   DWORD oldprot;
+  SIZE_T memsz;
   if (!len)
     return;
-  ((VirtualQuery (addr, &b, sizeof(b))) ? (void)0 :
_assert("VirtualQuery (addr, &b, sizeof(b))",
"/usr/src/devel/kernel/src/winsup/mingw/pseudo-reloc.c", 52));
+  memsz = VirtualQuery (addr, &b, sizeof(b));
+  ((memsz) ? (void)0 : _assert("memsz",
"/usr/src/devel/kernel/src/winsup/cygwin/lib/pseudo-reloc.c", 160));


[[[[ item #5, above. Hard to interpret in this form  ]]]]
[[[[ given the way the assert() macro is expanded by ]]]]
[[[[ the preprocessor. See below.                    ]]]]


   if (b.Protect != 0x0040 && b.Protect != 0x0004)
     VirtualProtect (b.BaseAddress, b.RegionSize, 0x0040,
     &oldprot);
@@ -13550,7 +13552,9 @@ do_pseudo_reloc (void * start, void * en
   if (reloc_target >= 12
       && v2_hdr->magic1 == 0 && v2_hdr->magic2 == 0
       && v2_hdr->version == 0)
-    v2_hdr++;
+    {
+      v2_hdr++;
+    }


[[[[ for clarity, given the way the extra 'if'     ]]]]
[[[[ clauses and the 'if' body-statement line up   ]]]]


   if (v2_hdr->magic1 != 0 || v2_hdr->magic2 != 0)
     {
       runtime_pseudo_reloc_item_v1 * o;
@@ -13611,7 +13615,7 @@ do_pseudo_reloc (void * start, void * en
      }
  }
 void
- _pei386_runtime_relocator ()
+_pei386_runtime_relocator ()


[[[[ remove extra leading space ]]]]


 {
   static int was_init = 0;
   if (was_init)



So, as you see, the changes have no actual impact on the mingw code,
except for issue #5.  In that case, in non-preprocessed form:

 static void
 __write_memory (void *addr,const void *src,size_t len)
 {
   MEMORY_BASIC_INFORMATION b;
   DWORD oldprot;
+  SIZE_T memsz;
+
   if (!len)
     return;
-  assert (VirtualQuery (addr, &b, sizeof(b)));
+
+  memsz = VirtualQuery (addr, &b, sizeof(b));
+
+#if defined(__CYGWIN__)
+  /* cygwin stuff elided */
+#else
+  /* comment elided */
+  assert (memsz);
+#endif
+
   /* Temporarily allow write access to read-only protected memory.  */


This ensures that VirtualQuery is always called, and 'b' is always
initialized -- but mingw will still assert if VirtualQuery returns 0, as
before.


Now, it is possible that, for mingw, you might want to also use some of
the cygwin changes -- e.g. __print_reloc_error [ slightly modified to
call TerminateProcess(GetCurrentProcess(),...) directly, instead of
cygwin_internal(CW_EXIT_PROCESS,...,useTerminateProcess=true) ] -- but I
believe that to be a discussion for a later patch.

Right now, I'm just trying to sync up the two (three [*]) versions with
the bare minimum of /actual/ changes to the mingw code.

[*] Once we stabilize the mingw version, I'll see what Kai wants to do
with respect to the mingw64 version.

OK?

2009-10-07  Charles Wilson  <...>

        Sync pseudo-reloc.c with cygwin/lib/
        * pseudo-reloc.c [CYGWIN]: Added comments throughout and various
        whitespace fixes. Exploit cygwin_internal(CW_EXIT_PROCESS,...)
        for fatal error handling that is consistent with cygwin process
        life-cycle. Ensure state variable (in _pei386_runtime_relocator)
        is unique to each address space, across fork().
        [CYGWIN] (__print_reloc_error): New function for reporting
        errors in a manner supported by cygwin at this early stage of
        the process life-cycle.
        [CYGWIN] (_pei386_runtime_relocator): Ensure relocations
        performed only once for each address space, but are repeated
        after fork() in the new address space.
        [MINGW] (__write_memory): Ensure that b is always initialized
        by call to VirtualQuery, even if -DNDEBUG.

--
Chuck



Thread: [MinGW-dvlpr] sync mingw pseudo-reloc.c with cygwin

A native Windows port of the GNU Compiler Collection (GCC)

mingw-dvlpr