readline 6.3 violates POSIX by doing #undef setjmp

2015-01-23 Thread Eric Blake 
http://pubs.opengroup.org/onlinepubs/9699919799/functions/setjmp.html is
clear:

"It is unspecified whether setjmp() is a macro or a function. If a macro
definition is suppressed in order to access an actual function, or a
program defines an external identifier with the name setjmp, the
behavior is undefined."

But in readline's posixjmp.h wrapper file, you do:

#if defined (HAVE_POSIX_SIGSETJMP)
#  define procenv_t sigjmp_buf
#  if !defined (__OPENNT)
#undef setjmp
#define setjmp(x)   sigsetjmp((x), 1)

which breaks on Cygwin.  There,  declares setjmp() as a
function, and sigsetjmp() as a macro in terms of setjmp().  Observe:

$ printf '#include 
sigsetjmp(a,0)\n' | gcc -E - | tail -n1
__extension__ ({ sigjump_buf *_sjbuf = &(a); ((*_sjbuf)[(13 * 4)] = 0,
pthread_sigmask (0, 0, (sigset_t *)((*_sjbuf) + ((13 * 4)+1))), setjmp
(*_sjbuf)); })

which, when combined with your wrapper, produces garbage:

$ printf '#include "config.h"
#include 
#include "posixjmp.h"
sigsetjmp(a,0)\n' | gcc -E - | tail -n1
__extension__ ({ sigjump_buf *_sjbuf = &(a); ((*_sjbuf)[(13 * 4)] = 0,
pthread_sigmask (0, 0, (sigset_t *)((*_sjbuf) + ((13 * 4)+1))),
sigsetjmp (*_sjbuf)); })

Basically, because your wrapper means there are now two macros defined
in terms of each other, the expansion of sigsetjmp is trying to invoke a
function named sigsetjmp, but no such function exists on Cygwin, leading
to this compilation failure:

readline.c: In function 'readline_internal_char':
readline.c:541:7: error: implicit declaration of function 'sigsetjmp'
[-Werror=implicit-function-declaration]

It looks like "posixjmp.h" exists to rewrite 'setjmp' into 'sigsetjmp' -
but that is not possible to portably do via macro expansion.  You'll
need to break down and actually do the replacement by hand.  You could
get away with something like:

#if defined (HAVE_POSIX_SIGSETJMP)
#  define Setjmp(x) sigsetjmp((x), 1)
#else
#  define Setjmp(x) setjmp(x)
#endif

and using Setjmp instead of setjmp throughout any file that includes
"posixjmp.h".  I'll leave it up to you to decide on the best solution
for your coding style.  [Meanwhile, I'll file a bug with the Cygwin
folks to see if we can use some namespace tricks in  to avoid
problems for code like readline, to be tolerant of your redefine attempt
the way Linux is, even if it is above and beyond what POSIX requires]

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: readline 6.3 violates POSIX by doing #undef setjmp

2015-01-23 Thread Eric Blake 
On 01/23/2015 02:58 PM, Eric Blake wrote:
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/setjmp.html is
> clear:
> 
> "It is unspecified whether setjmp() is a macro or a function. If a macro
> definition is suppressed in order to access an actual function, or a
> program defines an external identifier with the name setjmp, the
> behavior is undefined."
> 
> But in readline's posixjmp.h wrapper file, you do:
> 
> #if defined (HAVE_POSIX_SIGSETJMP)
> #  define procenv_t sigjmp_buf
> #  if !defined (__OPENNT)
> #undef setjmp
> #define setjmp(x)   sigsetjmp((x), 1)

You also define a setjmp_nosigs, but nothing in the readline sources
uses it.  For that matter, readline only has two instances of setjmp
clients, both which will never be reached when HAVE_POSIX_SIGSETJMP is
defined.

And why are you passing 0 to sigsetjmp in those two clients?  Don't you
generally want to preserve signal masks by passing a nonzero value,
rather than leave it unspecified whether they are preserved?

As for avoiding the undefined behavior, I think the patch is as simple as:

diff --git i/lib/readline/callback.c w/lib/readline/callback.c
index 6bb2c3e..56f43e7 100644
--- i/lib/readline/callback.c
+++ w/lib/readline/callback.c
@@ -1,6 +1,6 @@
 /* callback.c -- functions to use readline as an X `callback' mechanism. */

