Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IDEA] gtk3の実験的サポート #1

Closed
wants to merge 83 commits into from
Closed

[IDEA] gtk3の実験的サポート #1

wants to merge 83 commits into from

Conversation

ma8ma
Copy link
Owner

@ma8ma ma8ma commented Sep 15, 2018

edit: GTK3版は上流 JDimproved/JDim のmasterブランチにマージされています。(JDimproved#30, JDimproved#39)


このPRはマージしません
#1は閉じました。 仮PR第2版 #2 を見てください。
gtkmm-2.4のかわりにgtkmm-3.0を使ってJDをビルドするオプションを追加します。

Ubuntu 16.04のGNOME/LXDE でgtkmm3+std::threadのビルドがとりあえず動くことを確認していますが、まだいろいろな環境で使えるのかわからないので興味がある方に遊んでもらえたら幸いです。

注意

  • 実装可能か実験している段階なのでビルドに失敗したり、動作中にクラッシュ、フリーズやメモリリークする可能性があります。
  • gtk2/3でUIのルック・アンド・フィールが異なる部分があります。
  • 応急処置/暫定の変更が含まれています。
  • ビルドのオプションに関係なくプロファイルは共通です。大事なデータはバックアップをしてから試してください。

要件

JDの導入方法に加えて以下が必要です

  • C++11に対応したコンパイラ
  • gtkmm-3.0 (バージョン3.18以上を推奨、Ubuntuではlibgtkmm-3.0-devをインストール)
  • AX_CXX_COMPILE_STDCXX_11を使うためのautoconfのパッケージ(Ubuntuではautoconf-archiveをインストール)

configureのフラグ

  • --with-gtkmm3 gtkmm-2.4のかわりにgtkmm-3.0を使う(デフォルトは=no)
  • --with-stdthread pthreadのかわりにstd::threadを使う(デフォルトは=no)

2つのフラグは独立しているのでgtkmm3とpthreadまたはgthreadの組み合わせでビルドできます。(逆にgtkmm2とstdthreadの組み合わせも可能)

ビルド

リモートを追加してgtk3-ideaブランチをフェッチしチェックアウトするかリポジトリをクローンします。
./configureのフラグを追加してビルドしてください

# 既存のリポジトリにリモートを追加する場合
$ cd /path/to/JD
$ git remote add ma8ma https://github.com/ma8ma/JD.git
$ git fetch ma8ma gtk3-idea
$ git checkout -b gtk3-idea --track remotes/ma8ma/gtk3-idea
$ autoreconf -i
$ ./configure (...) --with-stdthread --with-gtkmm3
$ make

既知の問題

  • gtk3のビルドで板ビューのマウスホイールによるスクロールが動作しない
    → 環境変数GDK_CORE_DEVICE_EVENTS=1を設定します
  • gtk3のビルドではタブの上でマウスホイールをしてもスレッドや板が切り替わらない
    → gtk3で機能が削除されています
  • LXDEでgtk2のビルドを動作させると特定の状況(ポップアップが被っていた場所やタブのドラッグ後)でスレビューが描画が汚くなることがある
    → LXDE + gtk2 + cairoの組み合わせは相性が悪い?

修正の説明

修正のルール

コーディングスタイル

  • clang-formatで修正した部分を整形しています。(一部は手直し)
  • JDのコードをベースに設定を作っていますが、設定できないスタイルや意図的に変更したポイントがあります。(0caab6e を参照)

コミット

  • コミットメッセージのサブジェクトの先頭に修正タイプをつけています (E=compile error, C=crash, B=behavior)
  • 変更の余地が大きいコミットとしてQUICKFIXは応急処置、PROVISIONALは暫定を表します
  • コミットは大別してgtk2から有効な変更[gtk2]とgtk3にのみ影響を与える変更[gtk3]があります

実装の方針

  • gtkの機能を使います(同じような機能は作らない)
  • 実装の詳細を使う(gtkのソースコードを引用する)ロジックは避けます
  • gtk3で削除された機能は自前で再実装しません

変更の余地があるコミット/ロジック

  • タブ幅の調整
  • タブのドラッグ・アンド・ドロップ

LLVMスタイルを元にJDスタイルを再現する
表記ゆれのある部分はJDのコードを調べて多数の方を採用する
曖昧な部分はLLVMスタイルを踏襲する

JDスタイルから変更した部分

* 一行の最大文字数を80に設定(LLVMのデフォルト)

* 単独のジャンプ文(continue, break, goto, return, throw)を除き
  if-else文のブロックに波括弧を付ける
  ```cpp
  // YES
  if( expr ) return 1;
  if( long_long_expr1 && long_long_expr2 )
      return long_long_func( long_long_value );
  if( expr ) {
      var = 20;
  }

  // NO
  if( expr ) var = 20;
  if( expr )
      var = 20;
  ```

* 空の文(;)を除きループ文のブロックに波括弧を付ける
  ```cpp
  // YES
  while( expr )
      ;
  while( expr ) {
      var += 1;
  }

  // NO
  while( expr );
  while( expr ) var += 1;
  while( expr )
      var += 1;
  ```

ツールで再現できていないスタイル

* マクロGTKMM_CHECK_VERSION()の引数
  -> スペースを取り除いてJDスタイルに準ずる
  ```cpp
  // YES
  #if GTKMM_CHECK_VERSION(2,10,0)
  #endif

  // NO (clang-format)
  #if GTKMM_CHECK_VERSION( 2, 10, 0 )
  #endif
  ```

* 制御構文の条件式の閉じ括弧とブロックの開き波括弧の間
  -> ツールで整形したままでよい
  ```cpp
  // OK (clang-format)
  if( expr ) {
  }

  // (JD style)
  if( expr ){
  }
  ```

* 否定演算子の後
  -> ツールで整形したままでよい
  ```cpp
  // OK (clang-format)
  func( !expr );

  // (JD style)
  func( ! expr );
  ```

* for文の条件式の初期化パートが空のとき
  -> ツールで整形したままでよい
  ```cpp
  // OK (clang-format)
  for( ; expr1; expr2 ) {
  }

  // (JD style)
  for(; expr1; expr2 ){
  }
  ```
gtkmm-2.4の代わりにgtkmm-3.0を使ってビルドするconfigureフラグを追加する
gtkmm-3.0のバージョンは3.18以上を推奨
仮修正
./configureでC++11を使えるかチェックします
gtk3-ideaのパッチはgtkmm-2.4/3.0に関係なくC++11の機能を使っています

Ubuntuのメモ
configureチェックで下のようなsyntax errorが出る場合はaptで
autoconf-archiveをインストールしてautoreconfで./configureを作りなおします
```sh
./configure: line 16976: syntax error near unexpected token `noext,'
./configure: line 16976: `AX_CXX_COMPILE_STDCXX_11(noext, mandatory)'
```
それでもエラーがでるときはこのコミットをrevertして試してください
Glib::Threads is deprecated since glibmm 2.47.
You can set the configure flag --with-stdthread to enable std::thread
instead (require C++11 compiler).
Glib::RefPtr::clear() is deprecated.
Type: compile error

Gtk::ALIGN_LEFT is deprecated.
Type: compile error

Gtk::ComboBoxText::append_text() is deprecated.
Type: compile error

Gtk::TreePath::get_depth() is deprecated.
…st_cell_renderer()

Type: compile error

Gtk::TreeViewColumn::get_first_cell_renderer() is deprecated.
Type: compile error

Gtk::Widget::is_mapped() is deprecated.
Type: compile error

Gtk::Widget::is_realized() is deprecated.
Type: compile error

Gtk::Widget::is_drawable() is deprecated.
Type: compile error

Gtk::Widget::is_toplevel() is deprecated.
Type: compile error

Gtk::Widget::set_flags() is deprecated.
Type: compile error

GTK_WIDGET_XXX() macro is deprecated.
…ad of get_colorsel()

Type: compile error

Gtk::ColorSelectionDialog::get_colorsel() is deprecated.
Type: compile error

Gtk::Tooltips is removed in gtk3.
Type: compile error

The return type of Gtk::Container::get_children() becomes std::vector
instead of intermediate container since gtk3.
Type: compile error

The parameter type of Gtk::Widget::drag_dest_set() and
Gtk::Widget::drag_source_set() become std::vector instead of
intermediate container since gtk3.
Type: compile error

The parameter type of Gtk::Window::set_default_icon_list() becomes
std::vector instead of intermediate container since gtk3.
Type: compile error

The return type of Gtk::TreeSelection::get_selected_rows() becomes
std::vector instead of intermediate container since gtk3.
…umn_id field

Type: compile error

gtk3 removes many implementation details and struct members from
public headers.
Type: compile error

gtk3 removes many implementation details and struct members from
public headers.

挙動の変更
リセットフラグを設定できないため即時リセットするように変更する
Type: compile error

Gtk::BUTTONBOX_DEFAULT_STYLE is removed since gtk3.
https://developer.gnome.org/gtk3/stable/GtkButtonBox.html#GtkButtonBox--layout-style

NOTE: gtk2(2.24)の実装ではGtk::BUTTONBOX_DEFAULT_STYLEを指定すると
Gtk::BUTTONBOX_EDGEが設定される
…field

Type: compile error

gtk3 removes many implementation details and struct members from
public headers.
Type: crash

Gtk::TreeNodeChildren::reverse_iterator is deprecated and not worked.

reverse_iteratorは未修正のバグがあり廃止されている[1]
このバグの影響によりgtk3の環境でreverse_iteratorを使うとsegmentation
faultを起こしクラッシュする

[1]: https://bugzilla.gnome.org/show_bug.cgi?id=554889
Gdk::Window::draw_rectangle() is removed in gdk3.
Type: compile error

Gdk::Drawable API is deprecated.

cairomm 1.12.0 がメモリリークを起こしたので C API を使うことで
問題を回避する
…w_layout()

Type: compile error

Gdk::Drawable::draw_layout() is deprecated.
Gtk::Widget::unmap()で閉じるボタンに見えなくなることを通知する
…button

E QUICKFIX [gtk3] Omit to draw button relief regardless of Gtk::RELIEF_NONE

XXX: GTKテーマAmbianceではハックの有無でスタイルに違いはなかった
E [gtk2] Use alias to the pointer type to Gtk::Adjustment

Require C++11
…se_iterator

右辺値のデクリメントが動作することは保証されてないという情報[1]
を見つけたのでstd::prev()を使う
[1]: https://ja.cppreference.com/w/cpp/iterator/prev
@naniwaKun
Copy link

作業おつかれさまです。archlinuxで遊んでみました。動いています。
https://gist.github.com/naniwaKun/7c552ed493bac514874c0daa8c692d02

@ma8ma
Copy link
Owner Author

ma8ma commented Sep 20, 2018

@naniwaKun 試していただきありがとうございます。他の環境で動くことが分かったので胸をなでおろしました。

テーマによっては配色に問題があるようですね。gtk2との違いをチェックしてみます。

@naniwaKun
Copy link

@ma8ma
そうですね、gtk2版以上の実装をするつもりはないということはもう少し大きく言っておいたほうが楽かもしれません。保守のほうが大変だと思いますので、応援しています。

Copy link
Owner Author

@ma8ma ma8ma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

追加の微修正とコーディングスタイルの決定を反映するために新しい仮PRを開く作業を始めます。
新しい仮PRは別のブランチ(gtk3-idea-r2)で開き#1のブランチ(gtk3-idea)はしばらく残しておく予定です

edit: #1は次の仮PRを開いた後closeし、閉じてから1ヶ月程度経過したらブランチgtk3-ideaは削除されます。
edit2: gtk3-ideaは2018-10-27以降に削除されます。

BreakConstructorInitializers: BeforeComma
BreakAfterJavaFieldAnnotations: false
BreakStringLiterals: false
ColumnLimit: 80
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ColumnLimitを0に設定して改行をツールで行わないようようにする。 (JDimproved#7 (comment))
修正したコードは読みやすいところで改行し直す。(ブラウザで見ることを考えるとgithubの横幅くらいか)

AC_ARG_WITH(gtkmm3,
[AS_HELP_STRING([--with-gtkmm3], [use gtkmm-3.0 instead of gtkmm-2.4 [default=no]])],
[with_gtkmm3="$withval"],
[with_gtkmm3=no])
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--with-gtkmm3--with-stdthreadの説明をREADMEやINSTALLに追加する。
合わせてビルドの要件やgtk2版との違いも説明を追加する。

@@ -304,28 +343,49 @@ void DrawAreaBase::init_color()
const int usrcolor = colors.size();
m_color.resize( END_COLOR_FOR_THREAD + usrcolor );

#if GTKMM_CHECK_VERSION(3,0,0)
Glib::RefPtr< Gtk::StyleContext > context = get_style_context();
#else
Glib::RefPtr< Gdk::Colormap > colormap = get_default_colormap();
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

今のところスレビューに限らず色の設定の修正はQUICKFIXとする。

#ifdef WITH_STD_THREAD
#include <mutex>
typedef std::lock_guard< std::mutex > LockGuard;
#else
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mutexの条件コンパイルは何度も現れるのでヘッダーに移動する。(src/jdlib/jdmutex.hを作る予定)

@@ -145,13 +168,34 @@ bool AAMenu::move_down()
std::cout << "AAMenu::move_down\n";
#endif

#if GTKMM_CHECK_VERSION(3,0,0)
std::vector< Gtk::Widget* > items = get_children();
auto it = items.begin();
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gtk2のFoo_Helpers::FooListGListのラッパーなのでstd::vectorより構築のコストが低い。gtk2環境でstd::vectorへ置き換えるのは影響がでる変更になるのでやらない。

@naniwaKun
Copy link

@ma8ma ここで聞くのが適当かわかりませんが、私がメンテナになりますので、gtk3版のjdをArchlinuxの非公式リポジトリに登録してもいいでしょうか?

@ma8ma
Copy link
Owner Author

ma8ma commented Sep 24, 2018

@naniwaKun 開発版(alpha)であると分かるなら問題ないと思います。
私はArchlinuxをよく知らないのですがこちらでやったほうがいいことがあるなら教えてください。それから#1のブランチは次の仮PRを公開して1ヶ月くらい後に削除する予定なのでコメントでお知らせします。

@naniwaKun
Copy link

@ma8ma ありがとうございます。とりあえず、gtk3-ideaに依存する形で公開して、後にgtk3-idea-r2に対応させていただきます。ArchLinuxは最新パッケージを謳うローリングリリースですのでライブラリ等の更新の際のエラーなどは協力できると思います。AUR(Archlinuxの非公式リポジトリ)に登録する内容はArchに合わせたビルドスクリプトようなものですので、#1の内容があれば十分です。今後何かあればどうぞよろしくお願い致します。

@ma8ma
Copy link
Owner Author

ma8ma commented Sep 29, 2018

仮PR第2版 (#2) を開いたので#1を閉めます。リモートブランチgtk3-ideaは2018-10-27以降に削除されます。

@naniwaKun AURにjd-gtk3のページ (https://aur.archlinux.org/packages/jd-gtk3/) を見つけました。開発版の情報としてREADME.mdの事前準備にページのリンクを載せても大丈夫でしょうか。

@ma8ma ma8ma closed this Sep 29, 2018
@naniwaKun
Copy link

@ma8ma 載せていただいて大丈夫です。descriptionの情報は「A 2channel browser written in C++ using gtkmm3 (patched). This is alpha version by experimental support.」としていますが、#2のビルドを確認した段階で更新いたしますので、descriptionの文章で提案いただければありがたいです。

@naniwaKun
Copy link

@ma8ma
連投すいません。Archlinux向けにドキュメントを作成してくれるのであれば、AURのパッケージ名の「jd-gtk3」の紹介と、AURヘルパーを用いたインストールコマンド例の紹介「$yay -S jd-gtk3」を頂ければありがたいです。AURにjd-gtk3のページ (https://aur.archlinux.org/packages/jd-gtk3/) については、AURのパッケージになにかあった時(私がメンテナとしての仕事をサボったときか、ミスをしている場合、arch特有の問題が発生した場合などです。)の窓口として紹介して頂けると整理できるかと思います。お知らせするのが筋ですが、検索の手間をかけてしまいました。今後とも宜しくお願いいたします。

@ma8ma
Copy link
Owner Author

ma8ma commented Sep 29, 2018

@naniwaKun ドキュメントのコマンドわかりました。descriptionはそのままで問題ないです。

@ma8ma
Copy link
Owner Author

ma8ma commented Sep 29, 2018

@naniwaKun jd-gtk3とyayのリンク、インストールコマンドを載せてみました。 (df584e4)

@naniwaKun
Copy link

@ma8ma 確認しました。問題なさそうです。AURに#2のビルドスクリプトを更新したらお知らせします!

ma8ma added a commit that referenced this pull request Oct 26, 2018
GTK3サポートを追加する(開発版)

* GTK2の代わりにGTK3を使うconfigureオプションを追加する
* GTK2版でもcairoを使ってスレビューを描画する

詳細はPull Requestを参照してください
#1

Closes #4
@ma8ma ma8ma deleted the gtk3-idea branch October 26, 2018 23:17
ma8ma pushed a commit that referenced this pull request Jan 27, 2019
Always include crypt.h header for crypt function
ma8ma added a commit that referenced this pull request Feb 15, 2020
Occurred a buffer overrun on `make test` in the case of the build
using gcc-9 with configure CXXFLAGS="-fsanitize=address
-fno-omit-frame-pointer" LDFLAGS="-fsanitize=address".

==20309==ERROR: AddressSanitizer: global-buffer-overflow on address 0x56135a804967 at pc 0x56135a235afb bp 0x7fff9ce8b3e0 sp 0x7fff9ce8b3d0
READ of size 1 at 0x56135a804967 thread T0
    #0 0x56135a235afa in MISC::is_url_scheme_impl(char const*, int*) src/jdlib/miscutil.cpp:963
    #1 0x561359a69946 in MISC::is_url_scheme(char const*, int*) ../src/jdlib/miscutil.h:272
    #2 0x561359a69946 in TestBody test/gtest_jdlib_miscutil.cpp:94
    (snip)

0x56135a804967 is located 0 bytes to the right of global variable '*.LC85' defined in 'gtest_jdlib_miscutil.cpp' (0x56135a804960) of size 7
  '*.LC85' is ascii string 'sssp:/'
0x56135a804967 is located 57 bytes to the left of global variable '*.LC86' defined in 'gtest_jdlib_miscutil.cpp' (0x56135a8049a0) of size 32
  '*.LC86' is ascii string 'MISC::is_url_scheme( "sssp:/" )'
SUMMARY: AddressSanitizer: global-buffer-overflow src/jdlib/miscutil.cpp:963 in MISC::is_url_scheme_impl(char const*, int*)
Shadow bytes around the buggy address:
  0x0ac2eb4f88d0: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 05
  0x0ac2eb4f88e0: f9 f9 f9 f9 00 00 02 f9 f9 f9 f9 f9 07 f9 f9 f9
  0x0ac2eb4f88f0: f9 f9 f9 f9 00 00 00 00 f9 f9 f9 f9 06 f9 f9 f9
  0x0ac2eb4f8900: f9 f9 f9 f9 00 00 00 07 f9 f9 f9 f9 05 f9 f9 f9
  0x0ac2eb4f8910: f9 f9 f9 f9 00 00 00 06 f9 f9 f9 f9 06 f9 f9 f9
=>0x0ac2eb4f8920: f9 f9 f9 f9 00 00 00 07 f9 f9 f9 f9[07]f9 f9 f9
  0x0ac2eb4f8930: f9 f9 f9 f9 00 00 00 00 f9 f9 f9 f9 00 06 f9 f9
  0x0ac2eb4f8940: f9 f9 f9 f9 00 00 00 00 00 00 f9 f9 f9 f9 f9 f9
  0x0ac2eb4f8950: 00 00 02 f9 f9 f9 f9 f9 07 f9 f9 f9 f9 f9 f9 f9
  0x0ac2eb4f8960: 02 f9 f9 f9 f9 f9 f9 f9 00 05 f9 f9 f9 f9 f9 f9
  0x0ac2eb4f8970: 00 00 00 00 00 07 f9 f9 f9 f9 f9 f9 00 00 01 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==20309==ABORTING
ma8ma added a commit that referenced this pull request Feb 16, 2020
Occurred a buffer overrun on `make test` in the case of the build
using gcc-9 with configure CXXFLAGS="-fsanitize=address
-fno-omit-frame-pointer" LDFLAGS="-fsanitize=address".

==20309==ERROR: AddressSanitizer: global-buffer-overflow on address 0x56135a804967 at pc 0x56135a235afb bp 0x7fff9ce8b3e0 sp 0x7fff9ce8b3d0
READ of size 1 at 0x56135a804967 thread T0
    #0 0x56135a235afa in MISC::is_url_scheme_impl(char const*, int*) src/jdlib/miscutil.cpp:963
    #1 0x561359a69946 in MISC::is_url_scheme(char const*, int*) ../src/jdlib/miscutil.h:272
    #2 0x561359a69946 in TestBody test/gtest_jdlib_miscutil.cpp:94
    (snip)

0x56135a804967 is located 0 bytes to the right of global variable '*.LC85' defined in 'gtest_jdlib_miscutil.cpp' (0x56135a804960) of size 7
  '*.LC85' is ascii string 'sssp:/'
0x56135a804967 is located 57 bytes to the left of global variable '*.LC86' defined in 'gtest_jdlib_miscutil.cpp' (0x56135a8049a0) of size 32
  '*.LC86' is ascii string 'MISC::is_url_scheme( "sssp:/" )'
SUMMARY: AddressSanitizer: global-buffer-overflow src/jdlib/miscutil.cpp:963 in MISC::is_url_scheme_impl(char const*, int*)
Shadow bytes around the buggy address:
  0x0ac2eb4f88d0: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 05
  0x0ac2eb4f88e0: f9 f9 f9 f9 00 00 02 f9 f9 f9 f9 f9 07 f9 f9 f9
  0x0ac2eb4f88f0: f9 f9 f9 f9 00 00 00 00 f9 f9 f9 f9 06 f9 f9 f9
  0x0ac2eb4f8900: f9 f9 f9 f9 00 00 00 07 f9 f9 f9 f9 05 f9 f9 f9
  0x0ac2eb4f8910: f9 f9 f9 f9 00 00 00 06 f9 f9 f9 f9 06 f9 f9 f9
=>0x0ac2eb4f8920: f9 f9 f9 f9 00 00 00 07 f9 f9 f9 f9[07]f9 f9 f9
  0x0ac2eb4f8930: f9 f9 f9 f9 00 00 00 00 f9 f9 f9 f9 00 06 f9 f9
  0x0ac2eb4f8940: f9 f9 f9 f9 00 00 00 00 00 00 f9 f9 f9 f9 f9 f9
  0x0ac2eb4f8950: 00 00 02 f9 f9 f9 f9 f9 07 f9 f9 f9 f9 f9 f9 f9
  0x0ac2eb4f8960: 02 f9 f9 f9 f9 f9 f9 f9 00 05 f9 f9 f9 f9 f9 f9
  0x0ac2eb4f8970: 00 00 00 00 00 07 f9 f9 f9 f9 f9 f9 00 00 01 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==20309==ABORTING
ma8ma added a commit that referenced this pull request May 16, 2020
タイマーを使うコードで寿命管理が徹底されていなかったためメモリリークが
発生していた。スマートポインターを返すように変更してリークを修正する。

gcc-9のログ
```
Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7f18a17e03d1 in operator new(unsigned long) (/lib/x86_64-linux-gnu/liblsan.so.0+0x103d1)
    #1 0x55ff4e8a91f5 in JDLIB::Timeout::connect(sigc::slot<bool, sigc::nil, sigc::nil, sigc::nil, sigc::nil, sigc::nil, sigc::nil, sigc::nil>, unsigned int) src/jdlib/timeout.cpp:54
    #2 0x55ff4df0f034 in ARTICLE::ArticleViewMain::show_instruct_diag() src/article/articleview.cpp:654
    #3 0x55ff4df15888 in ARTICLE::ArticleViewMain::update_finish() src/article/articleview.cpp:593
    #4 0x55ff4e3ed537 in SKELETON::Admin::update_finish(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) src/skeleton/admin.cpp:1645
```
ma8ma added a commit that referenced this pull request May 16, 2020
タイマーを使うコードで寿命管理が徹底されていなかったためメモリリークが
発生していた。スマートポインターを返すように変更してリークを修正する。

gcc-9のログ
```
Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7f18a17e03d1 in operator new(unsigned long) (/lib/x86_64-linux-gnu/liblsan.so.0+0x103d1)
    #1 0x55ff4e8a91f5 in JDLIB::Timeout::connect(sigc::slot<bool, sigc::nil, sigc::nil, sigc::nil, sigc::nil, sigc::nil, sigc::nil, sigc::nil>, unsigned int) src/jdlib/timeout.cpp:54
    #2 0x55ff4df0f034 in ARTICLE::ArticleViewMain::show_instruct_diag() src/article/articleview.cpp:654
    #3 0x55ff4df15888 in ARTICLE::ArticleViewMain::update_finish() src/article/articleview.cpp:593
    #4 0x55ff4e3ed537 in SKELETON::Admin::update_finish(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) src/skeleton/admin.cpp:1645
```
ma8ma added a commit that referenced this pull request Mar 19, 2023
開発者向けテスト版の更新(スナップショット)

NOTE: 互換性をしっかりとチェックしていないためテスト用の
キャッシュディレクトリを用意することを推奨。

master(82a564b)からの主な変更点
- DAT/HTMLパーサーの機能を強化
- リファクタリング
- テストケースを追加

---
FIXME: ChunkedDecoder: heap-buffer-overflow

`ChunkedDecoder`のデコード処理で特定の条件のときバッファーオーバーフロー
が発生しました。以下の2条件を満たすときバッファの末尾にヌル文字を
書き込もうとしたときバッファの境界を越えるアクセスが起きます。

* 入力の長さと用意したバッファの長さが等しい
* デコード結果の長さと入力の長さが等しい

ASan report
```
==182784==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61d0014e0480 at pc 0x564e775c611b bp 0x7f763bcc0f00 sp 0x7f763bcc0ef0
WRITE of size 1 at 0x61d0014e0480 thread T48
    #0 0x564e775c611a in JDLIB::ChunkedDecoder::decode(char*, unsigned long&) ../src/jdlib/loader.cpp:351
    #1 0x564e775d6748 in JDLIB::Loader::run_main() ../src/jdlib/loader.cpp:949
    #2 0x564e775f5e9c in void std::__invoke_impl<void, void (JDLIB::Loader::*)(), JDLIB::Loader*>(std::__invoke_memfun_deref, void (JDLIB::Loader::*&&)(), JDLIB::Loader*&&) /usr/include/c++/12/bits/invoke.h:74
    #3 0x564e775f5a4c in std::__invoke_result<void (JDLIB::Loader::*)(), JDLIB::Loader*>::type std::__invoke<void (JDLIB::Loader::*)(), JDLIB::Loader*>(void (JDLIB::Loader::*&&)(), JDLIB::Loader*&&) /usr/include/c++/12/bits/invoke.h:96
    #4 0x564e775f57b9 in void std::thread::_Invoker<std::tuple<void (JDLIB::Loader::*)(), JDLIB::Loader*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/12/bits/std_thread.h:252
    #5 0x564e775f5607 in std::thread::_Invoker<std::tuple<void (JDLIB::Loader::*)(), JDLIB::Loader*> >::operator()() /usr/include/c++/12/bits/std_thread.h:259
    #6 0x564e775f53f9 in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (JDLIB::Loader::*)(), JDLIB::Loader*> > >::_M_run() /usr/include/c++/12/bits/std_thread.h:210
    #7 0x7f76586dc3a2  (/lib/x86_64-linux-gnu/libstdc++.so.6+0xdc3a2)
    #8 0x7f7657a90401 in start_thread nptl/pthread_create.c:442
    #9 0x7f7657b1f58f in clone3 ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

0x61d0014e0480 is located 0 bytes to the right of 2048-byte region [0x61d0014dfc80,0x61d0014e0480)
allocated by thread T48 here:
    #0 0x7f765a4c0488 in operator new(unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:95
    #1 0x564e76c068ca in std::__new_allocator<char>::allocate(unsigned long, void const*) /usr/include/c++/12/bits/new_allocator.h:137
    #2 0x564e76c04f6e in std::allocator_traits<std::allocator<char> >::allocate(std::allocator<char>&, unsigned long) /usr/include/c++/12/bits/alloc_traits.h:464
    #3 0x564e76c02ab9 in std::_Vector_base<char, std::allocator<char> >::_M_allocate(unsigned long) /usr/include/c++/12/bits/stl_vector.h:378
    #4 0x564e76c00813 in std::vector<char, std::allocator<char> >::_M_default_append(unsigned long) /usr/include/c++/12/bits/vector.tcc:650
    #5 0x564e76bfee6b in std::vector<char, std::allocator<char> >::resize(unsigned long) /usr/include/c++/12/bits/stl_vector.h:1011
    #6 0x564e775d3583 in JDLIB::Loader::run_main() ../src/jdlib/loader.cpp:798
    #7 0x564e775f5e9c in void std::__invoke_impl<void, void (JDLIB::Loader::*)(), JDLIB::Loader*>(std::__invoke_memfun_deref, void (JDLIB::Loader::*&&)(), JDLIB::Loader*&&) /usr/include/c++/12/bits/invoke.h:74
    #8 0x564e775f5a4c in std::__invoke_result<void (JDLIB::Loader::*)(), JDLIB::Loader*>::type std::__invoke<void (JDLIB::Loader::*)(), JDLIB::Loader*>(void (JDLIB::Loader::*&&)(), JDLIB::Loader*&&) /usr/include/c++/12/bits/invoke.h:96
    #9 0x564e775f57b9 in void std::thread::_Invoker<std::tuple<void (JDLIB::Loader::*)(), JDLIB::Loader*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/12/bits/std_thread.h:252
    #10 0x564e775f5607 in std::thread::_Invoker<std::tuple<void (JDLIB::Loader::*)(), JDLIB::Loader*> >::operator()() /usr/include/c++/12/bits/std_thread.h:259
    #11 0x564e775f53f9 in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (JDLIB::Loader::*)(), JDLIB::Loader*> > >::_M_run() /usr/include/c++/12/bits/std_thread.h:210
    #12 0x7f76586dc3a2  (/lib/x86_64-linux-gnu/libstdc++.so.6+0xdc3a2)

Thread T48 created by T0 here:
    #0 0x7f765a44af75 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:207
    #1 0x7f76586dc478 in std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) (/lib/x86_64-linux-gnu/libstdc++.so.6+0xdc478)
    #2 0x564e775d1f1d in JDLIB::Loader::create_thread() ../src/jdlib/loader.cpp:745
    #3 0x564e775d18cf in JDLIB::Loader::run(SKELETON::Loadable*, JDLIB::LOADERDATA const&) ../src/jdlib/loader.cpp:724
    #4 0x564e778f53e1 in SKELETON::Loadable::start_load(JDLIB::LOADERDATA const&) ../src/skeleton/loadable.cpp:123
    #5 0x564e7724c305 in DBTREE::BoardBase::download_subject(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool) ../src/dbtree/boardbase.cpp:1030
    #6 0x564e772bf21b in DBTREE::board_download_subject(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) ../src/dbtree/interface.cpp:422
    #7 0x564e76ddd485 in BOARD::BoardViewBase::show_view() ../src/board/boardviewbase.cpp:1294
    #8 0x564e76d9469a in BOARD::BoardView::show_view() ../src/board/boardview.cpp:115
    #9 0x564e76d94040 in BOARD::BoardView::reload() ../src/board/boardview.cpp:102
    #10 0x564e76de86f9 in BOARD::BoardViewBase::operate_view(int) ../src/board/boardviewbase.cpp:1552
    #11 0x564e76dfef89 in BOARD::BoardViewBase::slot_key_press(_GdkEventKey*) ../src/board/boardviewbase.cpp:2227
    #12 0x564e76e3f089 in sigc::bound_mem_functor1<bool, BOARD::BoardViewBase, _GdkEventKey*>::operator()(_GdkEventKey* const&) const /usr/include/sigc++-2.0/sigc++/functors/mem_fun.h:2066
    #13 0x564e76e39650 in sigc::adaptor_functor<sigc::bound_mem_functor1<bool, BOARD::BoardViewBase, _GdkEventKey*> >::deduce_result_type<_GdkEventKey* const&, void, void, void, void, void, void>::type sigc::adaptor_functor<sigc::bound_mem_functor1<bool, BOARD::BoardViewBase, _GdkEventKey*> >::operator()<_GdkEventKey* const&>(_GdkEventKey* const&) const /usr/include/sigc++-2.0/sigc++/adaptors/adaptor_trait.h:89
    #14 0x564e76e3464f in sigc::internal::slot_call<sigc::bound_mem_functor1<bool, BOARD::BoardViewBase, _GdkEventKey*>, bool, _GdkEventKey*>::call_it(sigc::internal::slot_rep*, _GdkEventKey* const&) /usr/include/sigc++-2.0/sigc++/functors/slot.h:451
    #15 0x564e76b74962 in sigc::internal::signal_emit1<bool, _GdkEventKey*, sigc::nil>::emit(sigc::internal::signal_impl*, _GdkEventKey* const&) /usr/include/sigc++-2.0/sigc++/signal.h:952
    #16 0x564e76b65d3d in sigc::signal1<bool, _GdkEventKey*, sigc::nil>::emit(_GdkEventKey* const&) const /usr/include/sigc++-2.0/sigc++/signal.h:2955
    #17 0x564e77a1eba4 in SKELETON::JDTreeViewBase::on_key_press_event(_GdkEventKey*) ../src/skeleton/treeviewbase.cpp:387
    #18 0x564e77849a46 in SKELETON::DragTreeView::on_key_press_event(_GdkEventKey*) ../src/skeleton/dragtreeview.cpp:431
    #19 0x7f765a10a317 in Gtk::Widget_Class::key_press_event_callback(_GtkWidget*, _GdkEventKey*) (/lib/x86_64-linux-gnu/libgtkmm-3.0.so.1+0x30a317)

SUMMARY: AddressSanitizer: heap-buffer-overflow ../src/jdlib/loader.cpp:351 in JDLIB::ChunkedDecoder::decode(char*, unsigned long&)
Shadow bytes around the buggy address:
  0x0c3a80294040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a80294050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a80294060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a80294070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a80294080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c3a80294090:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3a802940a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3a802940b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3a802940c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3a802940d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a802940e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==182784==ABORTING
```
ma8ma added a commit that referenced this pull request Jun 11, 2023
AddressSanitizerを有効にしてスレビューを表示したときに
`Pango::FontMetrics`のオブジェクトが開放されず
メモリリークが発生したためC APIに書き直して修正します。

AddressSanitizerのレポート
```
Direct leak of 40 byte(s) in 1 object(s) allocated from:
    #0 0x7fd2d34decaf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7fd2d1fa1948 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5f948) (BuildId: 88d7a1c8d79fc1becb2737be566dd98523394d55)
    #2 0x55c83a6e64a6 in ARTICLE::DrawAreaBase::init_font() ../src/article/drawareabase.cpp:357
    #3 0x55c83a6f14b7 in ARTICLE::DrawAreaBase::clear_screen() ../src/article/drawareabase.cpp:742
(snip)

Direct leak of 40 byte(s) in 1 object(s) allocated from:
    #0 0x7fd2d34decaf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7fd2d1fa1948 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5f948) (BuildId: 88d7a1c8d79fc1becb2737be566dd98523394d55)
    #2 0x55c83a6e64a6 in ARTICLE::DrawAreaBase::init_font() ../src/article/drawareabase.cpp:357
    #3 0x55c83a6df61b in ARTICLE::DrawAreaBase::setup(bool, bool, bool) ../src/article/drawareabase.cpp:211
(snip)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants