Re: gold work on mingw/mingw64 support

[Vladimir Simonov <sv at sw dot ru>]

On 12/30/2010 10:25 PM, Ian Lance Taylor wrote:
> Vladimir Simonov<sv@sw.ru>  writes:
>
> > Attaching the patch which allows to build gold working
> > in mingw/mingw64 environment.
> >
> > The patch's idea is to provide restricted emulation for
> > Linux-specific calls, like mremap, mremap, etc. And force
> > callers always to keep files in-memory.
> >
> > Yes, this approach is a bit memory consuming but
> > IMO it is not a problem, especially for mingw64.
> > Really, for 32-bit Windows cygwin-based gold may be used.
> >
> > It is a port of my patch against binutils 2.20.1 and
> > it was not tested deep. Original patch works
> > quite well - I saw only debug-info messing in very
> > large programs.
> >
> > What do you think about this approach and the patch itself?
>
> Thanks for sending the patch.  I definitely do not want to include a
> patch which sprinkles #ifndef __MINGW32__ across the gold sources.  A
> better way to handle this would be library routines which would only be
> used, or not used, on mingw, with the configure script and Makefile
> selecting the code to compile.
>
>
> > @@ -195,6 +199,7 @@
> >   {
> >     gold_assert(this->is_locked());
> >
> > +#ifndef __MINGW32__
> >     if (!parameters->options_valid() || parameters->options().stats())
> >       {
> >         file_counts_initialize_lock.initialize();
> > @@ -204,6 +209,7 @@
> >         if (File_read::current_mapped_bytes>  File_read::maximum_mapped_bytes)
> >   	File_read::maximum_mapped_bytes = File_read::current_mapped_bytes;
> >       }
> > +#endif
>
> Why #ifndef out this code?
>
>
> > @@ -414,7 +420,9 @@
> >       }
> >
> >     File_read::View* v;
> > +#ifndef __MINGW32__
> >     if (byteshift != 0)
> > +#endif
> >       {
> >         unsigned char* p = new unsigned char[psize + byteshift];
> >         memset(p, 0, byteshift);
> > @@ -422,6 +430,7 @@
> >         v = new File_read::View(poff, psize, p, byteshift, cache,
> >                                 View::DATA_ALLOCATED_ARRAY);
> >       }
> > +#ifndef __MINGW32__
> >     else
> >       {
> >         this->reopen_descriptor();
> > @@ -440,6 +449,7 @@
> >         v = new File_read::View(poff, psize, pbytes, 0, cache,
> >                                 View::DATA_MMAPPED);
> >       }
> > +#endif
>
> I don't mind adjusting the code to use new/read instead of mmap, but it
> would need to be done in a more clean fashion, such as by replacing mmap
> as you did in output.cc.  This replacement should be done without
> #ifndef __MINGW32__.
>
>
> > diff -ruN binutils-2.21//gold/incremental.cc ../i686-gcc-4.4.3-glibc-2.3.2-0.43/build/binutils-2.21/gold/incremental.cc
> > --- binutils-2.21//gold/incremental.cc	2010-10-15 02:10:22.000000000 +0400
> > +++ binutils-2.21/gold/incremental.cc	2010-12-29 11:41:04.776106400 +0300
> > @@ -117,12 +117,19 @@
> >   void
> >   vexplain_no_incremental(const char* format, va_list args)
> >   {
> > +#ifndef __MINGW32__
> >     char* buf = NULL;
> >     if (vasprintf(&buf, format, args)<  0)
> >       gold_nomem();
> >     gold_info(_("the link might take longer: "
> >                 "cannot perform incremental link: %s"), buf);
> >     free(buf);
> > +#else
> > +  char buf[4096];
> > +  vsnprintf(buf, sizeof(buf), format, args);
> > +  gold_info(_("the link might take longer: "
> > +              "cannot perform incremental link: %s"), buf);
> > +#endif
>
> Why is this necessary?  vasprintf is in libiberty.
>
> Ian

Hi Ian,

Thank you for attention and good review.
Attaching new version which is a bit better, I hope.

I still trying to keep changes minimal and live without
additional libraries, configure and Makefile changes.

IMO mremap.c, pread.c, etc. are a kind of such library.
Analysis of target OS capabilities is overkill here, we
know in advance - mingw doesn't have mmap and most probably
won't have it.

If __MINGW32__ looks too OS specific it may be
replaced with HAVE_NO_MMAP, HAVE_NO_CLOEXEC defined
in config/mh-mingw. Really __MINGW32__ looks quite strange
in the middle of descriptors.cc but I don't know how to do it
better - fake function in mremap.h would be also strange,
new fcntl.c - the same...

Best regards
Vladimir Simonov

Attachment: binutils-2.21-gold-mingw-2.patch
Description: Text document