Entries

スポンサーサイト

上記の広告は1ヶ月以上更新のないブログに表示されています。
新しい記事を書く事で広告が消せます。

最近見かけた酷いコード14

タグ: C++ 酷いコード VS2005

ぱっと見どこかに問題ありそうなのだが、その実なんとか動きそうで、でもやっぱり欠陥を含んでいるという、なかなか挑発的なコードに出会った。
開発環境はVC8。構造体アライメントは8バイト。バイトオーダーはリトルエンディアン。

unsigned char _buffer[27]; ///< データバッファ

まず27バイトのデータバッファがある。

/// データバッファ範囲
struct BufferRange
{
    size_t offset; ///< データバッファオフセット
    size_t size; ///< データサイズ
};

/// データバッファ格納データ範囲一覧
const BufferRange BUFFER_RANGES[] = {
    {  0, 10 }, // データHoge用バッファ範囲(0?9)
    { 10, 16 }, // データFuga用バッファ範囲(10?25)
    { 26,  1 }, // データPiyo用バッファ範囲(26)
};

/// データバッファ格納データインデックス一覧
enum BufferIndex {
    BUFFER_INDEX_HOGE = 0, ///< データHogeインデックス
    BUFFER_INDEX_FUGA, ///< データFugaインデックス
    BUFFER_INDEX_PIYO, ///< データPiyoインデックス
};

void Store(BufferIndex index, void* data)
{
    const BufferRange& range = BUFFER_RANGES[index];
    ::memcpy(_buffer + range.offset, data, range.size);
}

データバッファには

  1. データHoge(10バイト)
  2. データFuga(16バイト)
  3. データPiyo(1バイト)

の3つのデータを格納することになっている。

// データHoge構造体
struct Hoge
{
    long val1; // 4バイト
    long val2; // 4バイト
    short val3; // 2バイト
};

void StoreHoge(const Hoge& hoge)
{
    Store(BUFFER_INDEX_HOGE, &hoge);
}

StoreHoge関数は構造体Hogeの内容をデータHoge用バッファにコピーする。
構造体Hogeのサイズは4バイト・4バイト・2バイトに境界調整(JIS X 3014:2003 §3.9/5)用2バイトを含めて12バイト。
それに対しデータHoge用バッファのサイズは10バイトで、コピーするバイト数も10バイト。
一見、バッファにコピーするバイト数が足りていないように見える。
が、コピーされない2バイトは境界調整用の空き領域のため、最低限必要な情報量はコピーされる。

void LoadHoge(Hoge& hoge)
{
    const BufferRange& range = BUFFER_RANGES[BUFFER_INDEX_HOGE];
    ::memcpy(&hoge, _buffer + range.offset, sizeof(hoge));
}

LoadHoge関数はデータHoge用バッファから構造体Hogeにデータをコピーする。
この関数はStoreとは逆に2バイト余計に(データHoge用バッファから10バイト、データFuga用バッファから2バイト)コピーしているが、余計な2バイトは構造体末尾の境界調整用の空き領域にコピーされるため問題は起こらない。

// データFuga構造体
struct Fuga
{
    long val1; // 4バイト
    long val2; // 4バイト
    long val3; // 4バイト
};

void StoreFuga(const Fuga& fuga)
{
    Store(BUFFER_INDEX_FUGA, &fuga);
}

構造体Fugaのサイズ(12バイト)がデータFuga用バッファのサイズ(16バイト)より小さいという、Hogeの逆パターン。
一見、書き込み先の方が大きいためバッファオーバーフローの危険はないように見える。
が、逆に書き込み元が小さいためバッファオーバーリードが起こる。
今回書き起こした例は4バイトしかオーバーしないため問題が顕在化することはないかもしれない。
実際のコードでは30バイトほどオーバーしていたため、まれに読み込み可能領域外まで読みに行ってAccess Violationを起こすことがあった。

void LoadFuga(Fuga& fuga)
{
    const BufferRange& range = BUFFER_RANGES[BUFFER_INDEX_FUGA];
    ::memcpy(&fuga, _buffer + range.offset, sizeof(fuga));
}

Loadは問題ない。

// データPiyo列挙体(最大情報量1バイトの符号無し整数)
enum Piyo {
    PIYO1,
    PIYO2,
    PIYO3,
};

void StorePiyo(Piyo piyo)
{
    Store(BUFFER_INDEX_PIYO, &piyo);
}

