[patch] fixed crash in vim-7.2.40 when compiling with gcc -O3

[Dominique Pelle]

Hi

I notice that Vim-7.2.40 (huge) crashes on start up when I
compile it with gcc 4.3.2 with -O3 (that's the default gcc
version from Ubuntu-8.10), but it works perfectly fine when
compiled with -O0, -O1 or -O2.

Here is the crash:

================================================
$ ./vim
*** buffer overflow detected ***: ./vim terminated
======= Backtrace: =========
/lib/tls/i686/cmov/libc.so.6(__fortify_fail+0x48)[0xb7746558]
/lib/tls/i686/cmov/libc.so.6[0xb7744680]
/lib/tls/i686/cmov/libc.so.6(__strcpy_chk+0x44)[0xb7743944]
./vim[0x80817df]
./vim[0x8082ed1]
./vim[0x8089e2c]
./vim[0x8093ef1]
./vim[0x80b4438]
./vim[0x80b6b73]
./vim[0x80a6960]
./vim[0x80a7181]
./vim[0x80a3c14]
./vim[0x80a3d08]
./vim[0x80ffb7c]
/lib/tls/i686/cmov/libc.so.6(__libc_start_main+0xe5)[0xb7662685]
./vim[0x8052c41]
======= Memory map: ========
08048000-0823d000 r-xp 00000000 08:04 879094 /tmp/vim7/src/vim
0823d000-0823e000 r--p 001f4000 08:04 879094 /tmp/vim7/src/vim
0823e000-0824b000 rw-p 001f5000 08:04 879094 /tmp/vim7/src/vim
0824b000-08253000 rw-p 0824b000 00:00 0

... snip...

b7975000-b7976000 rw-p 0003c000 08:04 2180173
/usr/lib/libgobject-2.0.so.0.1800.2
b7976000-b79a1000 r-xp 00000000 08:04 2179260 /usr/lib/libfontconfig.so.1.3.0
b79a1000-b79a2000 r--p 0002a000 08:04 2179260 /usr/lib/libfontconfig.so.1.3.0
Vim: Caught deadly signal ABRT0 08:04 2179260 /usr/lib/lib
Vim: Finished.
Aborted (core dumped)
================================================

Valgrind also detects the following problem (only when compiled with -O3):

$ valgrind ./vim
==7840== Memcheck, a memory error detector.
==7840== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.
==7840== Using LibVEX rev 1854, a library for dynamic binary translation.
==7840== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.
==7840== Using valgrind-3.3.1-Debian, a dynamic binary instrumentation
framework.
==7840== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.
==7840== For more details, rerun with: -v
==7840==
**7840** *** strcpy_chk: buffer overflow detected ***: program terminated
==7840== at 0x4027871: VALGRIND_PRINTF_BACKTRACE (valgrind.h:3695)
==7840== by 0x4027A37: __strcpy_chk (mc_replace_strmem.c:614)
==7840== by 0x80817DE: call_user_func (string3.h:106)
==7840== by 0x8082ED0: call_func (eval.c:8017)
==7840== by 0x8089E2B: get_func_tv (eval.c:7864)
==7840== by 0x8093EF0: ex_call (eval.c:3315)
==7840== by 0x80B4437: do_one_cmd (ex_docmd.c:2621)
==7840== by 0x80B6B72: do_cmdline (ex_docmd.c:1095)
==7840== by 0x80A695F: do_source (ex_cmds2.c:3114)
==7840== by 0x80A7180: source_callback (ex_cmds2.c:2558)
==7840== by 0x80A3C13: do_in_runtimepath (ex_cmds2.c:2652)
==7840== by 0x80A3D07: source_runtime (ex_cmds2.c:2572)
==7840==
==7840== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 103 from 1)
==7840== malloc/free: in use at exit: 1,699,551 bytes in 11,588 blocks.
==7840== malloc/free: 23,010 allocs, 11,422 frees, 3,331,612 bytes allocated.
==7840== For counts of detected errors, rerun with: -v
==7840== searching for pointers to 11,588 not-freed blocks.
==7840== checked 2,085,980 bytes.
==7840==
==7840== LEAK SUMMARY:
==7840== definitely lost: 0 bytes in 0 blocks.
==7840== possibly lost: 9,198 bytes in 323 blocks.
==7840== still reachable: 1,690,353 bytes in 11,265 blocks.
==7840== suppressed: 0 bytes in 0 blocks.
==7840== Rerun with --leak-check=full to see details of leaked memory.

