32bit環境で profile_setlimit 内の (long)fsec が overflow してテストが通らない

svardew commented 1 year ago

不具合の内容

v9.0.1411 から profiler.c でナノ秒を使うようになりました。ナノ秒を long で扱っているため sizeof(long) == 4 の場合に整数オーバーフローがありえます。私の環境では多くの src/testdir/test_FEATURENAME.vim が影響を受けます。

現象・ログ

$ cd v9.0.1410
$ ./configure --disable-libsodium && make
$ cd /src/testdir && make
$ grep -A4 '^---' test_result.log
-------------------------------
Executed:  5534 Tests
 Skipped:    68 Tests
  FAILED:     2 Tests
$ grep -F 'Test timed out' test_result.log  |wc -l
0

$ cd v9.0.1411
$ ./configure --disable-libsodium && make
$ cd src/testdir && make
$ grep -A4 '^---' test_result.log
-------------------------------
Executed:  2634 Tests
Skipped:    43 Tests
 FAILED:    94 Tests
$ grep -F 'Test timed out' test_result.log |sort |uniq |wc -l
94

再現手順

testdir/runtest.vim の RunTheTest(test) は timer_start(30000, 'TestTimeout') を呼びます。 f_timer_start , create_timer , timer_start , profile_setlimit と辿って、 profile_setlimit(30000,tm) を実行時に fsec が必ず overflow してしまいます (30sec = 30_000_000_000 nanosec > 2^35) 次いで tm->tv_sec が negative に、そして TestTimeout がコールバックされテストが失敗します。

src/profiler.c

profile_setlimit(long msec, proftime_T *tm)
{
if (msec <= 0)   // no limit
profile_zero(tm);
else
{
# ifdef MSWIN
LARGE_INTEGER   fr;

QueryPerformanceCounter(tm);
QueryPerformanceFrequency(&fr);
tm->QuadPart += (LONGLONG)((double)msec / 1000.0 * (double)fr.QuadPart);
# else
long        fsec;

PROF_GET_TIME(tm);
fsec = (long)tm->tv_fsec + (long)msec * (TV_FSEC_SEC / 1000);
tm->tv_fsec = fsec % (long)TV_FSEC_SEC;
tm->tv_sec += fsec / (long)TV_FSEC_SEC;
# endif
}
}

reproduce

./configure --disable-libsodium && make
cd src/testdir
cat <<MYTESTFILE > "test_y1411.vim"
# Test snip from test_reltime.vim
source check.vim
func Test_y1411()
sleep 30m
endfunc
MYTESTFILE
make test_y1411.res
cat messages #=> Test timed out

期待動作

テストが通って欲しい。

src/structs.h で定義されている int64_t に変更したら元の挙動に戻りました。

--- 1411/src/profiler.c
+++ 1411mod/src/profiler.c
@@ -123,10 +123,10 @@
    QueryPerformanceFrequency(&fr);
    tm->QuadPart += (LONGLONG)((double)msec / 1000.0 * (double)fr.QuadPart);
 # else
-   long        fsec;
+   int64_t     fsec;

    PROF_GET_TIME(tm);
-   fsec = (long)tm->tv_fsec + (long)msec * (TV_FSEC_SEC / 1000);
+   fsec = (int64_t)tm->tv_fsec + (int64_t)msec * (int64_t)(TV_FSEC_SEC / 1000);
    tm->tv_fsec = fsec % (long)TV_FSEC_SEC;
    tm->tv_sec += fsec / (long)TV_FSEC_SEC;
 # endif

(正しいと思う動作が明確であれば記載してください)

Vimのバージョン

v9.0.1411 以降 (9.0.1411, 9.0.1516 で確認)