-/* Copyright (C) 1987-2009 Free Software Foundation, Inc.
+/* Copyright (C) 1987-2009, 2015 Free Software Foundation, Inc.

This file is part of the GNU Readline Library (Readline), a library
for reading lines of text with interactive input and history editing.
@@ -125,11 +125,7 @@ rl_callback_read_char ()
 }

   memcpy ((void *)olevel, (void *)_rl_top_level, sizeof (procenv_t));
-#if defined (HAVE_POSIX_SIGSETJMP)
   jcode = sigsetjmp (_rl_top_level, 0);
-#else
-  jcode = setjmp (_rl_top_level);
-#endif
   if (jcode)
 {
   (*rl_redisplay_function) ();
diff --git i/lib/readline/posixjmp.h w/lib/readline/posixjmp.h
index 98cf718..356828e 100644
--- i/lib/readline/posixjmp.h
+++ w/lib/readline/posixjmp.h
@@ -1,6 +1,6 @@
 /* posixjmp.h -- wrapper for setjmp.h with changes for POSIX systems. */

-/* Copyright (C) 1987,1991 Free Software Foundation, Inc.
+/* Copyright (C) 1987,1991,2015 Free Software Foundation, Inc.

This file is part of GNU Bash, the Bourne Again SHell.

@@ -27,16 +27,11 @@

 #if defined (HAVE_POSIX_SIGSETJMP)
 #  define procenv_tsigjmp_buf
-#  if !defined (__OPENNT)
-#undef setjmp
-#define setjmp(x)  sigsetjmp((x), 1)
-#define setjmp_nosigs(x)   sigsetjmp((x), 0)
-#undef longjmp
-#define longjmp(x, n)  siglongjmp((x), (n))
-#  endif /* !__OPENNT */
 #else
 #  define procenv_tjmp_buf
-#  define setjmp_nosigssetjmp
+#  ifndef sigsetjmp
+#define sigsetjmp(x, ignored) setjmp (x)
+#  endif
 #endif

 #endif /* _POSIXJMP_H_ */
diff --git i/lib/readline/readline.c w/lib/readline/readline.c
index abb29a0..f9ac97c 100644
--- i/lib/readline/readline.c
+++ w/lib/readline/readline.c
@@ -1,7 +1,7 @@
 /* readline.c -- a general facility for reading lines of input
with emacs style editing and completion. */

-/* Copyright (C) 1987-2013 Free Software Foundation, Inc.
+/* Copyright (C) 1987-2013, 2015 Free Software Foundation, Inc.

This file is part of the GNU Readline Library (Readline), a library
for reading lines of text with interactive input and history
editing.
@@ -534,11 +534,7 @@ readline_internal_charloop ()
 #endif
   lk = _rl_last_command_was_kill;

-#if defined (HAVE_POSIX_SIGSETJMP)
   code = sigsetjmp (_rl_top_level, 0);
-#else
-  code = setjmp (_rl_top_level);
-#endif

   if (code)
{


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: readline 6.3 violates POSIX by doing #undef setjmp

2015-01-23 Thread Eric Blake 
On 01/23/2015 03:53 PM, Eric Blake wrote:
> On 01/23/2015 02:58 PM, Eric Blake wrote:
>> http://pubs.opengroup.org/onlinepubs/9699919799/functions/setjmp.html is
>> clear:
>>
>> "It is unspecified whether setjmp() is a macro or a function. If a macro
>> definition is suppressed in order to access an actual function, or a
>> program defines an external identifier with the name setjmp, the
>> behavior is undefined."
>>
>> But in readline's posixjmp.h wrapper file, you do:
>>
>> #if defined (HAVE_POSIX_SIGSETJMP)
>> #  define procenv_t sigjmp_buf
>> #  if !defined (__OPENNT)
>> #undef setjmp
>> #define setjmp(x)   sigsetjmp((x), 1)
> 
> You also define a setjmp_nosigs, but nothing in the readline sources
> uses it.  For that matter, readline only has two instances of setjmp
> clients, both which will never be reached when HAVE_POSIX_SIGSETJMP is
> defined.
> 
> And why are you passing 0 to sigsetjmp in those two clients?  Don't you
> generally want to preserve signal masks by passing a nonzero value,
> rather than leave it unspecified whether they are preserved?

Okay, I see that while readline doesn't use setjmp_nosigs, the rest of
bash source does.

Remember, POSIX states that sigsetjmp(x, 0) is the same as setjmp(x),
which may or may not save the signal mask; on Cygwin, the signal mask is
ALWAYS saved (that is, the second argument of sigsetjmp() makes no
difference in behavior; there is never a way to explicitly opt out of
saving signals).

Thus, the following is the minimal patch for Cygwin to have correct
compilation (both setjmp and setjmp_nosigs usage points in bash will
have identical behavior).  Although I still think that you ought to fix
more than just Cygwin by auditing and eradicating all uses of setjmp in
favor of sigsetjmp to begin with, that's a bigger patch to bash, so I'm
not going to bother with doing it myself now that I have a correct
environment on cygwin.

diff --git i/lib/readline/posixjmp.h w/lib/readline/posixjmp.h
index 98cf718..1af5de7 100644
--- i/lib/readline/posixjmp.h
+++ w/lib/readline/posixjmp.h
@@ -1,6 +1,6 @@
 /* posixjmp.h -- wrapper for setjmp.h with changes for POSIX systems. */