(keep in mind that it's with -O3 so line numbers in stack
trace are not accurate)

Trying to narrow down further, I found that it's the -finline-functions
optimization option of gcc which triggers the crash. The option is
turned on with -O3 (among other optimizations). Compiling with
following options is enough to trigger the crash:

CFLAGS = -O2 -g -finline-functions

... whereas the following options works just fine

CFLAGS = -O2 -g

Adding some printf(), I see that Vim crashes at line 22154 in eval.c
in the STRCPY(...):

21153 v = &fc.fixvar[fixvar_idx++].var;
21154 STRCPY(v->di_key, "000");
21155 v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX;

v is declared as: 'dictitem_T *v;' with:

struct dictitem_S
{
typval_T di_tv; /* type and value of the variable */
char_u di_flags; /* flags (only used for variable) */
char_u di_key[1]; /* key (actually longer!) */
};

typedef struct dictitem_S dictitem_T;

So copying "000" in 'char_u di_key[1]' is of course a bit special
but it's not a bug I think, it's a classic pattern described in:

http://gcc.gnu.org/onlinedocs/gcc-4.3.2/gcc/Zero-Length.html#Zero-Length

There should in practice be no corruption since:

v = &fc.fixvar[fixvar_idx++].var;

... with fixvar defined as:

227 struct /* fixed variables for arguments */
228 {
229 dictitem_T var; /* variable (without
room for name) */
230 char_u room[VAR_SHORT_LEN]; /* room for the name */
231 } fixvar[FIXVAR_CNT];

.... with FIXVAR_CNT being defined as #define FIXVAR_CNT 12

This code looks OK to me. Maybe it's a bug in gcc or maybe this
construction is just not portable. I suspect it's a bug in gcc but
I'm not 100% sure.

In any case, I found that gcc has an extension to the C language
which allows to declare the di_key array without a size:

struct dictitem_S
{
typval_T di_tv; /* type and value of the variable */
char_u di_flags; /* flags (only used for variable) */
#ifdef __GNUC__
/* Declaring di_key[] instead of di_key[1] prevents crashes when
* compiling with gcc -O3 */
char_u di_key[]; /* key (actually longer!) */
#else
char_u di_key[1]; /* key (actually longer!) */
#endif
};

Doing that is enough to make it work with -O3, and also pacifies
valgrind memory checker. I guess gcc then relaxes the optimization
for di_key.

There are other places (which I did not change yet) where
the same problem could possibly happen. I see at least those:

$ egrep -n "\[1\].*actually longer" *.c *.h
eval.c:184: char_u uf_name[1]; /* name of function (actually longer); can
memline.c:92: PTR_EN pb_pointer[1]; /* list of pointers to blocks
(actually longer)
message.c:2164: char_u sb_text[1]; /* text to be displayed,
actually longer */
misc2.c:3862: char_u ffv_fname[1]; /* actually longer */
spell.c:582: char_u wc_word[1]; /* word, actually longer */
spell.c:4801: char_u sb_data[1]; /* data, actually longer */
spell.c:13081: char_u sft_word[1]; /* soundfolded word, actually longer */
tag.c:1336: char_u match[1]; /* actually longer */
regexp.h:37: char_u program[1]; /* actually longer.. */
structs.h:414: char_u b_str[1]; /* contents (actually longer) */
structs.h:768: char_u keyword[1]; /* actually longer */
structs.h:1101: char_u di_key[1]; /* key (actually longer!) */

I did not fix all those other places, but if you think that the
propose way to fix is good, I can put it in all those places.

