Re: cwrapper generates long strings.

2010-10-03 Thread Ralf Wildenhues
Hi Peter,

* Peter Rosin wrote on Sat, Oct 02, 2010 at 11:33:02PM CEST:
> Den 2010-10-02 13:53 skrev Ralf Wildenhues:
> > Yes.  Well, we might get the odd report about the Cygwin non-binmode
> > mount where the CR NL messes up things, but otherwise, it should work.
> 
> If you are on a text mount, it should fixup CR NL issues.  If you have
> CR NL files on a binary mount you deserve to suffer.  So, a non-issue.

Cool.  Thanks.

> I used 250 at the limit in the test as the 79 characters from the string
> splitter in ltmain might be doubled due to C string escapes and then I
> added some extra margin for quotes and the ", f);" trailer.  Still below
> 255 which might be an arbitrary limit in some grep implementations.

You can assume close to 2K as minimum limit for grep.
(Hmm, wonder why we didn't ever write down the value in autoconf.texi)

The patch is OK with nits addressed.

> --- a/tests/cwrapper.at
> +++ b/tests/cwrapper.at
> @@ -134,3 +134,53 @@ done
>  
>  AT_CLEANUP
>  
> +
> +AT_SETUP([cwrapper string length])
> +
> +eval "`$LIBTOOL --config | $EGREP '^(objdir)='`"
> +
> +AT_DATA([liba.c],
> +[[int liba_func1 (int arg)
> +{
> +  return arg + 1;
> +}
> +]])
> +AT_DATA([usea.c],
> +[[extern int liba_func1 (int arg);
> +int main (void)
> +{
> +  int a = 2;
> +  int b = liba_func1 (a);
> +  if (b == 3) return 0;
> +  return 1;
> +}
> +]])
> +
> +AT_CHECK([$LIBTOOL --mode=compile $CC $CPPFLAGS $CFLAGS -c liba.c],
> +  [], [ignore], [ignore])
> +AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -no-undefined ]dnl
> +  [-o liba.la -rpath /foo liba.lo],
> +  [], [ignore], [ignore])
> +AT_CHECK([$CC $CPPFLAGS $CFLAGS -c usea.c],
> +  [], [ignore], [ignore])
> +
> +# make sure PATH is at least 250 chars, should force line breaks in lt-usea.c
> +for i in 25 50 75 100 125 150 175 200 225 250
> +do
> +  PATH="$PATH$PATH_SEPARATOR/somewhere-that-eksists-not"

Intended typo 'exists'?  ;-)

Does $LIBTOOL or the autotest machinery ever intentionally try to run
commands that won't exist in $PATH within this shell?  If so, then we
might get the odd bug report from security-hardened distributions that
complain about read or execute accessses to non-allowed parts of the
file system.

This length doesn't yet trigger the compiler string literal length
limit; not sure whether it should?

Do we have to cater to the case where the environment is very large
already?  I'm not sure, we might want to deal with it when crossing
that bridge only, if it's hard to know off-hand.

> +done
> +export PATH
> +
> +AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -no-fast-install ]dnl
> +  [-o usea$EXEEXT usea.$OBJEXT liba.la],
> +  [], [ignore], [ignore])
> +
> +# skip if no cwrapper is generated
> +AT_CHECK([test -f $objdir/lt-usea.c || exit 77])
> +
> +# try to make sure the test is relevant
> +AT_CHECK([grep ' *fputs' $objdir/lt-usea.c > /dev/null])

Rather than redirecting stdout, put [ignore] in the third argument.

> +# check for no overly long fputs
> +AT_CHECK([grep ' *fputs.\{250\}' $objdir/lt-usea.c], [1])
> +
> +AT_CLEANUP

Cheers,
Ralf



Re: cwrapper generates long strings.

