Thread: Should libedit be preferred to libreadline?

Should libedit be preferred to libreadline?

From
Seneca Cunningham
Date:
It would certainly seem so on AIX.

In tracking down why postgres 8.x would segfault on AIX 5.3, it became
apparent to me that libreadline.a is a problematic library to link
against and that libedit.a is preferable (and for reasons other than
that readline is GPL while postgres is BSD-licensed).

With AIX 5, the easiest way to get a shared object is to pass "-bexpall"
to the linker.  This results in all symbols being exported.  The problem
with this is that the linker will export all of libreadline's and
libhistory's symbols.  In the case of libreadline.so.4 (and .5) on AIX 5
this includes symbols like strncpy and memmove, but on .4, not memcpy.
This is likely because libc.a does not export them.

What results from this is that when postgres is linked against readline
on AIX, it gets these memory functions through readline instead of its
own code.  When readline 4.3 is used (what IBM provides in their "AIX
Toolbox for Linux"), postgres is known to crash.  These segfaults (if
postgres was compiled with gcc) have occurred on AIX 5.3ML3, AIX 5.3ML1,
and AIX 5.2ML7.  With readline 5.0, postgres merely gets these functions
through the shared library memory segments instead of the user memory
segments[6].

While it is possible to build libreadline in a manner that doesn't
export strncpy, neither of the prebuilt readlines for AIX 5 that I
checked were both shared and did not export strncpy.  IBM's readline[5]
exports strncpy, UCLA's readline[4] is static.  Building a shared
readline that doesn't export strncpy requires creating export files for
libreadline and libhistory that only list the symbols that they are
supposed to export and editing the shared library Makefile to add the
exports flags to the appropriate linker calls.

Whatever strategy we might take, using readline on AIX requires
considerable trickery and hacking around with the build environments.
Simply put, it's ghastly.

On the other hand, the port of NetBSD's editline that I tried[1] works
without build-hackery to the library and has reasonable exports.  The
only changes to postgres that I needed to make were confined to telling
the configure script to check for libedit before libreadline and adding
a test for histedit.h.  The attached patch contains my modifications.

It is also possible to use a wrapper like rlwrap[2] instead of linking
postgres against libreadline or libedit.

[1] port of NetBSD's editline
    http://www.thrysoee.dk/editline/
[2] rlwrap
    http://utopia.knoware.nl/~hlub/uck/software/
[3] IBM Redbook "AIX 5L Porting Guide", section 9.2
    http://www.redbooks.ibm.com/abstracts/sg246034.html?Open
    http://www.redbooks.ibm.com/redbooks/pdfs/sg246034.pdf
[4] UCLA's readline package
    http://aixpdslib.seas.ucla.edu/packages/readline.html
[5] IBM's readline package
    http://www-03.ibm.com/servers/aix/products/aixos/linux/download.html
[6] IBM Redbook "Developing and Porting C and C++ Applications on AIX",
      page 110
    http://www.redbooks.ibm.com/abstracts/sg245674.html?Open
    http://www.redbooks.ibm.com/redbooks/pdfs/sg245674.pdf

--
Seneca Cunningham
scunning@ca.afilias.info

diff -wu postgresql-8.1.0.orig/configure postgresql-8.1.0/configure
--- postgresql-8.1.0.orig/configure    2005-11-04 23:01:38.000000000 -0500
+++ postgresql-8.1.0/configure    2005-11-21 12:47:28.000000000 -0500
@@ -998,7 +998,7 @@
     else
       echo "$as_me: WARNING: no configuration information is in $ac_dir" >&2
     fi
-    cd $ac_popdir
+    cd "$ac_popdir"
   done
 fi

@@ -6498,7 +6498,7 @@
 else
   pgac_cv_check_readline=no
 pgac_save_LIBS=$LIBS
