2008/11/16 Bram Moolenaar <[EMAIL PROTECTED]>: > Dominique Pelle wrote: > >> 2008/11/16 Dominique Pelle <[EMAIL PROTECTED]>: >> >> > 2008/11/16 Bram Moolenaar <[EMAIL PROTECTED]>: >> > >> >> 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 >> >> >> 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. > > 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; >
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 --~--~---------~--~----~------------~-------~--~----~ You received this message from the "vim_dev" maillist. For more information, visit http://www.vim.org/maillist.php -~----------~----~----~----~------~----~------~--~---