2010-10-03 Thread Peter Rosin
Den 2010-10-03 09:01 skrev Ralf Wildenhues:
>> I used 250 at the limit in the test as the 79 characters from the string
>> splitter in ltmain might be doubled due to C string escapes and then I
>> added some extra margin for quotes and the ", f);" trailer.  Still below
>> 255 which might be an arbitrary limit in some grep implementations.
> 
> You can assume close to 2K as minimum limit for grep.
> (Hmm, wonder why we didn't ever write down the value in autoconf.texi)
> 
> The patch is OK with nits addressed.
> 
>> --- a/tests/cwrapper.at
>> +++ b/tests/cwrapper.at
>> @@ -134,3 +134,53 @@ done
>>  
>>  AT_CLEANUP
>>  
>> +
>> +AT_SETUP([cwrapper string length])
>> +
>> +eval "`$LIBTOOL --config | $EGREP '^(objdir)='`"
>> +
>> +AT_DATA([liba.c],
>> +[[int liba_func1 (int arg)
>> +{
>> +  return arg + 1;
>> +}
>> +]])
>> +AT_DATA([usea.c],
>> +[[extern int liba_func1 (int arg);
>> +int main (void)
>> +{
>> +  int a = 2;
>> +  int b = liba_func1 (a);
>> +  if (b == 3) return 0;
>> +  return 1;
>> +}
>> +]])
>> +
>> +AT_CHECK([$LIBTOOL --mode=compile $CC $CPPFLAGS $CFLAGS -c liba.c],
>> + [], [ignore], [ignore])
>> +AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -no-undefined ]dnl
>> + [-o liba.la -rpath /foo liba.lo],
>> + [], [ignore], [ignore])
>> +AT_CHECK([$CC $CPPFLAGS $CFLAGS -c usea.c],
>> + [], [ignore], [ignore])
>> +
>> +# make sure PATH is at least 250 chars, should force line breaks in 
>> lt-usea.c
>> +for i in 25 50 75 100 125 150 175 200 225 250
>> +do
>> +  PATH="$PATH$PATH_SEPARATOR/somewhere-that-eksists-not"
> 
> Intended typo 'exists'?  ;-)

Yes :-)

> Does $LIBTOOL or the autotest machinery ever intentionally try to run
> commands that won't exist in $PATH within this shell?

I don't think so, and it is a really hard question to answer too.

>  If so, then we
> might get the odd bug report from security-hardened distributions that
> complain about read or execute accessses to non-allowed parts of the
> file system.

Using "$PATH$PATH_SEPARATOR$PATH" seemed like a much quicker way
to make the length explode so I didn't like that.

> This length doesn't yet trigger the compiler string literal length
> limit; not sure whether it should?

That's not reliable anyway as most compilers support so long strings
that it's hard to tickle it. On a tangent, another bug is that linking
doesn't fail (libtool returns zero) when building the cwrapper fails.
I think that's a separate issue from this one, which is why I haven't
mixed them up previously.  Another nit in that area is that there are
multiple levels of "$opt_dry_run || {" which seems superfluous, but
that might be intentional in order to keep the code copy-paste-safe?

> Do we have to cater to the case where the environment is very large
> already?  I'm not sure, we might want to deal with it when crossing
> that bridge only, if it's hard to know off-hand.

Are your three above paragraphs nits and part of what I need to
address, or just notes for the future?

>> +done
>> +export PATH
>> +
>> +AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -no-fast-install ]dnl
>> + [-o usea$EXEEXT usea.$OBJEXT liba.la],
>> + [], [ignore], [ignore])
>> +
>> +# skip if no cwrapper is generated
>> +AT_CHECK([test -f $objdir/lt-usea.c || exit 77])
>> +
>> +# try to make sure the test is relevant
>> +AT_CHECK([grep ' *fputs' $objdir/lt-usea.c > /dev/null])
> 
> Rather than redirecting stdout, put [ignore] in the third argument.

Certainly possible, but I didn't think anyone would be interested in a
couple of hundred lines of boilerplate in the log.  I don't really care
though, so if you still think [ignore] is a good idea, then ok.