-for pgac_rllib in -lreadline -ledit ; do
+for pgac_rllib in -ledit -lreadline ; do
   for pgac_lib in "" " -ltermcap" " -lncurses" " -lcurses" ; do
     LIBS="${pgac_rllib}${pgac_lib} $pgac_save_LIBS"
     cat >conftest.$ac_ext <<_ACEOF
@@ -9646,6 +9646,152 @@

 else

+for ac_header in histedit.h
+do
+as_ac_Header=`echo "ac_cv_header_$ac_header" | $as_tr_sh`
+if eval "test \"\${$as_ac_Header+set}\" = set"; then
+  echo "$as_me:$LINENO: checking for $ac_header" >&5
+echo $ECHO_N "checking for $ac_header... $ECHO_C" >&6
+if eval "test \"\${$as_ac_Header+set}\" = set"; then
+  echo $ECHO_N "(cached) $ECHO_C" >&6
+fi
+echo "$as_me:$LINENO: result: `eval echo '${'$as_ac_Header'}'`" >&5
+echo "${ECHO_T}`eval echo '${'$as_ac_Header'}'`" >&6
+else
+  # Is the header compilable?
+echo "$as_me:$LINENO: checking $ac_header usability" >&5
+echo $ECHO_N "checking $ac_header usability... $ECHO_C" >&6
+cat >conftest.$ac_ext <<_ACEOF
+/* confdefs.h.  */
+_ACEOF
+cat confdefs.h >>conftest.$ac_ext
+cat >>conftest.$ac_ext <<_ACEOF
+/* end confdefs.h.  */
+$ac_includes_default
+#include <$ac_header>
+_ACEOF
+rm -f conftest.$ac_objext
+if { (eval echo "$as_me:$LINENO: \"$ac_compile\"") >&5
+  (eval $ac_compile) 2>conftest.er1
+  ac_status=$?
+  grep -v '^ *+' conftest.er1 >conftest.err
+  rm -f conftest.er1
+  cat conftest.err >&5
+  echo "$as_me:$LINENO: \$? = $ac_status" >&5
+  (exit $ac_status); } &&
+     { ac_try='test -z "$ac_c_werror_flag"             || test ! -s conftest.err'
+  { (eval echo "$as_me:$LINENO: \"$ac_try\"") >&5
+  (eval $ac_try) 2>&5
+  ac_status=$?
+  echo "$as_me:$LINENO: \$? = $ac_status" >&5
+  (exit $ac_status); }; } &&
+     { ac_try='test -s conftest.$ac_objext'
+  { (eval echo "$as_me:$LINENO: \"$ac_try\"") >&5
+  (eval $ac_try) 2>&5
+  ac_status=$?
+  echo "$as_me:$LINENO: \$? = $ac_status" >&5
+  (exit $ac_status); }; }; then
+  ac_header_compiler=yes
+else
+  echo "$as_me: failed program was:" >&5
+sed 's/^/| /' conftest.$ac_ext >&5
+
+ac_header_compiler=no
+fi
+rm -f conftest.err conftest.$ac_objext conftest.$ac_ext
+echo "$as_me:$LINENO: result: $ac_header_compiler" >&5
+echo "${ECHO_T}$ac_header_compiler" >&6
+
+# Is the header present?
+echo "$as_me:$LINENO: checking $ac_header presence" >&5
+echo $ECHO_N "checking $ac_header presence... $ECHO_C" >&6
+cat >conftest.$ac_ext <<_ACEOF
+/* confdefs.h.  */
+_ACEOF
+cat confdefs.h >>conftest.$ac_ext
+cat >>conftest.$ac_ext <<_ACEOF
+/* end confdefs.h.  */
+#include <$ac_header>
+_ACEOF
+if { (eval echo "$as_me:$LINENO: \"$ac_cpp conftest.$ac_ext\"") >&5
+  (eval $ac_cpp conftest.$ac_ext) 2>conftest.er1
+  ac_status=$?
+  grep -v '^ *+' conftest.er1 >conftest.err
+  rm -f conftest.er1
+  cat conftest.err >&5
+  echo "$as_me:$LINENO: \$? = $ac_status" >&5
+  (exit $ac_status); } >/dev/null; then
+  if test -s conftest.err; then
+    ac_cpp_err=$ac_c_preproc_warn_flag
+    ac_cpp_err=$ac_cpp_err$ac_c_werror_flag
+  else
+    ac_cpp_err=
+  fi
+else
+  ac_cpp_err=yes
+fi
+if test -z "$ac_cpp_err"; then
+  ac_header_preproc=yes
+else
+  echo "$as_me: failed program was:" >&5
+sed 's/^/| /' conftest.$ac_ext >&5
+
+  ac_header_preproc=no
+fi
+rm -f conftest.err conftest.$ac_ext
+echo "$as_me:$LINENO: result: $ac_header_preproc" >&5
+echo "${ECHO_T}$ac_header_preproc" >&6
+
+# So?  What about this header?
+case $ac_header_compiler:$ac_header_preproc:$ac_c_preproc_warn_flag in
+  yes:no: )
+    { echo "$as_me:$LINENO: WARNING: $ac_header: accepted by the compiler, rejected by the preprocessor!" >&5
+echo "$as_me: WARNING: $ac_header: accepted by the compiler, rejected by the preprocessor!" >&2;}
+    { echo "$as_me:$LINENO: WARNING: $ac_header: proceeding with the compiler's result" >&5
+echo "$as_me: WARNING: $ac_header: proceeding with the compiler's result" >&2;}
+    ac_header_preproc=yes
+    ;;
+  no:yes:* )
+    { echo "$as_me:$LINENO: WARNING: $ac_header: present but cannot be compiled" >&5
+echo "$as_me: WARNING: $ac_header: present but cannot be compiled" >&2;}
+    { echo "$as_me:$LINENO: WARNING: $ac_header:     check for missing prerequisite headers?" >&5
+echo "$as_me: WARNING: $ac_header:     check for missing prerequisite headers?" >&2;}
+    { echo "$as_me:$LINENO: WARNING: $ac_header: see the Autoconf documentation" >&5
+echo "$as_me: WARNING: $ac_header: see the Autoconf documentation" >&2;}
+    { echo "$as_me:$LINENO: WARNING: $ac_header:     section \"Present But Cannot Be Compiled\"" >&5
+echo "$as_me: WARNING: $ac_header:     section \"Present But Cannot Be Compiled\"" >&2;}
+    { echo "$as_me:$LINENO: WARNING: $ac_header: proceeding with the preprocessor's result" >&5
+echo "$as_me: WARNING: $ac_header: proceeding with the preprocessor's result" >&2;}
+    { echo "$as_me:$LINENO: WARNING: $ac_header: in the future, the compiler will take precedence" >&5
+echo "$as_me: WARNING: $ac_header: in the future, the compiler will take precedence" >&2;}
+    (
+      cat <<\_ASBOX
+## ---------------------------------------- ##
+## Report this to pgsql-bugs@postgresql.org ##
+## ---------------------------------------- ##
+_ASBOX
+    ) |
+      sed "s/^/$as_me: WARNING:     /" >&2
+    ;;
+esac
+echo "$as_me:$LINENO: checking for $ac_header" >&5
+echo $ECHO_N "checking for $ac_header... $ECHO_C" >&6
+if eval "test \"\${$as_ac_Header+set}\" = set"; then
+  echo $ECHO_N "(cached) $ECHO_C" >&6
+else
+  eval "$as_ac_Header=\$ac_header_preproc"
+fi
+echo "$as_me:$LINENO: result: `eval echo '${'$as_ac_Header'}'`" >&5
+echo "${ECHO_T}`eval echo '${'$as_ac_Header'}'`" >&6
+
+fi
+if test `eval echo '${'$as_ac_Header'}'` = yes; then
+  cat >>confdefs.h <<_ACEOF
+#define `echo "HAVE_$ac_header" | $as_tr_cpp` 1
+_ACEOF
+
+else
+
 for ac_header in readline/history.h
 do
 as_ac_Header=`echo "ac_cv_header_$ac_header" | $as_tr_sh`