列挙体Piyoのサイズは処理系依存(JIS X 3014:2003 §7.2/5)だが、今回の環境ではintと同じ4バイト。
それに対してデータPiyo用バッファのサイズは1バイトで、コピーするバイト数も1バイト。
一見、バッファにコピーするバイト数が足りていないように見える。
が、バイトオーダーがリトルエンディアンなので、「コピーする先頭1バイト=下位1バイト=データPiyoの全情報」となり、先頭1バイトだけで必要十分な量のコピーが行われることになる。

void LoadPiyo(Piyo& piyo)
{
    const BufferRange& range = BUFFER_RANGES[BUFFER_INDEX_PIYO];
    piyo = static_cast<Piyo>(_buffer[range.offset]);
}

データバッファの1バイトを列挙型Piyoにstatic_castする。
列挙型の列挙値の範囲にある場合、値は変換によっても変化しないが、そうでない場合の結果の列挙値は規定されていない(JIS X 3014:2003 §7.2/9)。
データバッファの型が符号付き型でかつ列挙型Piyoに128以上の値が与えられた列挙子があったら(符号付き型では負数になるため)問題になるところだが、幸いデータバッファの型は符号無し型なので、ここは特に問題にはならない。
ほかが酷いためつい警戒してしまうが、これはそんな目くじらを立てるようなコードでもないか。

……とまあ、こんな感じのコードが延々と続く。
環境依存であることを理解した上で敢えてこう書いているのか、はたまた「動くようにがんばったらこうなった」のか。
いずれにしろレビューではじくべきコードである。

fgetsかfgetwsか

タグ: VS2005

VC8でShift JIS(マルチバイト文字列)のファイルをUnicode(ワイド文字列)に変換しながら読み込む方法として、

  • fgetws()でUnicodeに変換しながら読む
  • fgets()で読んでMultiByteToWideChar()でUnicodeに変換する

のどちらが速いか。

試しに、手元にあった29207行946459バイトのMS-IME用辞書ファイル(Shift JISのテキストファイル)の読み込み速度を、Pentium 4 3.6GHzのWindows XP PCで比較してみた。

#include <windows.h>
#include <stdio.h>
#include <locale.h>
#include <iostream>

void test()
{
    ::setlocale(LC_ALL, "japanese");

    // MS-IME用辞書ファイルを開く    
    FILE* fp = ::fopen("msime.txt", "rt");
    if (! fp)
    {
        std::cerr << "ひらけない(´・ω・`)" << std::endl;
        return;
    }
    
    DWORD start;
    int line;
    // 交互に5回試す
    for (int i = 0; i < 5; i++)
    {
        wchar_t wbuff[256] = { 0 };
        const size_t wbufflen = sizeof(wbuff) / sizeof(*wbuff);
    
        // fgetws()でUnicodeに変換しながら読む
        ::fseek(fp, 0, SEEK_SET);
        start = ::GetTickCount();
        for (line = 0; ::fgetws(wbuff, wbufflen, fp); line++)
        {
            // 1行254文字以下(改行コード混み)を想定
            if (wbuff[wbufflen - 2] != 0)
            {
                std::cerr << "ながい('・ω・`)" << std::endl;
                break;
            }
        }
        std::cout << "fgetws " << line << "lines " << ::GetTickCount() - start << "ms" << std::endl;
    
        // fgets()で読んでMultiByteToWideChar()でUnicodeに変換する
        ::fseek(fp, 0, SEEK_SET);
        start = ::GetTickCount();
        char buff[wbufflen * 2] = { 0 };
        const size_t bufflen = sizeof(buff) / sizeof(*buff);
        for (line = 0; ::fgets(buff, bufflen, fp); line++)
        {
            // 1行510バイト以下(改行コード混み)を想定
            if (buff[bufflen - 2] != 0)
            {
                std::cerr << "ながい('・ω・`)" << std::endl;
                break;
            }
    
            // CP932で最大510バイトをUnicode最大255文字にマップ
            if (::MultiByteToWideChar(932, MB_PRECOMPOSED, buff, -1, wbuff, wbufflen) == 0)
            {
                std::cerr << "おかしい('・ω・`)" << std::endl;
                break;
            }
        }
        std::cout << "fgets  " << line << "lines " << ::GetTickCount() - start << "ms" << std::endl;
    }
    
    ::fclose(fp);
}

void main()
{
    test();
}
fgetws 29207lines 125ms
fgets  29207lines 15ms
fgetws 29207lines 94ms
fgets  29207lines 16ms
fgetws 29207lines 93ms
fgets  29207lines 16ms
fgetws 29207lines 94ms
fgets  29207lines 15ms
fgetws 29207lines 78ms
fgets  29207lines 16ms

差はおよそ6倍。
fgetws()は「マルチバイト文字列用のバッファを用意する必要がない」とか「マルチバイト文字の泣き別れを気にしなくて済む」という点で楽をできるが、その分コストは高めである。

