Bug 50334 - APR_RESTORE_THE_ENVIRONMENT clobbers CPPFLAGS, LDFLAGS, LIBS, CFLAGS, INCLUDES when set to a single space
Summary: APR_RESTORE_THE_ENVIRONMENT clobbers CPPFLAGS, LDFLAGS, LIBS, CFLAGS, INCLUDE...
Status: CLOSED FIXED
Alias: None
Product: APR
Classification: Unclassified
Component: APR (show other bugs)
Version: HEAD
Hardware: PC Linux
: P2 blocker with 3 votes (vote)
Target Milestone: ---
Assignee: Apache Portable Runtime bugs mailinglist
URL:
Keywords: FixedInTrunk, PatchAvailable
Depends on:
Blocks:
 
Reported: 2010-11-25 05:49 UTC by eugaia
Modified: 2014-01-21 13:03 UTC (History)
2 users (show)



Attachments
Patch to correctly handle *empty* value in variables like $LIBS under srclib/apr where the value is a space rather than completely empty. (173 bytes, patch)
2010-11-25 14:22 UTC, eugaia
Details | Diff
apr-r1152852-build-flags-extra.patch (418 bytes, patch)
2011-09-19 03:31 UTC, Nathan Phillip Brink (binki)
Details | Diff
alternate patch to preserve the space (346 bytes, patch)
2011-09-20 21:45 UTC, Stefan Fritsch
Details | Diff
apr-r1152852-build-flags-extra-r1.patch (393 bytes, patch)
2011-09-21 00:23 UTC, Nathan Phillip Brink (binki)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description eugaia 2010-11-25 05:49:01 UTC
When compiling using :

../httpd-2.2.17/configure \
    --prefix=/path/to/install/dir \
    --enable-proxy \
    --enable-proxy-connect \
    --enable-proxy-ftp \
    --enable-proxy-http \
    --enable-ssl \
    --with-ssl=/path/to/openssl

The compilation fails because somewhere libs '-lrt-lcrypt' is added to the EXTRA_LIBS variable (rather than '-lrt -lcrypt'). This causes the following message :

--------------
/bin/bash /root/src/httpd-2.2.17-build/srclib/apr/libtool --silent --mode=link  gcc -pthread   -DHAVE_CONFIG_H -DLINUX=2 -D_REENTRANT -D_GNU_SOURCE -D_LARGEFILE64_SOURCE   -I./include -I/root/src/httpd-2.2.17/srclib/apr/include/arch/unix -I./include/arch/unix -I/root/src/httpd-2.2.17/srclib/apr/include/arch/unix -I/root/src/httpd-2.2.17/srclib/apr/include  -version-info 4:2:4    -o libapr-1.la -rpath /usr/local/simpl/apache/2.2.17/lib passwd/apr_getpass.lo strings/apr_cpystrn.lo strings/apr_strtok.lo strings/apr_snprintf.lo strings/apr_strnatcmp.lo strings/apr_fnmatch.lo strings/apr_strings.lo tables/apr_tables.lo tables/apr_hash.lo dso/unix/dso.lo file_io/unix/mktemp.lo file_io/unix/seek.lo file_io/unix/copy.lo file_io/unix/filedup.lo file_io/unix/dir.lo file_io/unix/flock.lo file_io/unix/buffer.lo file_io/unix/filepath_util.lo file_io/unix/readwrite.lo file_io/unix/open.lo file_io/unix/fileacc.lo file_io/unix/tempdir.lo file_io/unix/pipe.lo file_io/unix/filepath.lo file_io/unix/filestat.lo file_io/unix/fullrw.lo locks/unix/thread_rwlock.lo locks/unix/thread_mutex.lo locks/unix/global_mutex.lo locks/unix/proc_mutex.lo locks/unix/thread_cond.lo memory/unix/apr_pools.lo misc/unix/rand.lo misc/unix/start.lo misc/unix/otherchild.lo misc/unix/getopt.lo misc/unix/env.lo misc/unix/version.lo misc/unix/charset.lo misc/unix/errorcodes.lo mmap/unix/common.lo mmap/unix/mmap.lo network_io/unix/socket_util.lo network_io/unix/inet_ntop.lo network_io/unix/inet_pton.lo network_io/unix/sockets.lo network_io/unix/sockaddr.lo network_io/unix/multicast.lo network_io/unix/sockopt.lo network_io/unix/sendrecv.lo poll/unix/pollcb.lo poll/unix/port.lo poll/unix/select.lo poll/unix/epoll.lo poll/unix/pollset.lo poll/unix/kqueue.lo poll/unix/poll.lo random/unix/apr_random.lo random/unix/sha2.lo random/unix/sha2_glue.lo shmem/unix/shm.lo support/unix/waitio.lo threadproc/unix/procsup.lo threadproc/unix/threadpriv.lo threadproc/unix/proc.lo threadproc/unix/thread.lo threadproc/unix/signals.lo time/unix/timestr.lo time/unix/time.lo user/unix/userinfo.lo user/unix/groupinfo.lo atomic/unix/mutex.lo atomic/unix/solaris.lo atomic/unix/ia32.lo atomic/unix/s390.lo atomic/unix/builtins.lo atomic/unix/ppc.lo   -lrt-lcrypt -lpthread -ldl
/usr/bin/ld: cannot find -lrt-lcrypt
-----------