@@ -9815,6 +9961,10 @@

 fi

+done
+
+fi
+
 if test "$with_zlib" = yes; then
   if test "${ac_cv_header_zlib_h+set}" = set; then
   echo "$as_me:$LINENO: checking for zlib.h" >&5
diff -wu postgresql-8.1.0.orig/configure.in postgresql-8.1.0/configure.in
--- postgresql-8.1.0.orig/configure.in    2005-11-04 23:01:41.000000000 -0500
+++ postgresql-8.1.0/configure.in    2005-11-21 12:19:44.000000000 -0500
@@ -724,11 +724,12 @@
 Use --without-readline to disable libedit support.])])])])
   AC_CHECK_HEADERS(editline/history.h, [],
         [AC_CHECK_HEADERS(history.h, [],
+            [AC_CHECK_HEADERS(histedit.h, [],
                 [AC_CHECK_HEADERS(readline/history.h, [],
                         [AC_MSG_ERROR([history header not found
 If you have libedit already installed, see config.log for details on the
 failure.  It is possible the compiler isn't looking in the proper directory.
-Use --without-readline to disable libedit support.])])])])
+Use --without-readline to disable libedit support.])])])])])
 fi

 if test "$with_zlib" = yes; then

Re: [HACKERS] Should libedit be preferred to libreadline?