>> +# check for no overly long fputs
>> +AT_CHECK([grep ' *fputs.\{250\}' $objdir/lt-usea.c], [1])
>> +
>> +AT_CLEANUP

Cheers,
Peter



[PATCH] Add test case for 69e77671 (cwrapper PATH manipulation order)

2010-10-03 Thread libtool
* tests/cwrapper.at: Add new test 'cwrapper and installed shared
libraries.'
---
This patch was actually proposed by Roumen Petrov here:
http://lists.gnu.org/archive/html/bug-libtool/2009-12/msg00037.html

He mentioned here:
http://lists.gnu.org/archive/html/libtool-patches/2010-09/msg00222.html
that 69e77671 would actually fix the error exposed by this test. I
ran this test both with and without 69e77671; without 69e77671 this
test fails (cygwin), but with it the new test passes.  Examination
shows that Roumen's test is exactly what is needed to verify that
the problem fixed by 69e77671 does not regress; his test explicitly
verifies that a newly built DLL (more generally, shared library)
is used at runtime in preference to an installed version.


 tests/cwrapper.at |   70 +
 1 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/tests/cwrapper.at b/tests/cwrapper.at
index 248c0c0..ff3d04f 100644
--- a/tests/cwrapper.at
+++ b/tests/cwrapper.at
@@ -134,3 +134,73 @@ done
 
 AT_CLEANUP
 
+
+AT_SETUP([cwrapper and installed shared libraries])
+AT_KEYWORDS([libtool])
+
+# make sure existing libtool is configured for shared libraries
+AT_CHECK([$LIBTOOL --features | grep 'disable shared libraries' && exit 77],
+[1], [ignore])
+
+# FIXME with shared_fails for this test on AIX
+# copy from link-order2.at:
+eval `$LIBTOOL --config | $EGREP '^(shlibpath_var|allow_undefined_flag)='`
+
+undefined_setting=-no-undefined
+shared_fails=no
+case $host_os,$LDFLAGS,$allow_undefined_flag in
+aix*,*-brtl*,*) ;;
+aix*) shared_fails=yes ;;
+darwin*,*,*-flat_namespace*) undefined_setting= ;;
+darwin*,*,*) shared_fails=yes ;;
+esac
+# end of copy from link-order2.at
+
+LDFLAGS="$LDFLAGS $undefined_setting"
+
+inst=`pwd`/inst
+libdir=$inst/lib
+bindir=$inst/bin
+mkdir $inst $libdir $bindir
+
+# we must build foo library in a separate directory to avoid on some
+# platforms shared library to be loaded from "current" directory
+
+mkdir foo
+cd foo
+# build and install "old" library version
+AT_DATA([a.c], [[
+int liba_ver (void) { return(1); }
+]])
+$LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS -c a.c
+$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -version-info=0.0.0 -o liba.la 
-rpath $libdir a.lo
+$LIBTOOL --mode=install $lt_INSTALL liba.la $libdir
+
+# build a new library version
+AT_DATA([a.c], [[
+int liba_ver (void) { return(2); }
+]])
+$LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS -c a.c
+$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -version-info=0.0.0 -o liba.la 
-rpath $libdir a.lo
+
+cd ..
+
+# build and run test application
+AT_DATA([m.c], [[
+extern int liba_ver (void);
+int main (void)
+{
+  int r = (liba_ver () == 2) ? 0 : 1;
+  return(r);
+}
+]])
+
+$LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS -c m.c
+
+$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o m1$EXEEXT m.$OBJEXT foo/liba.la
+LT_AT_EXEC_CHECK([./m1], [0], [ignore], [ignore], [])
+
+$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o m2$EXEEXT m.$OBJEXT foo/liba.la 
-L$inst/lib
+LT_AT_EXEC_CHECK([./m2], [0], [ignore], [ignore], [])
+
+AT_CLEANUP
-- 
1.7.1