I did not see this bug when using prior version of Ubuntu-8.04.1
(can't remember which version of gcc it used)

Interestingly, compiling with -O2 -g -finline-functions also gives
the following warning:

gcc -c -I. -Iproto -DHAVE_CONFIG_H -DFEAT_GUI_GTK
-I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include
-I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0
-I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include
-I/usr/include/pixman-1 -I/usr/include/freetype2
-I/usr/include/libpng12 -O2 -g -finline-functions -o
objects/eval.o eval.c
In function 'strcpy',
inlined from 'add_nr_var' at eval.c:21397,
inlined from 'call_user_func' at eval.c:21151:
/usr/include/bits/string3.h:106: warning: call to
__builtin___strcpy_chk will always overflow destination buffer
In function 'strcpy',
inlined from 'add_nr_var' at eval.c:21397,
inlined from 'call_user_func' at eval.c:21169:
/usr/include/bits/string3.h:106: warning: call to
__builtin___strcpy_chk will always overflow destination buffer
In function 'strcpy',
inlined from 'add_nr_var' at eval.c:21397,
inlined from 'call_user_func' at eval.c:21171:
/usr/include/bits/string3.h:106: warning: call to
__builtin___strcpy_chk will always overflow destination buffer

$ gcc --version
gcc (Ubuntu 4.3.2-1ubuntu11) 4.3.2
Copyright (C) 2008 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Attached patch makes vim work with -O3.

-- Dominique

[Bram Moolenaar]

Dominique Pelle wrote:

> I notice that Vim-7.2.40 (huge) crashes on start up when I
> compile it with gcc 4.3.2 with -O3 (that's the default gcc
> version from Ubuntu-8.10), but it works perfectly fine when
> compiled with -O0, -O1 or -O2.
>
> Here is the crash:

[...]

> **7840** *** strcpy_chk: buffer overflow detected ***: program terminated
> ==7840== at 0x4027871: VALGRIND_PRINTF_BACKTRACE (valgrind.h:3695)
> ==7840== by 0x4027A37: __strcpy_chk (mc_replace_strmem.c:614)
> ==7840== by 0x80817DE: call_user_func (string3.h:106)
> ==7840== by 0x8082ED0: call_func (eval.c:8017)

[...]

> Adding some printf(), I see that Vim crashes at line 22154 in eval.c
> in the STRCPY(...):
>
> 21153 v = &fc.fixvar[fixvar_idx++].var;
> 21154 STRCPY(v->di_key, "000");
> 21155 v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX;
>
> v is declared as: 'dictitem_T *v;' with:
>
> struct dictitem_S
> {
> typval_T di_tv; /* type and value of the variable */
> char_u di_flags; /* flags (only used for variable) */
> char_u di_key[1]; /* key (actually longer!) */
> };
>
> typedef struct dictitem_S dictitem_T;
>
>
> So copying "000" in 'char_u di_key[1]' is of course a bit special
> but it's not a bug I think, it's a classic pattern described in:
>
> http://gcc.gnu.org/onlinedocs/gcc-4.3.2/gcc/Zero-Length.html#Zero-Length
>
> There should in practice be no corruption since:
>
> v = &fc.fixvar[fixvar_idx++].var;
>
> ... with fixvar defined as:
>
> 227 struct /* fixed variables for arguments */
> 228 {
> 229 dictitem_T var; /* variable (without
> room for name) */
> 230 char_u room[VAR_SHORT_LEN]; /* room for the name */
> 231 } fixvar[FIXVAR_CNT];
>
> .... with FIXVAR_CNT being defined as #define FIXVAR_CNT 12

And VAR_SHORT_LEN is 20, so there are 21 bytes available for the key
name. A bit more if the compiler rounds off struct sizes.

> This code looks OK to me. Maybe it's a bug in gcc or maybe this
> construction is just not portable. I suspect it's a bug in gcc but
> I'm not 100% sure.
>
> In any case, I found that gcc has an extension to the C language
> which allows to declare the di_key array without a size:
>
> struct dictitem_S
> {
> typval_T di_tv; /* type and value of the variable */
> char_u di_flags; /* flags (only used for variable) */
> #ifdef __GNUC__
> /* Declaring di_key[] instead of di_key[1] prevents crashes when
> * compiling with gcc -O3 */
> char_u di_key[]; /* key (actually longer!) */
> #else
> char_u di_key[1]; /* key (actually longer!) */
> #endif
> };
>
>
> Doing that is enough to make it work with -O3, and also pacifies
> valgrind memory checker. I guess gcc then relaxes the optimization
> for di_key.

Unfortunately this solution only works for the GNU compiler, requiring
#ifdefs.

> There are other places (which I did not change yet) where
> the same problem could possibly happen. I see at least those:
>
> $ egrep -n "\[1\].*actually longer" *.c *.h
> eval.c:184: char_u uf_name[1]; /* name of function (actually longer); can
> memline.c:92: PTR_EN pb_pointer[1]; /* list of pointers to blocks
> (actually longer)
> message.c:2164: char_u sb_text[1]; /* text to be displayed,
> actually longer */
> misc2.c:3862: char_u ffv_fname[1]; /* actually longer */
> spell.c:582: char_u wc_word[1]; /* word, actually longer */
> spell.c:4801: char_u sb_data[1]; /* data, actually longer */
> spell.c:13081: char_u sft_word[1]; /* soundfolded word, actually longer */
> tag.c:1336: char_u match[1]; /* actually longer */
> regexp.h:37: char_u program[1]; /* actually longer.. */
> structs.h:414: char_u b_str[1]; /* contents (actually longer) */
> structs.h:768: char_u keyword[1]; /* actually longer */
> structs.h:1101: char_u di_key[1]; /* key (actually longer!) */
>
>
> I did not fix all those other places, but if you think that the
> propose way to fix is good, I can put it in all those places.

I don't think we need the fix in all those places, as it is only
triggered by using strcpy copying into that space.

Another solution is to use the trick what's done for "self" a few lines
higher:

v = &fc.fixvar[fixvar_idx++].var;

name = v->di_key;
STRCPY(name, "self");

So we can do the same for "000":

v = &fc.fixvar[fixvar_idx++].var;

name = v->di_key;
STRCPY(name, "000");

Can you please verify this also fixes the crash?

These all look lik add_nr_var() calls. I'll change the strcpy() there
as well.

So this is my patch. Let me know if this fixes it with your compiler:

*** ../vim-7.2.040/src/eval.c Wed Nov 12 15:28:37 2008
--- src/eval.c Sat Nov 15 12:28:32 2008
***************
*** 21150,21157 ****
init_var_dict(&fc.l_avars, &fc.l_avars_var);
add_nr_var(&fc.l_avars, &fc.fixvar[fixvar_idx++].var, "0",
(varnumber_T)(argcount - fp->uf_args.ga_len));

v = &fc.fixvar[fixvar_idx++].var;

! STRCPY(v->di_key, "000");

v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX;

hash_add(&fc.l_avars.dv_hashtab, DI2HIKEY(v));
v->di_tv.v_type = VAR_LIST;
--- 21150,21160 ----
init_var_dict(&fc.l_avars, &fc.l_avars_var);
add_nr_var(&fc.l_avars, &fc.fixvar[fixvar_idx++].var, "0",
(varnumber_T)(argcount - fp->uf_args.ga_len));
+ /* Use "name" to avoid a warning from some compiler that checks the
+ * destination size. */

v = &fc.fixvar[fixvar_idx++].var;

! name = v->di_key;
! STRCPY(name, "000");

v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX;

hash_add(&fc.l_avars.dv_hashtab, DI2HIKEY(v));
v->di_tv.v_type = VAR_LIST;
***************
*** 21394,21400 ****
char *name;
varnumber_T nr;
{
! STRCPY(v->di_key, name);

v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX;

hash_add(&dp->dv_hashtab, DI2HIKEY(v));
v->di_tv.v_type = VAR_NUMBER;
--- 21397,21409 ----
char *name;
varnumber_T nr;
{
! char *nname;
!
! /* When inlining and the compiler adds a check for the destination of
! * strcpy() it may complain about di_key being too small. Happens with
! * GCC 4.3.2 with -O3. Use an intermediate variable to avoid that. */
! nname = v->di_key;
! STRCPY(nname, name);

v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX;

hash_add(&dp->dv_hashtab, DI2HIKEY(v));
v->di_tv.v_type = VAR_NUMBER;
*** ../vim-7.2.040/src/version.c Wed Nov 12 16:04:43 2008
--- src/version.c Wed Nov 12 16:54:35 2008
***************
*** 678,679 ****
--- 678,681 ----
{ /* Add new patch number below this line */
+ /**/
+ 41,
/**/

--
hundred-and-one symptoms of being an internet addict:
257. Your "hundred-and-one" lists include well over 101 items, since you
automatically interpret all numbers in hexadecimal notation.
(hex 101 = decimal 257)

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ download, build and distribute -- http://www.A-A-P.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[Nico Weber]

> This code looks OK to me. Maybe it's a bug in gcc or maybe this
> construction is just not portable. I suspect it's a bug in gcc but
> I'm not 100% sure.

Isn't it better to file this as a bug with gcc?

[Dominique Pelle]

2008/11/15 Bram Moolenaar <Br...@moolenaar.net>:

> Another solution is to use the trick what's done for "self" a few lines
> higher:
>
> v = &fc.fixvar[fixvar_idx++].var;
> name = v->di_key;
> STRCPY(name, "self");
>
> So we can do the same for "000":
>
> v = &fc.fixvar[fixvar_idx++].var;
> name = v->di_key;
> STRCPY(name, "000");
>
> Can you please verify this also fixes the crash?

No, the patch does not help. But I see now I that gave the
wrong location for the crash! Sorry about that. Crash
happens in the STRCPY(...) inside add_nr_var(...):

static void
add_nr_var(dp, v, name, nr)
dict_T *dp;
dictitem_T *v;
char *name;
varnumber_T nr;
{
STRCPY(v->di_key, name); /* Crash when compiled with -O3 */

Your patch tried to fixed this line as well anyway, but
it does not work (same crash).

The solution I proposed does work, but as you say, it requires
an ugly #ifdef __GNUC__

Maybe it's not worth trying to tweak the code, if it's a gcc
bug (not 100% sure yet). I will post it in the gcc mailing list,
and link to this thread.

-- Dominique

[Dominique Pelle]

Agreed. I just opened a gcc bug in bugzilla, and linked to
this discussion. See:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38136

I'll try to see if it's possible to simplify the gcc bug to
a simpler test case.

-- Dominique

[Tony Mechelynck]

On 15/11/08 11:12, Dominique Pelle wrote:
> Hi
>
> I notice that Vim-7.2.40 (huge) crashes on start up when I
> compile it with gcc 4.3.2 with -O3 (that's the default gcc
> version from Ubuntu-8.10), but it works perfectly fine when
> compiled with -O0, -O1 or -O2.

[...]

-O3 is a nondefault setting in Vim, isn't it? I didn't make any changes
to how the Vim configure and make optimizes its compile, and I end up
(for a Huge Gnome2 GUI, but also for a Tiny Console-only build) with -O2
-fno-strength-reduce -Wall

"gcc --version" answers:
gcc (SUSE Linux) 4.3.1 20080507 (prerelease) [gcc-4_3-branch revision
135036]

followed by a copyright notice and liability disclaimer.

Best regards,
Tony.
--
What this country needs is a good five cent nickel.

[Dominique Pelle]

2008/11/15 Tony Mechelynck <antoine.m...@gmail.com>:

When I said "this is the default", I meant gcc-4.3.2 is the
default compiler of Ubuntu-8.10. I did not mean that -O3
is the default optimization level when compiling Vim.
Maybe that was confusing.

In any case, I would expect -O3 to work. One of the most
important optimization brought by -O3 is inlining of functions.
And this is precisely the optimization which somehow breaks vim.

You can build with -O3 by editing src/makefile, or by doing...

make CFLAGS=-O3

Doing a google search, I found afterwards that it's not a new
issue. It was already reported here with the same suggested
fix:

http://www.mail-archive.com/vim...@vim.org/msg03320.html

However, the above thread mentions that -fstack-protector triggers
it, whereas I observe it with -O3 or with -O2 -g -finline-functions.

Cheers
-- Dominique

[Bram Moolenaar]

Dominique Pelle wrote:

Apparently -fstack-protector is on by default. The "inline-functions"
apparently does something to reveal the size of the destination to
strcpy(). That's a bit unexpected though.

Why not compile Vim with -fno-stack-protector ? Can you try with -O3
and that flag? It's not clear to me that this stack-protector function
is what actually adds the check for the array size.

--
Two fish in a tank. One says to the other:
"Do you know how to drive this thing?"

[Dominique Pelle]

2008/11/16 Bram Moolenaar <Br...@moolenaar.net>:

> Apparently -fstack-protector is on by default. The "inline-functions"
> apparently does something to reveal the size of the destination to
> strcpy(). That's a bit unexpected though.
>
> Why not compile Vim with -fno-stack-protector ? Can you try with -O3
> and that flag? It's not clear to me that this stack-protector function
> is what actually adds the check for the array size.

Adding -fno-stack-protector does not help either (same warning +
same crash). But reading through the man page of gcc, I stumbled
upon this in the section about -O2:

==================================================
NOTE: In Ubuntu 8.10 and later versions, -D_FORTIFY_SOURCE=2
is set by default, and is activated when -O is set to 2 or higher.
This enables additional compile-time and run-time checks for several
libc functions. To disable, specify either -U_FORTIFY_SOURCE or
-D_FORTIFY_SOURCE=0.
==================================================

So I tried adding compiling with -O3 -D_FORTIFY_SOURCE=0
and it makes it work!

So far I don't observe anything wrong so fat with
-O3 -D_FORTIFY_SOURCE=0. 'make test' succeeds
in every tests.

I'm not 100% sure whether adding -D_FORTIFY_SOURCE=0 silents
a real bug, or whether it was reporting a spurious error (more likely
from looking at vim code). But even if it silents a spurious bug in this
case, adding -D_FORTIFY_SOURCE=0 may also silent other real
bugs, which is a shame. I'll add the info to the gcc bug buzilla, but
it was already and quickly marked as INVALID, so I don't expect
much there.

-- Dominique

[Dominique Pelle]

2008/11/16 Dominique Pelle <dominiq...@gmail.com>:

I should add that building with -O3 -D_FORTIFY_SOURCE=1 also
works which is better.

Reading about _FORTIFY_SOURCE in the following link, everything
makes sense now.

Snippet from http://mail-index.netbsd.org/tech-userlevel/2007/05/23/0001.html

===============================================
The diffence between -D_FORTIFY_SOURCE=1 and -D_FORTIFY_SOURCE=2
is e.g. for
struct S { struct T { char buf[5]; int x; } t; char buf[20]; } var;
With -D_FORTIFY_SOURCE=1,
strcpy (&var.t.buf[1], "abcdefg");
is not considered an overflow (object is whole VAR), while
with -D_FORTIFY_SOURCE=2
strcpy (&var.t.buf[1], "abcdefg");
will be considered a buffer overflow.
===============================================

This example is very close to what vim does. So it makes sense
that -D_FORTIFY_SOURCE=2 detects an overflow, while
-D_FORTIFY_SOURCE=1 does not.

Adding -D_FORTIFY_SOURCE=1 to Vim makefile sounds like
a good idea.

-- Dominique

[Bram Moolenaar]

Dominique Pelle wrote:

This makes sense. It actually mentions that -D_FORTIFY_SOURCE=2 may
break confirming programs. This also means it should never be the
default. So perhaps you can file a bug that the default should be to
use 1.

The argument is only needed for GCC 4 and later, right? That's why I
didn't notice this, I'm using gcc 3.4.6 (FreeBSD is very conservative
about gcc versions, for a good reason). So we can add a configure
check.

The patch below should work, if the information is correct.

*** ../vim-7.2.042/src/auto/configure Thu Jul 24 17:20:50 2008
--- src/auto/configure Sun Nov 16 17:08:44 2008
***************
*** 16819,16839 ****
LDFLAGS="$LDFLAGS -isysroot /Developer/SDKs/MacOSX10.4u.sdk -arch i386 -arch ppc"
fi

- { $as_echo "$as_me:$LINENO: checking for GCC 3 or later" >&5
- $as_echo_n "checking for GCC 3 or later... " >&6; }
DEPEND_CFLAGS_FILTER=
if test "$GCC" = yes; then
gccmajor=`echo "$gccversion" | sed -e 's/^\([1-9]\)\..*$/\1/g'`
if test "$gccmajor" -gt "2"; then
DEPEND_CFLAGS_FILTER="| sed 's+-I */+-isystem /+g'"
! fi
! fi
! if test "$DEPEND_CFLAGS_FILTER" = ""; then
! { $as_echo "$as_me:$LINENO: result: no" >&5
$as_echo "no" >&6; }
! else
! { $as_echo "$as_me:$LINENO: result: yes" >&5
$as_echo "yes" >&6; }
fi


--- 16819,16847 ----
LDFLAGS="$LDFLAGS -isysroot /Developer/SDKs/MacOSX10.4u.sdk -arch i386 -arch ppc"
fi

DEPEND_CFLAGS_FILTER=
if test "$GCC" = yes; then
+ { $as_echo "$as_me:$LINENO: checking for GCC 3 or later" >&5
+ $as_echo_n "checking for GCC 3 or later... " >&6; }
gccmajor=`echo "$gccversion" | sed -e 's/^\([1-9]\)\..*$/\1/g'`
if test "$gccmajor" -gt "2"; then
DEPEND_CFLAGS_FILTER="| sed 's+-I */+-isystem /+g'"
! { $as_echo "$as_me:$LINENO: result: yes" >&5
! $as_echo "yes" >&6; }
! else
! { $as_echo "$as_me:$LINENO: result: no" >&5
$as_echo "no" >&6; }
! fi
! { $as_echo "$as_me:$LINENO: checking whether we need -D_FORTIFY_SOURCE=1" >&5
! $as_echo_n "checking whether we need -D_FORTIFY_SOURCE=1... " >&6; }
! if test "$gccmajor" -gt "3"; then
! CFLAGS="$CFLAGS -D_FORTIFY_SOURCE=1"
! { $as_echo "$as_me:$LINENO: result: yes" >&5
$as_echo "yes" >&6; }
+ else
+ { $as_echo "$as_me:$LINENO: result: no" >&5
+ $as_echo "no" >&6; }
+ fi
fi


*** ../vim-7.2.042/src/configure.in Thu Jul 24 17:20:31 2008
--- src/configure.in Sun Nov 16 17:08:40 2008
***************
*** 3152,3169 ****
dnl But only when making dependencies, cproto and lint don't take "-isystem".
dnl Mac gcc returns "powerpc-apple-darwin8-gcc-4.0.1 (GCC)...", need to allow
dnl the number before the version number.
- AC_MSG_CHECKING(for GCC 3 or later)
DEPEND_CFLAGS_FILTER=
if test "$GCC" = yes; then
gccmajor=`echo "$gccversion" | sed -e 's/^\([[1-9]]\)\..*$/\1/g'`
if test "$gccmajor" -gt "2"; then
DEPEND_CFLAGS_FILTER="| sed 's+-I */+-isystem /+g'"
fi
- fi
- if test "$DEPEND_CFLAGS_FILTER" = ""; then
- AC_MSG_RESULT(no)
- else
- AC_MSG_RESULT(yes)
fi
AC_SUBST(DEPEND_CFLAGS_FILTER)

--- 3152,3176 ----
dnl But only when making dependencies, cproto and lint don't take "-isystem".
dnl Mac gcc returns "powerpc-apple-darwin8-gcc-4.0.1 (GCC)...", need to allow
dnl the number before the version number.
DEPEND_CFLAGS_FILTER=
if test "$GCC" = yes; then
+ AC_MSG_CHECKING(for GCC 3 or later)
gccmajor=`echo "$gccversion" | sed -e 's/^\([[1-9]]\)\..*$/\1/g'`
if test "$gccmajor" -gt "2"; then
DEPEND_CFLAGS_FILTER="| sed 's+-I */+-isystem /+g'"
+ AC_MSG_RESULT(yes)
+ else
+ AC_MSG_RESULT(no)
+ fi
+ dnl -D_FORTIFY_SOURCE=2 crashes Vim on strcpy(buf, "000") when buf is
+ dnl declared as char x[1] but actually longer. Introduced in gcc 4.0.
+ AC_MSG_CHECKING(whether we need -D_FORTIFY_SOURCE=1)
+ if test "$gccmajor" -gt "3"; then
+ CFLAGS="$CFLAGS -D_FORTIFY_SOURCE=1"
+ AC_MSG_RESULT(yes)
+ else
+ AC_MSG_RESULT(no)
fi
fi
AC_SUBST(DEPEND_CFLAGS_FILTER)

*** ../vim-7.2.042/src/eval.c Wed Nov 12 15:28:37 2008
--- src/eval.c Sun Nov 16 17:00:17 2008

***************
*** 21150,21157 ****
init_var_dict(&fc.l_avars, &fc.l_avars_var);
add_nr_var(&fc.l_avars, &fc.fixvar[fixvar_idx++].var, "0",
(varnumber_T)(argcount - fp->uf_args.ga_len));

v = &fc.fixvar[fixvar_idx++].var;

! STRCPY(v->di_key, "000");
v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX;
hash_add(&fc.l_avars.dv_hashtab, DI2HIKEY(v));
v->di_tv.v_type = VAR_LIST;
--- 21150,21160 ----
init_var_dict(&fc.l_avars, &fc.l_avars_var);
add_nr_var(&fc.l_avars, &fc.fixvar[fixvar_idx++].var, "0",
(varnumber_T)(argcount - fp->uf_args.ga_len));
+ /* Use "name" to avoid a warning from some compiler that checks the
+ * destination size. */

v = &fc.fixvar[fixvar_idx++].var;

! name = v->di_key;
! STRCPY(name, "000");
v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX;
hash_add(&fc.l_avars.dv_hashtab, DI2HIKEY(v));
v->di_tv.v_type = VAR_LIST;
***************
*** 21394,21400 ****
char *name;
varnumber_T nr;
{
! STRCPY(v->di_key, name);
v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX;
hash_add(&dp->dv_hashtab, DI2HIKEY(v));
v->di_tv.v_type = VAR_NUMBER;

--- 21397,21408 ----
char *name;
varnumber_T nr;
{
! char_u *kname;
!
! /* Use an intermediate variable to avoid a warning when the compiler does
! * function inlining. */
! kname = v->di_key;
! STRCPY(kname, name);

v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX;
hash_add(&dp->dv_hashtab, DI2HIKEY(v));
v->di_tv.v_type = VAR_NUMBER;

--
hundred-and-one symptoms of being an internet addict:

268. You get up in the morning and go online before getting your coffee.

[Dominique Pelle]

2008/11/16 Bram Moolenaar <Br...@moolenaar.net>:

Yes, the patch adds -D_FORTIFY_SOURCE=1
to CFLAGS in src/auto/config.mk (good).

Patching eval.c is not necessary, at least for gcc
since compilation warning & runtime crash do not
happen with _FORTIFY_SOURCE=1, they
only happens when doing -D_FORTIFY_SOURCE=2.

-- Dominique

[Bram Moolenaar]

Dominique Pelle wrote:

> > This makes sense. It actually mentions that -D_FORTIFY_SOURCE=2 may
> > break confirming programs. This also means it should never be the
> > default. So perhaps you can file a bug that the default should be to
> > use 1.
> >
> > The argument is only needed for GCC 4 and later, right? That's why I
> > didn't notice this, I'm using gcc 3.4.6 (FreeBSD is very conservative
> > about gcc versions, for a good reason). So we can add a configure
> > check.
> >
> > The patch below should work, if the information is correct.

[...]

> Yes, the patch adds -D_FORTIFY_SOURCE=1
> to CFLAGS in src/auto/config.mk (good).

Glad this works. Thanks for testing.

> Patching eval.c is not necessary, at least for gcc
> since compilation warning & runtime crash do not
> happen with _FORTIFY_SOURCE=1, they
> only happens when doing -D_FORTIFY_SOURCE=2.

It's not for this specific problem, but to make it consistent with the
other place where the warning is avoided. But one of them doesn't have
a constant string, so we don't actually need it there. Only where
"self" and "000" are copied into v->di_key.

--
hundred-and-one symptoms of being an internet addict:

269. You wonder how you can make your dustbin produce Sesame Street's
Oscar's the Garbage Monster song when you empty it.