From
Tom Lane
Date:
Andrew Dunstan <andrew@dunslane.net> writes:
> Also, I suspect we'd want to enable the libedit preference with a switch
> rather than just force it, if we want to go this way.

Quite.  My recollection is that there are other platforms on which
readline works and libedit is broken.  (Readline used to work just
fine even on AIX ;-))

            regards, tom lane

Re: [HACKERS] Should libedit be preferred to libreadline?

From
"Jim C. Nasby"
Date:
On Mon, Nov 21, 2005 at 07:50:48PM -0500, Andrew Dunstan wrote:
>
> Nice analysis, but we can't hack configure like that. It has to be able
> to be fully generated from its sources. I think the other source file
> you would need to look at is config/programs.m4. (Not sure about quoting
> $ac_popdir - why only that one?)
>
> Also, I suspect we'd want to enable the libedit preference with a switch
> rather than just force it, if we want to go this way.

BTW, we've run into issues with readline from a licensing standpoint. It
would be really nice if libedit was supported where practical (I suspect
most mainstream OSes support libedit) since it's BSD licensed.
--
Jim C. Nasby, Sr. Engineering Consultant      jnasby@pervasive.com
Pervasive Software      http://pervasive.com    work: 512-231-6117
vcard: http://jim.nasby.net/pervasive.vcf       cell: 512-569-9461

Re: [HACKERS] Should libedit be preferred to libreadline?

From
Bruce Momjian
Date:
Jim C. Nasby wrote:
> On Mon, Nov 21, 2005 at 07:50:48PM -0500, Andrew Dunstan wrote:
> >
> > Nice analysis, but we can't hack configure like that. It has to be able
> > to be fully generated from its sources. I think the other source file
> > you would need to look at is config/programs.m4. (Not sure about quoting
> > $ac_popdir - why only that one?)
> >
> > Also, I suspect we'd want to enable the libedit preference with a switch
> > rather than just force it, if we want to go this way.
>
> BTW, we've run into issues with readline from a licensing standpoint. It
> would be really nice if libedit was supported where practical (I suspect
> most mainstream OSes support libedit) since it's BSD licensed.

Why don't we have a libedit configure flag?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: [PATCHES] [HACKERS] Should libedit be preferred to libreadline?

