GNU bug report logs - #3772
23.0.95; Segmentation fault: ffap/image/C-x d

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 3772 in the body.
You can then email your comments to 3772 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> emacsbugs.donarmstrong.com (full text, mbox):

From: jidanni <at> jidanni.org
To: emacs-pretest-bug <at> gnu.org
Cc: rfrancoise <at> debian.org
Subject: 23.0.95; Segmentation fault: ffap/image/C-x d
Date: Tue, 07 Jul 2009 07:37:05 +0800

1. Pick a large (16KB) .JPG from
http://jidanni.org/geo/taipower/images/ , e.g,.
$ cd /tmp && wget \
http://jidanni/jidanni.org/geo/taipower/images/19990716ab67wow.jpg
$ emacs-snapshot -Q /tmp/19990716ab67wow.jpg
M-x ffap-bindings
C-x d
Segfault... running again:
(gdb) run -Q /tmp/19990716ab67wow.jpg
Program received signal SIGSEGV, Segmentation fault.
0xb755b6b8 in bcopy () from /lib/i686/cmov/libc.so.6
>please include the output from the following gdb commands:
    `bt full' and `xbacktrace'.
(gdb) bt full
#0  0xb755b6b8 in bcopy () from /lib/i686/cmov/libc.so.6
#1  0x08170638 in ?? ()
No symbol table info available...
(gdb) xbacktrace
Undefined command: "xbacktrace".  Try "help".

In GNU Emacs 23.0.95.1 (i486-pc-linux-gnu, GTK+ Version 2.16.4)
 of 2009-07-04 on elegiac, modified by Debian
 (emacs-snapshot package, version 1:20090703-1)
Celeron(R) CPU 2.60GHz

Message #10 received at 3772 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Chong Yidong <cyd <at> stupidchicken.com>
To: Jason Rumney  <jasonr <at> gnu.org>
Cc: jidanni <at> jidanni.org, 3772 <at> debbugs.gnu.org
Subject: Re: 23.0.95; Segmentation fault: ffap/image/C-x d
Date: Tue, 07 Jul 2009 18:07:32 -0400

> $ cd /tmp && wget \
> http://jidanni.org/geo/taipower/images/19990716ab67wow.jpg
> $ emacs-snapshot -Q /tmp/19990716ab67wow.jpg
> M-x ffap-bindings
> C-x d
> Segfault...

I can reproduce this.  From bisecting prior revisions, I think the
problem first appeared with this change:

2009-03-24  Jason Rumney  <jasonr <at> gnu.org>

	* fileio.c (Fsubstitute_in_file_name): Always work on a copy.
	Calculate total size precisely.  Decode environment variables
	before substituting.  (Bug#38)

If I revert this change---the CVS diff is shown below---then the problem
goes away.  Though, with such memory problems, that's no guarantee that
this change is directly responsibe for the bug.

Jason, could you quickly double-check this checkin?  Thanks.


Index: fileio.c
===================================================================
RCS file: /sources/emacs/emacs/src/fileio.c,v
retrieving revision 1.650
retrieving revision 1.651
diff -c -r1.650 -r1.651
*** fileio.c	19 Mar 2009 06:26:07 -0000	1.650
--- fileio.c	24 Mar 2009 14:14:54 -0000	1.651
***************
*** 1629,1639 ****
--- 1629,1642 ----
    unsigned char *target = NULL;
    int total = 0;
    int substituted = 0;
+   int multibyte;
    unsigned char *xnm;
    Lisp_Object handler;
  
    CHECK_STRING (filename);
  
+   multibyte = STRING_MULTIBYTE (filename);
+ 
    /* If the file name has special constructs in it,
       call the corresponding file handler.  */
    handler = Ffind_file_name_handler (filename, Qsubstitute_in_file_name);
***************
*** 1641,1648 ****
      return call2 (handler, Qsubstitute_in_file_name, filename);
  
    nm = SDATA (filename);
! #ifdef DOS_NT
    nm = strcpy (alloca (strlen (nm) + 1), nm);
    CORRECT_DIR_SEPS (nm);
    substituted = (strcmp (nm, SDATA (filename)) != 0);
  #endif
--- 1644,1654 ----
      return call2 (handler, Qsubstitute_in_file_name, filename);
  
    nm = SDATA (filename);
!   /* Always work on a copy of the string, in case GC happens during
!      decode of environment variables, causing the original Lisp_String
!      data to be relocated.  */
    nm = strcpy (alloca (strlen (nm) + 1), nm);
+ #ifdef DOS_NT
    CORRECT_DIR_SEPS (nm);
    substituted = (strcmp (nm, SDATA (filename)) != 0);
  #endif
***************
*** 1655,1663 ****
         again.  Important with filenames like "/home/foo//:/hello///there"
         which whould substitute to "/:/hello///there" rather than "/there".  */
      return Fsubstitute_in_file_name
!       (make_specified_string (p, -1, endp - p,
! 			      STRING_MULTIBYTE (filename)));
! 
  
    /* See if any variables are substituted into the string
       and find the total length of their values in `total' */
--- 1661,1667 ----
         again.  Important with filenames like "/home/foo//:/hello///there"
         which whould substitute to "/:/hello///there" rather than "/there".  */
      return Fsubstitute_in_file_name
!       (make_specified_string (p, -1, endp - p, multibyte));
  
    /* See if any variables are substituted into the string
       and find the total length of their values in `total' */
***************
*** 1703,1710 ****
  	/* Get variable value */
  	o = (unsigned char *) egetenv (target);
  	if (o)
! 	  { /* Eight-bit chars occupy upto 2 bytes in multibyte.  */
! 	    total += strlen (o) * (STRING_MULTIBYTE (filename) ? 2 : 1);
  	    substituted = 1;
  	  }
  	else if (*p == '}')
--- 1707,1722 ----
  	/* Get variable value */
  	o = (unsigned char *) egetenv (target);
  	if (o)
! 	  {
! 	    /* Don't try to guess a maximum length - UTF8 can use up to
! 	       four bytes per character.  This code is unlikely to run
! 	       in a situation that requires performance, so decoding the
! 	       env variables twice should be acceptable. Note that
! 	       decoding may cause a garbage collect.  */
! 	    Lisp_Object orig, decoded;
! 	    orig = make_unibyte_string (o, strlen (o));
! 	    decoded = DECODE_FILE (orig);
! 	    total += SBYTES (decoded);
  	    substituted = 1;
  	  }
  	else if (*p == '}')
***************
*** 1762,1782 ****
  	    *x++ = '$';
  	    strcpy (x, target); x+= strlen (target);
  	  }
