Re: Libtool stresstest.at segfault on Cygwin/MinGW

[Ralf Wildenhues]

Hi Peter,

* Peter Ekberg wrote on Mon, Sep 19, 2005 at 12:11:23PM CEST:
> * Peter Ekberg wrote on Friday, September 16, 2005 11:11 CEST:
> > 
> > In short, when you specify the symbols to export with
> > "-export-symbols foo.sym", the file foo.sym is simply prepended
> > with an EXPORTS line (if missing) and used as a .def input file
> > for the linker. But when a .def file is specified, the auto-export
> > magic is shorted out of the loop and the .def file is trusted
> > blindly. So, the correct fix is that when the symfile given to
> > libtool does not start with an EXPORTS line, the symbols in it has
> > to be filtered and " DATA" should be appended to those symbols
> > that are data symbols, probably similar to what is done when the
> > -export-symbols-regex option is used.
> 
> And here's a patch to fix it. It doesn't work 100% on MinGW as
> MinGW seems to put most const symbols in the .text segment, so
> the nm output has T for some symbols where the nm output on
> Cygwin has R.

Is this a linker bug?  Should we mention this on the mingw list?
OTOH, if, for the symbols that mingw puts in .text, it also works
fine without the DATA, I don't think this is an issue at all.
Is that maybe the case?  (Note I don't *really* know what I'm talking
about here, just stabbing in the dark..)

> But it seems to work OK on Cygwin so I think the
> patch should still be considered. I have no clue as to how to
> detect all symbols that are data symbols on MinGW. The symbols
> in stresstest.at that are placed in the .text segment are v5,
> v6, v8 and v12 (apart from the obvious function, v9). These
> symbols are all const, The last const symbol (v7) isn't in the
> .text segment on MinGW, that's probably for relocation issues.

I see that I need to hurry to extend stresstest so you don't
run out of bugs to fix.  ;-)

>       * libltdl/config/ltmain.m4sh (func_mode_link): Filter
>       user supplied symfile to tag relevant symbols as data
>       symbols. Fixes segfault in stresstest.at on Cygwin and
>       makes the test pass.

Very clever idea, by the way, to reuse the already-present
export_symbols_cmds machinery!

> The patch should perhaps be backported to 1.5 as the bug is
> most likely present there as well, but I don't use 1.5...

Yes, I agree.  Same thing with your other stresstest-induced fix.

Some comments to your patch (untested so far):

*snip first hunk*
| @@ -4967,6 +4983,14 @@
|  
|       if test -n "$export_symbols" && test -n "$include_expsyms"; then
|         $opt_dry_run || eval '$ECHO "X$include_expsyms" | $Xsed | $SP2NL >> 
"$export_symbols"'
| +     fi

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.

Good thing you bring include_expsyms up here: stresstest does not cover
this line.  Another thing to check. :)

| +
| +     if test "X$skipped_export" != "X:" && test -n "$orig_export_symbols"; 
then
| +       # The given exports_symbols file has to be filtered, so filter it.
| +       func_echo "filter symbol list for \`$libname.la' to tag DATA exports"
| +       $opt_dry_run || $SED -e '/[[ ,]]DATA/!d;s,\(.*\)\([[ 
,]].*\),s/^\1$/\1\2/,' < $export_symbols > $output_objdir/$libname.filter

Please escape the separator (in this case, the bracketed comma) within
patterns (this is a POSIX necessity), or use a separator not present in
the pattern; also, in light of the other recent bug report, it may be
safer to use slashes as little as possible (shown without m4 quoting):
  /[ ,]DATA/!d; s,\(.*\)\([ \,].*\),s|^\1$|\1\2|,

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).

| +       export_symbols=$output_objdir/$libname.def
| +       $opt_dry_run || $SED -f $output_objdir/$libname.filter < 
$orig_export_symbols > $export_symbols
|       fi
|  
|       tmp_deplibs=

Thank you for working on this!

Cheers,
Ralf