From
Peter Eisentraut
Date:
Bruce Momjian wrote:
> Why don't we have a libedit configure flag?

Well, I can code up a configure flag, but that doesn't mean that the
thing will compile at the end. :)

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Re: [HACKERS] Should libedit be preferred to libreadline?

From
Andrew Dunstan
Date:
Nice analysis, but we can't hack configure like that. It has to be able
to be fully generated from its sources. I think the other source file
you would need to look at is config/programs.m4. (Not sure about quoting
$ac_popdir - why only that one?)

Also, I suspect we'd want to enable the libedit preference with a switch
rather than just force it, if we want to go this way.

cheers

andrew

Seneca Cunningham wrote:

>It would certainly seem so on AIX.
>
>In tracking down why postgres 8.x would segfault on AIX 5.3, it became
>apparent to me that libreadline.a is a problematic library to link
>against and that libedit.a is preferable (and for reasons other than
>that readline is GPL while postgres is BSD-licensed).
>
>With AIX 5, the easiest way to get a shared object is to pass "-bexpall"
>to the linker.  This results in all symbols being exported.  The problem
>with this is that the linker will export all of libreadline's and
>libhistory's symbols.  In the case of libreadline.so.4 (and .5) on AIX 5
>this includes symbols like strncpy and memmove, but on .4, not memcpy.
>This is likely because libc.a does not export them.
>
>What results from this is that when postgres is linked against readline
>on AIX, it gets these memory functions through readline instead of its
>own code.  When readline 4.3 is used (what IBM provides in their "AIX
>Toolbox for Linux"), postgres is known to crash.  These segfaults (if
>postgres was compiled with gcc) have occurred on AIX 5.3ML3, AIX 5.3ML1,
>and AIX 5.2ML7.  With readline 5.0, postgres merely gets these functions
>through the shared library memory segments instead of the user memory
>segments[6].
>
>While it is possible to build libreadline in a manner that doesn't
>export strncpy, neither of the prebuilt readlines for AIX 5 that I
>checked were both shared and did not export strncpy.  IBM's readline[5]
>exports strncpy, UCLA's readline[4] is static.  Building a shared
>readline that doesn't export strncpy requires creating export files for
>libreadline and libhistory that only list the symbols that they are
>supposed to export and editing the shared library Makefile to add the
>exports flags to the appropriate linker calls.
>
>Whatever strategy we might take, using readline on AIX requires
>considerable trickery and hacking around with the build environments.
>Simply put, it's ghastly.
>
>On the other hand, the port of NetBSD's editline that I tried[1] works
>without build-hackery to the library and has reasonable exports.  The
>only changes to postgres that I needed to make were confined to telling
>the configure script to check for libedit before libreadline and adding
>a test for histedit.h.  The attached patch contains my modifications.
>
>It is also possible to use a wrapper like rlwrap[2] instead of linking
>postgres against libreadline or libedit.
>
>[1] port of NetBSD's editline
>    http://www.thrysoee.dk/editline/
>[2] rlwrap
>    http://utopia.knoware.nl/~hlub/uck/software/
>[3] IBM Redbook "AIX 5L Porting Guide", section 9.2
>    http://www.redbooks.ibm.com/abstracts/sg246034.html?Open
>    http://www.redbooks.ibm.com/redbooks/pdfs/sg246034.pdf
>[4] UCLA's readline package
>    http://aixpdslib.seas.ucla.edu/packages/readline.html
>[5] IBM's readline package
>    http://www-03.ibm.com/servers/aix/products/aixos/linux/download.html
>[6] IBM Redbook "Developing and Porting C and C++ Applications on AIX",
>      page 110
>    http://www.redbooks.ibm.com/abstracts/sg245674.html?Open
>    http://www.redbooks.ibm.com/redbooks/pdfs/sg245674.pdf
>
>

[patch snipped]