Re: [PATCH] Apparent thinko in closures.c causing GCC bootstrap failure on Cygwin

[Timothy Wall <twalljava at dev dot java dot net>]

On Jun 30, 2009, at 8:17 AM, Dave Korn wrote:

>    Hello lists,
>
>  Some minor fallout resulting after the libffi merge.  The code in  
> closures.c
> is currently structured like this:
>
> > #if !defined(X86_WIN32) && !defined(X86_WIN64)
> > /* Use these for mmap and munmap within dlmalloc.c.  */
> > static void *dlmmap(void *, size_t, int, int, int, off_t);
> > static int dlmunmap(void *, size_t);
> > #endif /* !defined(X86_WIN32) && !defined(X86_WIN64) */
> >
> > #define mmap dlmmap
> > #define munmap dlmunmap
> >
> > #include "dlmalloc.c"
> >
> > #undef mmap
> > #undef munmap
> >
> > #if !defined(X86_WIN32) && !defined(X86_WIN64)
> >
> > [ definitions of dlmmap, dlmunmap ]
> >
> > #endif
>
>  It currently causes bootstrap failure on cygwin like this:
>
> libtool: link: /gnu/gcc/obj-patched3/./gcc/gcj
> -B/gnu/gcc/obj-patched3/i686-pc-cygwin/libjava/ -B/gnu/gcc/obj- 
> patched3/./gcc/
> -B/opt/gcc-tools/i686-pc-cygwin/bin/ -B/opt/gcc-tools/i686-pc-cygwin/ 
> lib/
> -isystem /opt/gcc-tools/i686-pc-cygwin/include -isystem
> /opt/gcc-tools/i686-pc-cygwin/sys-include -ffloat-store -fomit-frame- 
> pointer
> -Usun -g -O2 -o .libs/jv-convert.exe --main=gnu.gcj.convert.Convert
> -shared-libgcc  -L/gnu/gcc/obj-patched3/i686-pc-cygwin/libjava/.libs
> -L/gnu/gcc/obj-patched3/i686-pc-cygwin/libjava ./.libs/libgcj.a -ldl
> -L/opt/gcc-tools/lib/gcc/i686-pc-cygwin/4.5.0
> /opt/gcc-tools/bin/ld: Dwarf Error: mangled line number section.
> ./.libs/libgcj.a(closures.o):closures.c:(.text+0x8af): undefined  
> reference to
> `_dlmunmap'
> ./.libs/libgcj.a(closures.o):closures.c:(.text+0xe59): undefined  
> reference to
> `_dlmmap'
> ./.libs/libgcj.a(closures.o):closures.c:(.text+0x1173): undefined  
> reference to
> `_dlmmap'
> ./.libs/libgcj.a(closures.o):closures.c:(.text+0x1c82): undefined  
> reference to
> `_dlmunmap'
> ./.libs/libgcj.a(closures.o):closures.c:(.text+0x1d5f): undefined  
> reference to
> `_dlmunmap'
> collect2: ld returned 1 exit status
> make[3]: *** [jv-convert.exe] Error 1
>
>  Ignoring the dwarf error, which is caused by an unrelated problem,  
> it looks
> to me like the intent of the above code is to provide - only on non- 
> windows
> platforms - these two replacement functions and override dlmalloc's  
> use of
> mmap/munmap with them.  But in that case, the #defines should be  
> inside the
> #if guards as well, otherwise we're unconditionally doing the  
> replacement but
> only conditionally supplying the replacement functions!

When compiling for windows, there shouldn't be any mmap/munmap  
references at all.  While cygwin may supply some version of these,  
dlmalloc defines w32mmap/w32unmap which directly calls the appropriate  
routines for windows when WIN32 is defined (which I believe is always  
the case for the mingw compiler).

Apparently the cygwin build is defining macros differently than -mno- 
cygwin (which is what I use for w32/w64), resulting in extant  
references to mmap/munmap.

Have you run the tests on a system with DEP enabled (System control  
panel->Performance Options->Data Execution Prevention)?  That's likely  
the only place they'd fail if mmap isn't set up correctly.  Prior to  
the w64 patches the dlmalloc stuff never enabled the w32 mmap  
equivalents, so it was always using vanilla malloc for closures.

>  I'm testing the attached on Cygwin.  I'm not set up for win64  
> testing but I
> don't suppose it offers dl* functions either; I'll see if I can find  
> one of
> the w64 guys to comment.
>
> libffi/ChangeLog:
>
> 	* closures.c (mmap, munmap):  Don't define replacement macros  
> pointing
> 	to dl* versions on windows platforms.
>
>  Assuming it passes bootstrap, OK?
>
>    cheers,
>      DaveK
> Index: libffi/src/closures.c
> ===================================================================
> --- libffi/src/closures.c	(revision 149030)
> +++ libffi/src/closures.c	(working copy)
> @@ -189,18 +189,18 @@
> /* Use these for mmap and munmap within dlmalloc.c.  */
> static void *dlmmap(void *, size_t, int, int, int, off_t);
> static int dlmunmap(void *, size_t);
> -#endif /* !defined(X86_WIN32) && !defined(X86_WIN64) */
>
> #define mmap dlmmap
> #define munmap dlmunmap
> +#endif /* !defined(X86_WIN32) && !defined(X86_WIN64) */
>
> #include "dlmalloc.c"
>
> +#if !defined(X86_WIN32) && !defined(X86_WIN64)
> +
> #undef mmap
> #undef munmap
>
> -#if !defined(X86_WIN32) && !defined(X86_WIN64)
> -
> /* A mutex used to synchronize access to *exec* variables in this  
> file.  */
> static pthread_mutex_t open_temp_exec_file_mutex =  
> PTHREAD_MUTEX_INITIALIZER;