-/* Copyright (C) 1987,1991 Free Software Foundation, Inc.
+/* Copyright (C) 1987,1991,2015 Free Software Foundation, Inc.

This file is part of GNU Bash, the Bourne Again SHell.

@@ -27,7 +27,7 @@

 #if defined (HAVE_POSIX_SIGSETJMP)
 #  define procenv_tsigjmp_buf
-#  if !defined (__OPENNT)
+#  if !defined (__OPENNT) && !defined (__CYGWIN__)
 #undef setjmp
 #define setjmp(x)  sigsetjmp((x), 1)
 #define setjmp_nosigs(x)   sigsetjmp((x), 0)


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

silence some compiler warnings about readline dead code

2015-01-23 Thread Eric Blake 
Noticed this while fixing compiler warnings for readline on cygwin:

diff --git i/lib/readline/readline.c w/lib/readline/readline.c
index abb29a0..9bb552f 100644
--- i/lib/readline/readline.c
+++ w/lib/readline/readline.c
@@ -1,7 +1,7 @@
 /* readline.c -- a general facility for reading lines of input
with emacs style editing and completion. */

-/* Copyright (C) 1987-2013 Free Software Foundation, Inc.
+/* Copyright (C) 1987-2013, 2015 Free Software Foundation, Inc.

This file is part of the GNU Readline Library (Readline), a library
for reading lines of text with interactive input and history
editing.
@@ -95,7 +95,6 @@ static void bind_arrow_keys_internal PARAMS((Keymap));
 static void bind_arrow_keys PARAMS((void));

 static void readline_default_bindings PARAMS((void));
-static void reset_default_bindings PARAMS((void));

 static int _rl_subseq_result PARAMS((int, Keymap, int, int));
 static int _rl_subseq_getchar PARAMS((int));
@@ -522,13 +521,16 @@ readline_internal_char ()
 readline_internal_charloop ()
 #endif
 {
-  static int lastc, eof_found;
+#ifndef READLINE_CALLBACKS
+  static int eof_found;
+#endif
+  static int lastc;
   int c, code, lk;

   lastc = -1;
-  eof_found = 0;

 #if !defined (READLINE_CALLBACKS)
+  eof_found = 0;
   while (rl_done == 0)
 {
 #endif
@@ -1204,18 +1206,6 @@ readline_default_bindings ()
 rl_tty_set_default_bindings (_rl_keymap);
 }

-/* Reset the default bindings for the terminal special characters we're
-   interested in back to rl_insert and read the new ones. */
-static void
-reset_default_bindings ()
-{
-  if (_rl_bind_stty_chars)
-{
-  rl_tty_unset_default_bindings (_rl_keymap);
-  rl_tty_set_default_bindings (_rl_keymap);
-}
-}
-
 /* Bind some common arrow key sequences in MAP. */
 static void
 bind_arrow_keys_internal (map)

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

fix ctype usage bugs in readline

2015-01-23 Thread Eric Blake 
On platforms where 'char' is signed, using ctype functions on
'char' arguments produces undefined behavior for arguments that
are larger than 127.  In particular, on Cygwin, isspace((char)255)
MUST return the same 0 result as isspace(EOF), while there are some
single-byte locales in which isspace((unsigned char)255) returns 1.

POSIX is explicit that the ctype functions are only well-defined on
the range of EOF plus the values of unsigned char.

Compilation on Cygwin with -Wall intentionally flags suspicious use of
suspect conditions (I've tried in the past to also get glibc to flag
such suspicious usage, but so far no one has listened to me):

vi_mode.c: In function 'rl_vi"fword':
vi_mode.c:557:7: error: array subscript has type 'char'
[-Werror=char-subscripts]
if (_rl_isindent (rl_line_buffer[rl_point]))
^

and many other similar warnings.