#pragma pack vs コンパイラ最適化

タグ: C++ VS2005

Visual Studio 2005のARMv4I向けコンパイラが構造体アライメントをどのように扱うか実験。

#pragma pack(push, 1)
// 1バイトアライメントの構造体
// sizeof(Hoge) == 10
struct Hoge
{
    short a; // 2バイト
    int b[2]; // 4バイト×2
};
#pragma pack(pop)

void zerohoge(Hoge& hoge);
void zeroint(int& n);
// 非インライン関数
void zerohoge(Hoge& hoge)
{
    // 3. 1バイトアライメントの構造体のデータメンバに値を代入する
    hoge.a = 0;
    hoge.b[0] = 0;
    hoge.b[1] = 0;
}

// 非インライン関数
void zeroint(int& n)
{
    // 4. 組み込み型に値を代入する
    n = 0;
}
void test()
{
    Hoge hoges[2];

    // 1. 1バイトアライメント構造体の4バイトのデータメンバが4バイト境界に整列されている
    hoges[1].a = 0;
    hoges[1].b[0] = 0;
    hoges[1].b[1] = 0;

    // 2. 1バイトアライメント構造体の4バイトのデータメンバが4バイト境界に整列されていない
    hoges[0].a = 0;
    hoges[0].b[0] = 0;
    hoges[0].b[1] = 0;

    // 3.a. 4バイト境界から始まる1バイトアライメントデータを渡す
    zerohoge(hoges[0]);

    // 3.b. 4バイト境界以外から始まる1バイトアライメントデータを渡す
    zerohoge(hoges[1]);

    // 4.a. 4バイト境界以外から始まる4バイトアライメントデータを渡す(!)
    zeroint(hoges[0].b[0]);

    Hoge* hoge = new Hoge;

    // 5.a 1バイトアライメント構造体のデータメンバにmemsetする
    ::memset(hoge->b, 0, sizeof(hoge->b));

    // memsetが最適化で消滅するのを防止するために非インライン関数呼び出しを行う
    zerohoge(*hoge);

    // 5.b 1バイトアライメント構造体のデータメンバではないとコンパイラに誤認させてmemsetする(!)
    ::memset(&hoge->b[0], 0, sizeof(hoge->b));

    // memset(ry
    zerohoge(*hoge);

    delete hoge;
}

このコードを/O2(実行速度最適化)でビルドし、要所要所「逆アセンブルを表示」しながら実行してみる。

    // 1. 1バイトアライメント構造体の4バイトのデータメンバが4バイト境界に整列されている
    hoges[1].a = 0;
    hoges[1].b[0] = 0;
    hoges[1].b[1] = 0;
