Re: [PATCH] msvc: handle symbols from different files independently.
Den 2010-09-30 23:29 skrev Ralf Wildenhues:
* Peter Rosin wrote on Thu, Sep 30, 2010 at 10:53:13PM CEST:
Den 2010-09-30 21:25 skrev Ralf Wildenhues:
* Peter Rosin wrote on Thu, Sep 30, 2010 at 09:06:16PM CEST:
Ok to push when I have tested with various MSVC versions?
I guess, but you should really add testsuite exposure.
I don't know any controlled way to create a pair of object
files that clash, should I just fake it and extract
global_symbol_pipe from the libtool script and feed it
example output that clashes, and check that the symbol
isn't hidden?
I guess. Hmm, from your first description I thought this
was easier to reproduce.
I'm perhaps not fluent enough in PE internals?
Anyway, is this test case good enough? Should I find a better
way to skip on non-dumpbin runs? How?
Cheers,
Peter
From 8189aca834f90ab5d439c58370fcceb699eb0bef Mon Sep 17 00:00:00 2001
From: Peter Rosin p...@lysator.liu.se
Date: Fri, 1 Oct 2010 13:32:32 +0200
Subject: [PATCH] msvc: handle symbols from different files independently.
* libltdl/m4/libtool.m4 (_LT_CMD_GLOBAL_SYMBOLS)
dumpbin, lt_cv_sys_global_symbol_pipe: Make all sections
viable for symbol extraction again when the symbols from a new
file starts. Fixes tests/tagdemo-make.test for MSVC 10.
* tests/dumpbin-symbols.at: New test, making sure we don't
regress.
* Makefile.am (TESTSUITE_AT): Update.
Signed-off-by: Peter Rosin p...@lysator.liu.se
---
ChangeLog| 11 +
Makefile.am |3 +-
libltdl/m4/libtool.m4|1 +
tests/dumpbin-symbols.at | 106 ++
4 files changed, 120 insertions(+), 1 deletions(-)
create mode 100644 tests/dumpbin-symbols.at
diff --git a/ChangeLog b/ChangeLog
index a7aa489..a0ed532 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2010-09-30 Peter Rosin p...@lysator.liu.se
+
+ msvc: handle symbols from different files independently.
+ * libltdl/m4/libtool.m4 (_LT_CMD_GLOBAL_SYMBOLS)
+ dumpbin, lt_cv_sys_global_symbol_pipe: Make all sections
+ viable for symbol extraction again when the symbols from a new
+ file starts. Fixes tests/tagdemo-make.test for MSVC 10.
+ * tests/dumpbin-symbols.at: New test, making sure we don't
+ regress.
+ * Makefile.am (TESTSUITE_AT): Update.
+
2010-09-27 Peter Rosin p...@lysator.liu.se
tests: check if sys_lib_search_path_spec works on MSVC.
diff --git a/Makefile.am b/Makefile.am
index 6e29a29..c4aec10 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -504,7 +504,8 @@ TESTSUITE_AT= tests/testsuite.at \
tests/pic_flag.at \
tests/darwin.at \
tests/deplibs-mingw.at \
- tests/sysroot.at
+ tests/sysroot.at \
+ tests/dumpbin-symbols.at
EXTRA_DIST += $(srcdir)/$(TESTSUITE) $(TESTSUITE_AT)
$(srcdir)/tests/package.m4
diff --git a/libltdl/m4/libtool.m4 b/libltdl/m4/libtool.m4
index fd732d0..967dd38 100644
--- a/libltdl/m4/libtool.m4
+++ b/libltdl/m4/libtool.m4
@@ -3645,6 +3645,7 @@ for ac_symprfx in _; do
# which start with @ or ?.
lt_cv_sys_global_symbol_pipe=$AWK ['\
{last_section=section; section=\$ 3};\
+ /^COFF SYMBOL TABLE/{for(i in hide) delete hide[i]};\
/Section length .*#relocs.*(pick any)/{hide[last_section]=1};\
\$ 0!~/External *\|/{next};\
/ 0+ UNDEF /{next}; / UNDEF \([^|]\)*()/{next};\
diff --git a/tests/dumpbin-symbols.at b/tests/dumpbin-symbols.at
new file mode 100644
index 000..048560a
--- /dev/null
+++ b/tests/dumpbin-symbols.at
@@ -0,0 +1,106 @@
+# dumpbin-symbols.at -- libtool dumpbin -symbols support-*- Autotest -*-
+
+# Copyright (C) 2010 Free Software Foundation, Inc.
+#
+# This file is part of GNU Libtool.
+#
+# GNU Libtool is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# GNU Libtool is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GNU Libtool; see the file COPYING. If not, a copy
+# can be downloaded from http://www.gnu.org/licenses/gpl.html,
+# or obtained by writing to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+
+
+AT_BANNER([Symbol extraction])
+AT_SETUP([dumpbin -symbols section hiding])
+
+# I don't know of a stable way to create a pair of objects that
+# exhibits the potential problem, so this test fakes it by
+# testing with output from a case that do have the potential
+# problem.
+
+eval `$LIBTOOL --config | $EGREP '^(global_symbol_pipe)='`
+
Re: cwrapper generates long strings.
Hi Chuck,
Den 2010-10-01 16:03 skrev Charles Wilson:
On 10/1/2010 7:28 AM, Peter Rosin wrote:
I.e. I have this on line 865 in lt-main.c:
fputs (relink_command=\(cd
/c/cygwin/home/peda/libtool/git/msvc/tests/testsuite.dir/112;
PATH=\\\/LOADS:/OF:/ENTRIES\\\; export PATH;
PATH=\\\/LOADS:/OF:/ENTRIES\\\; export PATH;
/c/cygwin/home/peda/automake/lib/compile cl -MD -Zi -EHsc -o @OUTPUT@
.libs/main-static.obj sub2/.libs/a.lib )\\n, f);
In my case the string is 3400+ characters which is too much for MSVC 6,
but this appears to not be really compiler specific, and I can easily
imagine other compilers with other arbitrary (and possibly standardized)
limits.
c99 compilers must support const char* arrays of at least 4096. Most
compilers actually support much larger ones.
c89 requires only support for up to 509. See:
http://lists.gnu.org/archive/html/libtool-patches/2008-04/msg00161.html
http://lists.gnu.org/archive/html/libtool-patches/2009-01/msg4.html
I vaguely remember that, and rereading reveals that noone thought about
one individual line being too long (not publicly anyway).
One thing that could be done is to only have the PATH once, but that is
not a real fix.
It's also not general. One is explicitly setting PATH, the other is
setting $shlibpath_var (which happens to be PATH on win32).
Should we worry about my insane case?
I don't think it is a high priority. What we could do is implement a
line emitter in ltmain.m4sh, for use by func_emit_cwrapper_exe. Right
now, it takes the output of func_emit_wrapper and puts fputs(\ and
\n\ around each line.
Instead, each line could be fed to a line-emitter function that chops
the line into segments of = 500 chars.
But...that's a lot of effort for very little gain.
Ok, I wouldn't say a lot of effort, and it was a bit of fun.
I think a better approach would be to start asking, why do we still need
the cwrapper to contain and emit the shell wrapper code in the first
place? Maybe instead, we should just remove --lt-dump-script and all
related code.
It was originally present because the cwrapper would emit the shwrapper,
and then exec it. But now, the cwrapper exec's the real program
directly, so this functionality is no longer needed. It hasn't been
needed for years, but I didn't want to make too many changes at once so
I left it there even after cwrapper was changed to direct exec the real
program. But...I always intended to get rid of it. Perhaps now is the
time.
Right, but I had already sent the first version of the line splitter when
I read this. Here's an improved version as a proper git patch (why is it
so hard to keep your fingers away from small scriptlets?). Now, someone
needs to decide which way to go, and provide a --lt-dump-script removal
patch if that is decided.
Hardish testing outside libtool, and limited testing with the actual
patch, but at least stresstest.at is happy with my looong PATH from the
original post, and the lt-main.c code looks good too.
Cheers,
Peter
From 9c30540472d66c0caf56bdc90a7bf7152ad771d4 Mon Sep 17 00:00:00 2001
From: Peter Rosin p...@lysator.liu.se
Date: Fri, 1 Oct 2010 22:55:55 +0200
Subject: [PATCH] cwrapper: split long lines when dumping the wrapper script.
* libltdl/config/ltmain.m4sh (func_emit_cwrapperexe_src): If
the wrapper script contains long lines, split them for
readability and to conform with C standards.
Signed-off-by: Peter Rosin p...@lysator.liu.se
---
ChangeLog |7 +++
libltdl/config/ltmain.m4sh | 11 ---
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index a7aa489..515c23a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2010-10-01 Peter Rosin p...@lysator.liu.se
+
+ cwrapper: split long lines when dumping the wrapper script.
+ * libltdl/config/ltmain.m4sh (func_emit_cwrapperexe_src): If
+ the wrapper script contains long lines, split them for
+ readability and to conform with C standards.
+
2010-09-27 Peter Rosin p...@lysator.liu.se
tests: check if sys_lib_search_path_spec works on MSVC.
diff --git a/libltdl/config/ltmain.m4sh b/libltdl/config/ltmain.m4sh
index 0418007..6eb13f0 100644
--- a/libltdl/config/ltmain.m4sh
+++ b/libltdl/config/ltmain.m4sh
@@ -4268,9 +4268,14 @@ void lt_dump_script (FILE* f)
{
EOF
func_emit_wrapper yes |
- $SED -e 's/\([\\]\)/\\\1/g' \
- -e 's/^/ fputs (/' -e 's/$/\\n, f);/'
-
+ $SED -n -e '
+s/^\(.\{79\}\)\(..*\)/\1\n\2/
+h
+s/\([\\]\)/\\\1/g
+s/$/\\n/
+s/\([^\n]*\).*/ fputs (\1, f);/p
+g
+D'
cat EOF
}
EOF
--
1.7.1