The following patch silences the particular warnings in vi_mode.c,
although I have not yet audited ALL places in the bash/readline source
code to see if there is any further abuse of ctype functions called with
out-of-range inputs.  Furthermore, my patch takes liberty with
IN_CTYPE_DOMAIN - note that the only places where I used to_uchar were
in macros where EOF as input should validly return 0, but since
isspace((unsigned char)EOF) might return non-zero, I took the shortcut
that because all of those macros were gated by IN_CTYPE_DOMAIN, I could
let IN_CTYPE_DOMAIN filter out EOF.  Of course, my hack is a gross
misnomer (EOF _is_ in the ctype domain - it is the only integer value
outside the range of unsigned char that is afforded that standing).  So
you may want to do a more thorough audit of any place that you are
explicitly passing an 'int' which could be either EOF or an unsigned
char value, rather than assuming that these macros will only ever be
used on 'char', and/or change the naming or other aspects of how the
gating is done.

diff --git i/lib/readline/chardefs.h w/lib/readline/chardefs.h
index 1fa1b08..7cb8575 100644
--- i/lib/readline/chardefs.h
+++ w/lib/readline/chardefs.h
@@ -1,6 +1,6 @@
 /* chardefs.h -- Character definitions for readline. */

-/* Copyright (C) 1994-2009 Free Software Foundation, Inc.
+/* Copyright (C) 1994-2009, 2015 Free Software Foundation, Inc.

This file is part of the GNU Readline Library (Readline), a library
for reading lines of text with interactive input and history editing.
@@ -67,13 +67,14 @@
 #define UNCTRL(c) _rl_to_upper(((c)|control_character_bit))

 #if defined STDC_HEADERS || (!defined (isascii) && !defined (HAVE_ISASCII))
-#  define IN_CTYPE_DOMAIN(c) 1
+#  define IN_CTYPE_DOMAIN(c) (c != EOF)
 #else
 #  define IN_CTYPE_DOMAIN(c) isascii(c)
 #endif
+#define to_uchar(c) ((unsigned char) (c))

 #if !defined (isxdigit) && !defined (HAVE_ISXDIGIT) && !defined
(__cplusplus)
-#  define isxdigit(c)   (isdigit((c)) || ((c) >= 'a' && (c) <= 'f') ||
((c) >= 'A' && (c) <= 'F'))
+#  define isxdigit(c)   (isdigit((to_uchar (c))) || ((c) >= 'a' && (c)
<= 'f') || ((c) >= 'A' && (c) <= 'F'))
 #endif

 #if defined (CTYPE_NON_ASCII)
@@ -87,13 +88,13 @@

 /* Beware:  these only work with single-byte ASCII characters. */

-#define ISALNUM(c) (IN_CTYPE_DOMAIN (c) && isalnum (c))
-#define ISALPHA(c) (IN_CTYPE_DOMAIN (c) && isalpha (c))
-#define ISDIGIT(c) (IN_CTYPE_DOMAIN (c) && isdigit (c))
-#define ISLOWER(c) (IN_CTYPE_DOMAIN (c) && islower (c))
-#define ISPRINT(c) (IN_CTYPE_DOMAIN (c) && isprint (c))
-#define ISUPPER(c) (IN_CTYPE_DOMAIN (c) && isupper (c))
-#define ISXDIGIT(c)(IN_CTYPE_DOMAIN (c) && isxdigit (c))
+#define ISALNUM(c) (IN_CTYPE_DOMAIN (c) && isalnum (to_uchar (c)))
+#define ISALPHA(c) (IN_CTYPE_DOMAIN (c) && isalpha (to_uchar (c)))
+#define ISDIGIT(c) (IN_CTYPE_DOMAIN (c) && isdigit (to_uchar (c)))
+#define ISLOWER(c) (IN_CTYPE_DOMAIN (c) && islower (to_uchar (c)))
+#define ISPRINT(c) (IN_CTYPE_DOMAIN (c) && isprint (to_uchar (c)))
+#define ISUPPER(c) (IN_CTYPE_DOMAIN (c) && isupper (to_uchar (c)))
+#define ISXDIGIT(c)(IN_CTYPE_DOMAIN (c) && isxdigit (to_uchar (c)))

 #define _rl_lowercase_p(c) (NON_NEGATIVE(c) && ISLOWER(c))
 #define _rl_uppercase_p(c) (NON_NEGATIVE(c) && ISUPPER(c))

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Tab completion freezes shell

2015-01-23 Thread Artur Rataj 
Hello. After typing the following, then space, then Tab,

tar --exclude-vcs cBvf a.tar.bz2

the shell or whatever freezes.

Best regards,
Artur