00011008  mov         r3, #0 
0001100C  mov         r2, #0 
00011010  mov         r1, #0 
00011014  add         r0, sp, #0 
00011018  strh        r3, [sp, #0xA] 
0001101C  str         r2, [sp, #0xC] 
00011020  str         r1, [sp, #0x10] 

Hoge構造体のアライメントは1バイトだが、hoges[1]のデータメンバbはスタックメモリ上において4バイト境界に整列されることが確実なため、strh(2バイトストア)命令1つとstr(4バイトストア)命令2つという最少の命令数で全てのデータメンバに0を設定している。

    // 2. 1バイトアライメント構造体の4バイトのデータメンバが4バイト境界に整列されていない
    hoges[0].a = 0;
    hoges[0].b[0] = 0;
    hoges[0].b[1] = 0;
00011028  mov         lr, #0 
0001102C  mov         r4, #0 
00011030  mov         r3, lr, lsr #16 
00011034  mov         r2, r4, lsr #16 
00011038  mov         r1, #0 
0001103C  add         r0, sp, #0xA 
00011040  strh        r3, [sp, #4] 
00011044  strh        r2, [sp, #8] 
00011048  strh        r1, hoges 
0001104C  strh        lr, [sp, #2] 
00011050  strh        r4, [sp, #6] 

hoges[0]のデータメンバは、4バイト境界には整列されていないものの2バイト境界には揃えられているので、strh命令5つで0を設定している。

    // 3. 1バイトアライメントの構造体のデータメンバに値を代入する
    hoge.a = 0;
000110B8  mov         r3, #0 
000110BC  strb        r3, hoge 
000110C0  strb        r3, [r0, #1] 
    hoge.b[0] = 0;
000110C4  strb        r3, [r0, #2] 
000110C8  strb        r3, [r0, #3] 
000110CC  strb        r3, [r0, #4] 
000110D0  strb        r3, [r0, #5] 
    hoge.b[1] = 0;
000110D4  strb        r3, [r0, #6] 
000110D8  strb        r3, [r0, #7] 
000110DC  strb        r3, [r0, #8] 
000110E0  strb        r3, [r0, #9] 

zerohoge()の引数hogeは、コンパイル段階では偶数バイト奇数バイトどちらから始まるかも不明なので、どちらでも問題なく動作するようstrb(1バイトストア)命令10個で0を設定している。

    // 4. 組み込み型に値を代入する
    n = 0;
000110E8  mov         r3, #0 
000110EC  str         r3, n 

zeroint()の引数nは4バイトアライメントの組み込み型の参照型(=実引数が4バイト境界に整列されていることを呼び出し側が保証しなければならない)なので、str命令1つで0を設定する。
よって以下のように「4バイト境界に整列されていないデータメンバ」を渡すと、

    // 4.a. 4バイト境界以外から始まる4バイトアライメントデータを渡す(!)
    zeroint(hoges[0].b[0]);
00011058  add         r0, sp, #2 
0001105C  bl          |zeroint ( 110e8h )| 

ARMv4Iのような非境界整列アクセスに対応していない環境では問題が起こる。

Data Abort: Thread=8eff10a8 Proc=8c369c10 'test.exe'
AKY=00002001 PC=000110ec(test.exe+0x000010ec) RA=00011060(test.exe+0x00001060) BVA=1c02fe46 FSR=00000003
Unhandled exception at 0x000110ec in test.exe: 0x80000002: Datatype misalignment.

私の実行環境はここで落ちるので、とりあえずzeroint()の中身をコメントアウトして先に進める。

    Hoge* hoge = new Hoge;
00011060  mov         r0, #0xA 
00011064  bl          000110FC 
00011068  mov         r4, r0 

    // 5.a 1バイトアライメント構造体のデータメンバにmemsetする
    ::memset(hoge->b, 0, sizeof(hoge->b));
0001106C  add         r0, r4, #2 
00011070  mov         r2, #8 
00011074  mov         r1, #0 
00011078  bl          00011114 

ここは特に何の工夫もなく、普通にmemset()を呼んでいる。

    // 5.b 1バイトアライメント構造体のデータメンバではないとコンパイラに誤認させてmemsetする(!)
    ::memset(&hoge->b[0], 0, sizeof(hoge->b));
00011084  mov         r3, #0 
00011088  str         r3, [r4, #2] 
00011090  str         r3, [r4, #6] 

4バイト境界に整列されているはずの8バイトを0初期化するのにmemset()を呼ぶのも煩わしいと、str命令2個で片付けられている。
当然これは非境界整列アクセスになってしまうため、ARMv4Iでは問題になる。

Data Abort: Thread=8efe8658 Proc=8c369d00 'test.exe'
AKY=00004001 PC=00011088(test.exe+0x00001088) RA=00011084(test.exe+0x00001084) BVA=1e030062 FSR=00000003
Unhandled exception at 0x00011088 in test.exe: 0x80000002: Datatype misalignment.

「1バイトアライメントの領域を指すポインタ(参照)」から「4バイトアライメントの領域を指すポインタ(参照)」への変換は、本来reinterpret_cast並に気を配らなければならない行為である。
しかし構造体アライメントはC言語の仕様の範疇ではなく、その変換を明示的に行う方法(unpack_castとでも言おうか)も用意されていない。
コンパイラはコードを信じて「暗黙のunpack」を行うしかなく、プログラマはコンパイラが「誤解」しない書き方を把握しなければならない。

「ビルド後のイベント」でコードカバレッジを計測するvsprops

タグ: VS2005 C++

大分昔に作ったVisual Studio 2005 Team Edition用プロパティシートを久しぶりに使った記念メモ。

<?xml version="1.0" encoding="shift_jis"?>
<VisualStudioPropertySheet
	ProjectType="Visual C++"
	Version="8.00"
	Name="Code Coverage"
	DeleteExtensionsOnClean="*.pdb;*.coverage"
	>
	<Tool
		Name="VCLinkerTool"
		GenerateDebugInformation="true"
		Profile="true"
	/>
	<Tool
		Name="VCNMakeTool"
		BuildCommandLine=""
		ReBuildCommandLine=""
		CleanCommandLine=""
		Output=""
		PreprocessorDefinitions=""
		IncludeSearchPath=""
		ForcedIncludes=""
		AssemblySearchPath=""
		ForcedUsingAssemblies=""
		CompileAsManaged=""
	/>
	<Tool
		Name="VCPostBuildEventTool"
		Description="コードカバレッジ情報を収集しています..."
		CommandLine="set UNIQUECOVERAGEPATH=$(OutDir)\$(ConfigurationName) %DATE:/=.% %TIME::=.%.coverage&#x0D;&#x0A;&quot;$(VSInstallDir)\Team Tools\Performance Tools\vsinstr&quot; /coverage $(Excludes) &quot;$(TargetPath)&quot;&#x0D;&#x0A;&quot;$(VSInstallDir)\Team Tools\Performance Tools\vsperfcmd&quot; /start:coverage /output:&quot;$(CoveragePath)&quot;&#x0D;&#x0A;&quot;$(TargetPath)&quot;&#x0D;&#x0A;&quot;$(VSInstallDir)\Team Tools\Performance Tools\vsperfcmd&quot; /shutdown&#x0D;&#x0A;cp &quot;$(CoveragePath)&quot; &quot;%UNIQUECOVERAGEPATH%&quot;&#x0D;&#x0A;&quot;%UNIQUECOVERAGEPATH%&quot;&#x0D;&#x0A;"
	/>
	<UserMacro
		Name="Excludes"
		Value="/exclude:std::* /exclude:stdext::* /exclude:boost::* /exclude:ATL::* /exclude:CppUnit::*"
	/>
	<UserMacro
		Name="CoveragePath"
		Value="$(OutDir)\$(ConfigurationName).coverage"
	/>
</VisualStudioPropertySheet>
  1. ↑を「CodeCoverage.vsprops」というファイル名で保存しておく
  2. プロジェクトを開く
  3. 構成マネージャでDebug構成をコピーしてCoverage構成を作る
  4. プロパティマネージャでCoverage構成に「CodeCoverage.vsprops」を加える
  5. Coverage構成をビルドする
  6. あとは「ビルド後のイベント」が勝手に成果物を起動してコードカバレッジを計測し、結果のレポートファイルを開いてくれる

コードカバレッジ

このプロパティシートはもともと「ATLやBoostを使ったライブラリをテストするCppUnitベースのユニットテスト」のコードカバレッジを計測するために作ったものなので、バイナリのインストルメント時に

/exclude:std::* /exclude:stdext::* /exclude:boost::* /exclude:ATL::* /exclude:CppUnit::*

として余計なものまで計測に含まれないようにしている。

この結果をHudson辺りで集約できれば素敵なのだが……

プロパティシート作成時に参考にしたページ:

Hudson Warnings PluginとVCBuild /M2

タグ: Hudson VS2005

VCBuild.exeは/Mオプションを使うことでソリューション内の複数のプロジェクトを並行ビルドできる。

C:\temp>VCBuild.exe /M2 test.sln

並行ビルドでは複数のビルドプロセスの診断メッセージが入り交じることになるので、そのメッセージがどのプロセスに属するものかを識別するために、診断メッセージの先頭にはビルドプロセス番号が記される。

1>ビルド 開始: プロジェクト: Hoge、構成: Debug|Win32
2>ビルド 開始: プロジェクト: Fuga、構成: Debug|Win32
1>コンパイルしています...
2>コンパイルしています...
1>hoge.cpp
2>fuga.cpp
1>c:\temp\hoge\hoge.cpp(15) : warning C6386: バッファ オーバーランです:
1>c:\temp\hoge\hoge.cpp(24) : warning C6386: バッファ オーバーランです:
1>c:\temp\hoge\hoge.cpp(34) : warning C6386: バッファ オーバーランです:
1>c:\temp\hoge\hoge.cpp(35) : warning C6386: バッファ オーバーランです:
2>c:\temp\fuga\fuga.cpp(13) : warning C6386: バッファ オーバーランです:
2>c:\temp\fuga\fuga.cpp(36) : warning C6385: 無効なデータです:

しかるにHudson Warnings Plugin 3.2のMSBuildパーサはこのビルドプロセス番号を含む表記に正しく対応しておらず、コンパイラの警告ビューの詳細タブからソースコードを表示させようとしても

Caused by: java.io.FileNotFoundException: 1>c:\temp\hoge\hoge.cpp (No such file or directory)

とエラーになってしまう。
原因はMSBuildパーサがビルドプロセス番号までファイルパスの一部と認識しているからなので、MsBuildParser.javaの正規表現の先頭部分

private static final String MS_BUILD_WARNING_PATTERN = "(?:(?:

にビルドプロセス番号を無視する記述を加えて

private static final String MS_BUILD_WARNING_PATTERN = "(?:\\d+>)?(?:(?:

こうすれば表示できるようになる。

Appendix

タグ

Blog内検索

上記広告は1ヶ月以上更新のないブログに表示されています。新しい記事を書くことで広告を消せます。