VIM - Vi IMproved 9.0 (2022 Jun 28, compiled May 11 2023 10:20:52)
Included patches: 1-1411
Compiled by AtomN270i386
Huge version with GTK3 GUI.  Features included (+) or not (-):
+acl               +file_in_path      +mouse_urxvt       -tag_any_white
+arabic            +find_in_path      +mouse_xterm       -tcl
+autocmd           +float             +multi_byte        +termguicolors
+autochdir         +folding           +multi_lang        +terminal
-autoservername    -footer            -mzscheme          +terminfo
+balloon_eval      +fork()            +netbeans_intg     +termresponse
+balloon_eval_term +gettext           +num64             +textobjects
+browse            -hangul_input      +packages          +textprop
++builtin_terms    +iconv             +path_extra        +timers
+byte_offset       +insert_expand     -perl              +title
+channel           +ipv6              +persistent_undo   +toolbar
+cindent           +job               +popupwin          +user_commands
+clientserver      +jumplist          +postscript        +vartabs
+clipboard         +keymap            +printer           +vertsplit
+cmdline_compl     +lambda            +profile           +vim9script
+cmdline_hist      +langmap           -python            +viminfo
+cmdline_info      +libcall           -python3           +virtualedit
+comments          +linebreak         +quickfix          +visual
+conceal           +lispindent        +reltime           +visualextra
+cryptv            +listcmds          +rightleft         +vreplace
+cscope            +localmap          -ruby              +wildignore
+cursorbind        -lua               +scrollbind        +wildmenu
+cursorshape       +menu              +signs             +windows
+dialog_con_gui    +mksession         +smartindent       +writebackup
+diff              +modify_fname      -sodium            +X11
+digraphs          +mouse             +sound             -xfontset
+dnd               +mouseshape        +spell             +xim
-ebcdic            +mouse_dec         +startuptime       +xpm
+emacs_tags        -mouse_gpm         +statusline        +xsmp_interact
+eval              -mouse_jsbterm     -sun_workshop      +xterm_clipboard
+ex_extra          +mouse_netterm     +syntax            -xterm_save
+extra_search      +mouse_sgr         +tag_binary        
-farsi             -mouse_sysmouse    -tag_old_static    
   system vimrc file: "$VIM/vimrc"
     user vimrc file: "$HOME/.vimrc"
 2nd user vimrc file: "~/.vim/vimrc"
      user exrc file: "$HOME/.exrc"
  system gvimrc file: "$VIM/gvimrc"
    user gvimrc file: "$HOME/.gvimrc"
2nd user gvimrc file: "~/.vim/gvimrc"
       defaults file: "$VIMRUNTIME/defaults.vim"
    system menu file: "$VIMRUNTIME/menu.vim"
  fall-back for $VIM: "/usr/local/share/vim"
Compilation: gcc -c -I. -Iproto -DHAVE_CONFIG_H -DFEAT_GUI_GTK -pthread -I/usr/include/gtk-3.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/at-spi-2.0 -I/usr/include/dbus-1.0 -I/usr/lib/i386-linux-gnu/dbus-1.0/include -I/usr/include/gtk-3.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/mirclient -I/usr/include/mircore -I/usr/include/mircookie -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/harfbuzz -I/usr/include/pango-1.0 -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/libpng12 -I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include -O0 -g 
Linking: gcc -L/usr/local/lib -Wl,--as-needed -o vim -lgtk-3 -lgdk-3 -lpangocairo-1.0 -lpango-1.0 -latk-1.0 -lcairo-gobject -lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lgobject-2.0 -lglib-2.0 -lSM -lICE -lXpm -lXt -lX11 -lXdmcp -lSM -lICE -lm -ltinfo -lcanberra -lrt -lacl -lattr -ldl

OSの種類/ディストリ/バージョン

Ubuntu 16.04 (32bit)
Linux 4.15.0-142-generic #146~16.04.1-Ubuntu SMP Tue Apr 13 09:26:57 UTC 2021 i686

使用している or 関係していそうなプラグイン

なし

その他

当初は https://github.com/vim/vim/issues/12242 のように、clock_gettime を使わない（HAVE_TIMER_CREATEを削除)で対処してました。

(関連して何か気がついたこと、気になることがあればココに書いてください)

k-takata commented 1 year ago

パッチは問題なさそうに思います。そのまま https://github.com/vim/vim に PR を作成されてはいかがでしょうか。

k-takata commented 1 year ago

PRの出し方はこちらを参考にしてください。 https://vim-jp.org/docs/how_to_write_patches.html

svardew commented 1 year ago

ありがとうございます。 PRしました。 https://github.com/vim/vim/pull/12412

k-takata commented 1 year ago

9.0.1567 採用おめでとうございます🎉 閉じます。

vim-jp / issues