The value EXTRA_LIBS=-lrt-lcrypt can be found in the config.log.

Note : the path /path/to/openssl used is not a valid path to OpenSSL (I've had troubles compiling that too). I'm not sure if a valid SSL path would work properly because configure would add the correct libs. (I'll add additional information here when I've tested.)
Comment 1 Rainer Jung 2010-11-25 05:58:57 UTC
So if you don't have OpenSSL available, why don't you simply use

   --without-ssl

instead of

   --with-ssl=/path/to/openssl

with a broken path? Does it work using the "without" syntax?
Comment 2 eugaia 2010-11-25 06:19:12 UTC
(In reply to comment #1)
> So if you don't have OpenSSL available, why don't you simply use
> 
>    --without-ssl
> 
> instead of
> 
>    --with-ssl=/path/to/openssl
> 
> with a broken path? Does it work using the "without" syntax?

I want to use a custom path to OpenSSL, but because of issues with the latest version of OpenSSL (1.0.0b), I'm having difficulties in both compiling it and compiling Apache with it.

Although logically there is no difference to --without-ssl and providing a non-working path, it may be that the bug lies in ./configure when testing for an OpenSSL path that isn't valid. Just having --without-ssl may not reproduce the bug (though I'm in the process of testing it to see if that's the case, and will report back here).

I'm using a virtualized system for testing, so each compilation test takes quite a long time, and I wanted to report the bug early and add extra info as I'd tested.
Comment 3 eugaia 2010-11-25 07:11:14 UTC
I've now tested with a valid path to OpenSSL (/usr/local/openssl/default), and get the following information provided by ./configure :

-------
checking for SSL/TLS toolkit base... /usr/local/openssl/default
  adding "-I/usr/local/openssl/default/include" to CPPFLAGS
  adding "-I/usr/local/openssl/default/include" to INCLUDES
  adding "-L/usr/local/openssl/default/lib" to LDFLAGS
checking for OpenSSL version... checking openssl/opensslv.h usability... yes
checking openssl/opensslv.h presence... yes
checking for openssl/opensslv.h... yes
checking openssl/ssl.h usability... yes
checking openssl/ssl.h presence... yes
checking for openssl/ssl.h... yes
OK
  forcing SSL_LIBS to "-lssl -lcrypto  -lrt-lcrypt -lpthread -ldl"
  adding "-lssl" to LIBS
  adding "-lcrypto" to LIBS
  adding "-lrt-lcrypt" to LIBS
  adding "-lpthread" to LIBS
  adding "-ldl" to LIBS
checking openssl/engine.h usability... yes
checking openssl/engine.h presence... yes
checking for openssl/engine.h... yes
checking for SSLeay_version... no
checking for SSL_CTX_new... no
checking for ENGINE_init... no
checking for ENGINE_load_builtin_engines... no
checking for SSL_set_cert_store... no
configure: error: ... Error, SSL/TLS libraries were missing or unusable
-------------

The problem is in the line :

  forcing SSL_LIBS to "-lssl -lcrypto  -lrt-lcrypt -lpthread -ldl"

From looking at config.log, the configure test (at least for SSL_set_cert_store) include these libs, and so fail.

I'll see if I can track down where -lrt-lcrypt is being added.
Comment 4 eugaia 2010-11-25 13:25:43 UTC
I have tracked the problem to file $src_dir/srclib/apr/configure line 48747 (in version 2.2.17) : 

    EXTRA_LIBS=`echo $LIBS | sed -e "s%${apr_ste_save_LIBS}%%"`

The issue is that $apr_ste_save_LIBS = ' ', so the first space in $LIBS is translated to nothing, hence joining '-lrt -lcrypt' to make '-lrt-lcrypt'.

There appear to be two solutions to this :

1 - guarantee that $apr_ste_save_LIBS does not = ' '
2 - handle the case when $apr_ste_save_LIBS = ' '

Given that this code is generate from a template, and given that it's likely to be less error-prone in the future, it seems to make more sense to me to do 2.

Having a quick look at the configure.in file, it appears that the code is generated from the APR_RESTORE_THE_ENVIRONMENT() macro.

I'll try to track down that, and post a patch here soon.
Comment 5 eugaia 2010-11-25 14:22:32 UTC
Created attachment 26347 [details]
Patch to correctly handle *empty* value in variables like $LIBS under srclib/apr where the value is a space rather than completely empty.
Comment 6 eugaia 2010-11-25 14:25:21 UTC
The posted patch fixes this issue.  Until it is merged into the main distribution, anyone applying the patch should run $src_dir/srclib/apr/buildconf after applying the patch before re-running ./configure.
Comment 7 eugaia 2010-11-25 14:34:33 UTC
(In reply to comment #6)

i.e.

cd $src_dir/srclib/apr && ./buildconf

Just executing in one command won't work.
Comment 8 Nathan Phillip Brink (binki) 2011-09-19 03:31:13 UTC
Created attachment 27532 [details]
apr-r1152852-build-flags-extra.patch

This is still a hacky method to handle the case where CPPFLAGS=' '. But it fixes the issue.

See Gentoo bug #383549 (https://bugs.gentoo.org/383549) where -D_GNU_SOURCE is clobbered against -D_REENTRANT, resulting in `-D_REENTRANT-D_GNU_SOURCE'.
Comment 9 Nathan Phillip Brink (binki) 2011-09-19 03:37:27 UTC
Example 2, https://bugs.gentoo.org/383549 :

checking for a sed that does not truncate output... /bin/sed
Applying APR hints file rules for x86_64-pc-linux-gnu
  adding "-D_REENTRANT" to CPPFLAGS
  adding "-D_GNU_SOURCE" to CPPFLAGS
(Default will be unix)
checking whether make sets $(MAKE)... yes
...
Restore user-defined environment settings...
  restoring CPPFLAGS to " "
  setting EXTRA_CPPFLAGS to "-D_REENTRANT-D_GNU_SOURCE"
  restoring CFLAGS to "-march=nocona -O2 -pipe "
  setting EXTRA_CFLAGS to "-pthread"
  restoring LDFLAGS to "-Wl,-O1 -Wl,--as-needed "
  setting EXTRA_LDFLAGS to ""
  restoring LIBS to ""
  setting EXTRA_LIBS to "-luuid -lrt -lcrypt  -lpthread -ldl"
  restoring INCLUDES to ""
  setting EXTRA_INCLUDES to ""
...
/bin/bash /usr/bin/libtool --silent --mode=compile x86_64-pc-linux-gnu-gcc -pthread  -march=nocona -O2 -pipe  -DHAVE_CONFIG_H -D_REENTRANT-D_GNU_SOURCE   -I./include -I/var/tmp/portage/dev-libs/apr-1.4.5/work/apr-1.4.5/include/arch/unix -I./include/arch/unix -I/var/tmp/portage/dev-libs/apr-1.4.5/work/apr-1.4.5/include/arch/unix -I/var/tmp/portage/dev-libs/apr-1.4.5/work/apr-1.4.5/include  -o locks/unix/proc_mutex.lo -c locks/unix/proc_mutex.c && touch locks/unix/proc_mutex.lo
<command-line>:0:11: warning: missing whitespace after the macro name
<command-line>:0:0: warning: "_REENTRANT" redefined
<command-line>:0:0: note: this is the location of the previous definition
/bin/bash /usr/bin/libtool --silent --mode=compile x86_64-pc-linux-gnu-gcc -pthread  -march=nocona -O2 -pipe  -DHAVE_CONFIG_H -D_REENTRANT-D_GNU_SOURCE   -I./include -I/var/tmp/portage/dev-libs/apr-1.4.5/work/apr-1.4.5/include/arch/unix -I./include/arch/unix -I/var/tmp/portage/dev-libs/apr-1.4.5/work/apr-1.4.5/include/arch/unix -I/var/tmp/portage/dev-libs/apr-1.4.5/work/apr-1.4.5/include  -o locks/unix/thread_cond.lo -c locks/unix/thread_cond.c && touch locks/unix/thread_cond.lo
<command-line>:0:11: warning: missing whitespace after the macro name
<command-line>:0:0: warning: "_REENTRANT" redefined
<command-line>:0:0: note: this is the location of the previous definition
/bin/bash /usr/bin/libtool --silent --mode=compile x86_64-pc-linux-gnu-gcc -pthread  -march=nocona -O2 -pipe  -DHAVE_CONFIG_H -D_REENTRANT-D_GNU_SOURCE   -I./include -I/var/tmp/portage/dev-libs/apr-1.4.5/work/apr-1.4.5/include/arch/unix -I./include/arch/unix -I/var/tmp/portage/dev-libs/apr-1.4.5/work/apr-1.4.5/include/arch/unix -I/var/tmp/portage/dev-libs/apr-1.4.5/work/apr-1.4.5/include  -o locks/unix/thread_mutex.lo -c locks/unix/thread_mutex.c && touch locks/unix/thread_mutex.lo
<command-line>:0:11: warning: missing whitespace after the macro name
<command-line>:0:0: warning: "_REENTRANT" redefined
<command-line>:0:0: note: this is the location of the previous definition
/bin/bash /usr/bin/libtool --silent --mode=compile x86_64-pc-linux-gnu-gcc -pthread  -march=nocona -O2 -pipe  -DHAVE_CONFIG_H -D_REENTRANT-D_GNU_SOURCE   -I./include -I/var/tmp/portage/dev-libs/apr-1.4.5/work/apr-1.4.5/include/arch/unix -I./include/arch/unix -I/var/tmp/portage/dev-libs/apr-1.4.5/work/apr-1.4.5/include/arch/unix -I/var/tmp/portage/dev-libs/apr-1.4.5/work/apr-1.4.5/include  -o locks/unix/thread_rwlock.lo -c locks/unix/thread_rwlock.c && touch locks/unix/thread_rwlock.lo
locks/unix/proc_mutex.c: In function 'proc_mutex_proc_pthread_create':
locks/unix/proc_mutex.c:393:53: error: 'PTHREAD_PRIO_INHERIT' undeclared (first use in this function)
locks/unix/proc_mutex.c:393:53: note: each undeclared identifier is reported only once for each function it appears in
make[1]: *** [locks/unix/proc_mutex.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
<command-line>:0:11: warning: missing whitespace after the macro name
<command-line>:0:0: warning: "_REENTRANT" redefined
<command-line>:0:0: note: this is the location of the previous definition
<command-line>:0:11: warning: missing whitespace after the macro name
<command-line>:0:0: warning: "_REENTRANT" redefined
<command-line>:0:0: note: this is the location of the previous definition
make[1]: Leaving directory `/var/tmp/portage/dev-libs/apr-1.4.5/work/apr-1.4.5'
make: *** [all-recursive] Error 1
emake failed


This is all caused by CPPFLAGS=" " .
Comment 10 Stefan Fritsch 2011-09-20 21:45:42 UTC
Created attachment 27545 [details]
alternate patch to preserve the space

I think the problem is more that the leading space is lost due to missing quoting.  Does this patch work, too?
Comment 11 Nathan Phillip Brink (binki) 2011-09-21 00:22:24 UTC
Comment on attachment 27545 [details]
alternate patch to preserve the space

This patch fixes this particular bug but does not solve the root problem. ./configure need to treat the user's CPPFLAGS value as a string potentially containing any number of any sorts of characters in it.

- The user may use the '%' character in CPPFLAGS, changing the action of the sed expression.

- The user may use BRE metacharacters, such as `.', `*', `[', or `\', in CPPFLAGS. sed's s command is not a strict string find-and-replace mechanism, it first matches BREs.

Of course, I cannot think of an obvious reason for the user to do any of these things. But leaving ./configure _this_ obviously sensitive to the contents of any of the *FLAGS seems to be asking for further trouble similar in nature to this bug.

My patch, on the other hand, follows POSIX shell rules and it handles any string... though I still use sed which feels like overkill for this...
Comment 12 Nathan Phillip Brink (binki) 2011-09-21 00:23:07 UTC
Created attachment 27550 [details]
apr-r1152852-build-flags-extra-r1.patch

This version avoids sed completely.
Comment 13 Stefan Fritsch 2011-10-15 21:18:24 UTC
r1183686, r1183710, r1183711

will be in 1.4.6
Comment 14 Rainer Jung 2011-11-08 03:10:14 UTC
Switched back to using sed because /bin/sh on Solaris doesn't support ${A#B}.

The whitespace problem should be still fixed.
Comment 15 Nathan Phillip Brink (binki) 2011-11-08 03:13:11 UTC
(In reply to comment #14)
> Switched back to using sed because /bin/sh on Solaris doesn't support ${A#B}.

May you please point to the revisions this change is made in? Thanks.
Comment 16 Rainer Jung 2011-11-08 07:23:09 UTC
Revisions r1199072 (trunk), r1199076 (1.5.x) and 1199078 (1.4.x).
Comment 17 Jeff Trawick 2014-01-21 13:03:41 UTC
Fixed in 1.4.6 and later releases