- 	else if (STRING_MULTIBYTE (filename))
- 	  {
- 	    /* If the original string is multibyte,
- 	       convert what we substitute into multibyte.  */
- 	    while (*o)
- 	      {
- 		int c = *o++;
- 		c = unibyte_char_to_multibyte (c);
- 		x += CHAR_STRING (c, x);
- 	      }
- 	  }
  	else
  	  {
! 	    strcpy (x, o);
! 	    x += strlen (o);
  	  }
        }
  
--- 1774,1795 ----
  	    *x++ = '$';
  	    strcpy (x, target); x+= strlen (target);
  	  }
  	else
  	  {
! 	    Lisp_Object orig, decoded;
! 	    int orig_length, decoded_length;
! 	    orig_length = strlen (o);
! 	    orig = make_unibyte_string (o, orig_length);
! 	    decoded = DECODE_FILE (orig);
! 	    decoded_length = SBYTES (decoded);
! 	    strncpy (x, SDATA (decoded), decoded_length);
! 	    x += decoded_length;
! 
! 	    /* If environment variable needed decoding, return value
! 	       needs to be multibyte.  */
! 	    if (decoded_length != orig_length
! 		|| strncmp (SDATA (decoded), o, orig_length))
! 	      multibyte = 1;
  	  }
        }
  
***************
*** 1789,1795 ****
         need to quote some $ to $$ first.  */
      xnm = p;
  
!   return make_specified_string (xnm, -1, x - xnm, STRING_MULTIBYTE (filename));
  
   badsubst:
    error ("Bad format environment-variable substitution");
--- 1802,1808 ----
         need to quote some $ to $$ first.  */
      xnm = p;
  
!   return make_specified_string (xnm, -1, x - xnm, multibyte);
  
   badsubst:
    error ("Bad format environment-variable substitution");

Message #15 received at 3772-done <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Chong Yidong <cyd <at> stupidchicken.com>
To: Jason Rumney  <jasonr <at> gnu.org>
Cc: jidanni <at> jidanni.org, 3772-done <at> debbugs.gnu.org
Subject: Re: 23.0.95; Segmentation fault: ffap/image/C-x d
Date: Tue, 07 Jul 2009 18:29:09 -0400

> I can reproduce this.  From bisecting prior revisions, I think the
> problem first appeared with this change:
>
> 2009-03-24  Jason Rumney  <jasonr <at> gnu.org>
>
> 	* fileio.c (Fsubstitute_in_file_name): Always work on a copy.
> 	Calculate total size precisely.  Decode environment variables
> 	before substituting.  (Bug#38)

OK, I found the problem.  The way the string data was copied was
incorrect:

  nm = SDATA (filename);
  nm = strcpy (alloca (strlen (nm) + 1), nm);

This should have been

  nm = (unsigned char *) alloca (SBYTES (filename) + 1);
  bcopy (SDATA (filename), nm, SBYTES (filename) + 1);

I've checked a fix into CVS.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.