RE: Libtool stresstest.at segfault on Cygwin/MinGW

[Peter Ekberg]

Ralf Wildenhues wrote on Monday, September 19, 2005 17:10 CEST
> * Peter Ekberg wrote on Mon, Sep 19, 2005 at 04:17:56PM CEST:

*snip*

> > Well, the test segfaults on MinGW with the patch, and if I add
> > DATA to all symbols manually in asyms the (reordered) test goes
> > on until the same problem is triggered when export_symbols_cmds
> > is invoked because of the -export-symbols-regex option, so I
> > assume it is not good enough for MinGW. I think the import lib
> > gets screwed up if data symbols are not correctly tagged.
> 
> OK.  I assume it's not a linker bug then.

(nit: s/linker/compiler/ I suppose)

*snip*

> > The patch makes exporting non-const data work though, which is
> > what I need...
> 
> Surely non-constant data are more important.  But you do realize that
> exporting data objects should be avoided if possible?  ;-)

I just the poor sod trying to make the dang lib work on
windows. Have mercy... :-)

*snip*

> > > Hmm. You set export_symbols to empty before, so in fact 
> you kill the
> > > (little known, probably unused) include_expsyms feature 
> here.  Can't
> > > we have both?  By the way, your bug report also shows that
> > > include_expsyms needs a similar treatment as your filter, 
> otherwise
> > > one would have to add
> > >   symbol,DATA
> > > to the include_expsyms list on cygwin, right?  Not a 
> pressing issue
> > > though, this feature looks little-used.
> > 
> > Actually, I had that in mind when I wrote the patch. So,
> > include_expsyms isn't killed as I set up the variables so that
> > the big if statement is entered and in there somewhere there's
> > a new assignment of export_symbols.
> 
> But then include_expsyms is appended to export_symbols.  That 
> in turn is
> used to build the filter.  The filter is in turn used on
> orig_export_symbols, which does not contain the 
> include_expsyms.  Right?
> Looks wrong to me.  I believe you want to add the include_expsyms to
> orig_export_symbols instead of export_symbols.
> 
> Please note all of above babble is from reading alone, I will 
> eventually
> test this, but it might take a while.

Hey, you're right. Silly me.

> Also, while thinking about it, please provide an empty default value
> for orig_export_symbols right before the first hunk of your 
> patch; that
> way it's clear this is a locally-used variable.

Ok.

> > Perhaps I should add a test for $orig_export_symbols in the big if
> > instead? Whatever suits you best, it's all the same to me...
> 
> Somewhere, a helpful comment would be good.  This section of the code
> contains loads of intertwined magic (combined with the settings of
> export_symbols_cmds in libtool.m4).  If only to keep this from being
> broken by the next casual patch, stresstest needs to be extended to
> cover all (un)likely cases.

Ok, a comment it is.

> > > Good thing you bring include_expsyms up here: stresstest does 
> > > not cover this line.  Another thing to check. :)
> > 
> > Make sure to check it when exports are skipped due to command
> > line length too (there's a bug to fix). And also check
> > -export-symbols-regex when exports are skipped due to command
> > line length (oops, another one). I better shut up... :-)
> 
> I noted that, too.  But thanks for reminding, I added them to the TODO
> list.  Erm, did I mention I take patches for all of these issues.  ;-)

:-)

Thinking about it further, include_expsyms is perhaps not
buggy with skipped exports, as then the needed symbols perhaps
get exported anyway with the export_symbols_cmds functionality.
A check wouldn't hurt though...

> > But thinking further, my patch is also less than perfect if
> > exports are skipped due to command line length. It would
> > export all global symbols, instead of only those specified,
> > and that would be a regression for those only exporting
> > functions from a *large* number of objects. Crap.
> 
> Your patch is better than what we had before.  We should fix the
> piecewise linking ones separately.  But if you want to improve on
> the patch first, go right ahead.

I'll quite happily think about that later...

> Note also there was once a plan to kill the quadratic scaling behavior
> in the piecewise linking case..

Ok.

*snip*

> > > This surely needs to be tested for possible mingw 
> path-translation or
> > > other issues (try by calling it with SED referring to a 
> path component
> > > starting outside the msys tree).
> > 
> > Should I:
> > 
> > $ mkdir /c/foo
> > $ cp /bin/sed /c/foo
> > $ ./configure SED=/c/foo/sed
> > 
> > or what do you mean?
> 
> Something like this.  I'd start less drastic, though, by only 
> replacing
> SED in the created `libtool', or you might be finding more issues than
> you like.  OTOH, comments from mingw people indicate this should not
> happen in a good setup, so maybe it's not important after all.  We
> really don't need to support people who break their setups too much.

Ok, tested with a copy of sed in /c/foo and /c/foo/sed where the
patch has $SED in ltmain.m4sh, and it works. I don't know if that's
a good enough test for MinGW file mode issues though.

Updated patch attached. I think this is good to go now.

Cheers,
Peter (who will play with your stresstest patch tomorrow)

head-filter-data-symbols-3.patch
Description: head-filter-data-symbols-3.patch