2011-01-25 8 views
12

間違った関数を呼び出すと思われるC++コード(他人によって書かれた)があります。状況は次のとおりです。C++はオブジェクトの完全に間違った(仮想)メソッドを呼び出す

UTF8InputStreamFromBuffer* cstream = foo(); 
wstring fn = L"foo"; 
DocumentReader* reader; 

if (a_condition_true_for_some_files_false_for_others) { 
    reader = (DocumentReader*) _new GoodDocumentReader(); 
} else { 
    reader = (DocumentReader*) _new BadDocumentReader(); 
} 

// the crash happens inside the following call 
// when a BadDocumentReader is used 
doc = reader->readDocument(*cstream, fn); 

条件がtrueのファイルは正常に処理されます。それが誤ってクラッシュするもの。

class GenericDocumentReader { 
    virtual Document* readDocument(InputStream &strm, const wchar_t * filename) = 0; 
} 

class DocumentReader : public GenericDocumentReader { 
    virtual Document* readDocument(InputStream &strm, const wchar_t * filename) { 
     // some stuff 
    } 
}; 

class GoodDocumentReader : public DocumentReader { 
    Document* readDocument(InputStream & strm, const wchar_t * filename); 
} 

class BadDocumentReader : public DocumentReader { 
    virtual Document* readDocument(InputStream &stream, const wchar_t * filename); 
    virtual Document* readDocument(const LocatedString *source, const wchar_t * filename); 
    virtual Document* readDocument(const LocatedString *source, const wchar_t * filename, Symbol inputType); 
} 

次も関連しています:DocumentReaderのクラス階層は次のようになります

のVisual C++デバッガで実行
class UTF8InputStreamFromBuffer : public wistringstream { 
    // foo 
}; 
typedef std::basic_istream<wchar_t> InputStream; 

、それはBadDocumentReader上readDocumentコールが

ない呼び出していることを示しています
readDocument(InputStream&, const wchar_t*) 

ではなく、むしろ

readDocument(const LocatedString* source, const wchar_t *, Symbol) 

これは、すべてのreadDocumentsにcoutステートメントを張り付けることで確認できます。呼び出しの後、source引数はもちろんゴミでいっぱいで、まもなくクラッシュが発生します。 LocatedStringはInputStreamからの1つの引数を持つ暗黙的なコンストラクタを持っていますが、coutをチェックすると呼び出されないことがわかります。何がこれを説明することができる任意のアイデア?

を編集します。その他の可能性のある関連情報:DocumentReaderクラスは、呼び出しコードとは別のライブラリにあります。私はまた、すべてのコードの完全な再構築を行い、問題は残っていました。

編集2:私は、Visual C++ 2008

編集3を使用しています:私は同じ動作で「最低限コンパイルの例」を作ってみましたが、問題を再現することができませんでした。

編集4

ビリーONealの提案で、IはBadDocumentReaderヘッダ内readDocument方法の順序を変更しようとしました。確かに、私が注文を変更すると、それは関数のどれが呼び出されるかを変更します。これは私の疑惑を確認するために、vtableにインデックスを付けて何か変なことが起こっているように見えますが、何が原因かわかりません。

編集5: ここでは、関数呼び出しの前に数行のための解体だ:

00559728 mov   edx,dword ptr [reader] 
0055972E mov   eax,dword ptr [edx] 
00559730 mov   ecx,dword ptr [reader] 
00559736 mov   edx,dword ptr [eax] 
00559738 call  edx 

私は多くのアセンブリを知らないが、それは読者の変数のポインタを逆参照ているように、私には見えます。メモリのこの部分に格納されている最初のものは、vtableへのポインタでなければならないので、それをeaxにデリファレンスします。次に、を最初にのものをedxのvtableに置き、それを呼び出します。メソッドの異なる順序で再コンパイルすることはこれを変更していないようです。常にvtableの最初のものを呼びたいと思っています。 (私はこれを完全に誤解している可能性があり、組み立ての知識は全くありません...)

ありがとうございます。

編集6:問題が見つかりました。誰もが時間を無駄にしていたことを謝ります。問題は、GoodDocumentReaderがDocumentReaderのサブクラスとして宣言されているはずだったが、実際はそうではなかったことです。 Cスタイルのキャストはコンパイラのエラーを抑制していました(@ sellibitzeを聞いたはずですが、あなたのコメントを回答として投稿したい場合は、それを正しいものとしてマークします)。難しいのは、誰かがGoodDocumentReaderに2つ以上の仮想関数を追加して、もはや運が適切な関数を呼び出していないようになるまで、改訂まで純粋な事故によってコードが数ヶ月間働いていたことです。

+0

「DefaultDocumentReader」とは何ですか? –

+2

これを問題を示す最小限のコンパイル可能な例にカットできますか? –

+0

Oli:申し訳ありませんが、 "BadDocumentReader"である必要があります。私は投稿を更新しました。 –

答えて

2

まず、Cキャストを取り除こうとします。

  • それは、完全に不必要であるベースに派生(それがになっていないが)それは、実際には、それがどのように見える

バグを引き起こす可能性があり

  • 言語で自然であるからキャストコンパイラのバグ...それは確かにVSの最初のいずれかではありません。私は残念ながら手元にVS 2008はありません

    、GCCでキャストが正しく行われます

    struct Base1 
    { 
        virtual void foo() {} 
    }; 
    
    struct Base2 
    { 
        virtual void bar() {} 
    }; 
    
    struct Derived: Base1, Base2 
    { 
    }; 
    
    int main(int argc, char* argv[]) 
    { 
        Derived d; 
        Base1* b1 = (Base1*) &d; 
        Base2* b2 = (Base2*) &d; 
    
        std::cout << "Derived: " << &d << ", Base1: " << b1 
               << ", Base2: " << b2 << "\n"; 
    
        return 0; 
    } 
    
    
    > Derived: 0x7ffff1377e00, Base1: 0x7ffff1377e00, Base2: 0x7ffff1377e08 
    
  • 11

    これは、さまざまなソースファイルがクラスのvtableレイアウトで一致しないために起こります。関数を呼び出すコードは、readDocument(InputStream &, const wchar_t *)が特定のオフセットにあると考えていますが、実際のvtableは異なるオフセットにあります。

    これは通常、クラスまたはその親クラスの仮想メソッドを追加または削除してvtableを変更した後、別のソースファイルを再コンパイルしないで1つのソースファイルを再コンパイルするときに発生します。そして、互換性のないオブジェクトファイルを取得し、それらをリンクすると事が急増します。

    この問題を解決するには、すべてのコードを完全にクリーンアップして再構築します。ライブラリコードとライブラリを使用するコードの両方です。ライブラリへのソースコードがなくても、クラス定義のヘッダーファイルがある場合、それはオプションではありません。その場合、クラス定義を変更することはできません。クラス定義を元に戻し、すべてのコードを再コンパイルする必要があります。

    +0

    +1。 .hファイルはvtableを含むオブジェクトのレイアウトを定義します。ライブラリを再コンパイルせずに変更すると、すべてが本当にうんざりになります。 –

    +1

    理論的には、ソースが異なるコンパイラ、異なるバージョンまたは異なる設定でコンパイルされている場合、これは問題です。しかし、実際には、この問題が発生するためにはコンパイラー/オプションがかなり異なっている必要があります(再コンパイルが必要であることが検出されない場合はビルドシステムが非常に悪いとは言えません)。私は、ポインタの値を正しくシフトしないCスタイルのキャストで問題があると思う(派生クラスのvtableから基本クラスのvtableに戻す)。 –

    +0

    @Mikael Persson:クラスレイアウトが変更される前に非常によく似た問題に遭遇しましたが、それを使用したすべてのソースファイルが再コンパイルされませんでした。その理由は、使用されているコンパイラフラグが '-I- -Isomedir'のようなものだったためです。依存関係が' -MM'で生成されたとき、 'somedir'のヘッダは依存関係としてリストアップされませんでした。 –

    0

    アセンブリに基づいて、バインディングが動的で、vtableの最初のエントリからかなり明らかです。問題は、どの仮想テーブルです!?! Cスタイルのキャストの代わりにstatic_castを使用することをお勧めします(もちろん、@VJo:dynamic_castはこの場合は必要ありません)。標準では、ポインタBadDocumentReader* ptrは、キャストstatic_cast<DocumentReader*>(ptr)と同じ実際の値(アドレス)を持つ必要はありません。これは、呼び出しが、BadDocumentReaderのvtableの最初のエントリにバインドされ、その基本クラスのvtableにバインドされない理由を説明します。そして、btw、この場合、キャストは一切必要ありません。

    asmと実際には同意しないが、まだよく分かっている可能性があります。 reader->readDocumentを呼び出すのと同じスコープでBadDocumentReaderを作成するので、コンパイラは少し賢明になり、vtableで動的に検索することなくコールを解決できると判断します。これは、リーダポインタの「実際の」タイプが実際にはBadDocumentReaderであることを知っているからです。したがって、vtableをバイナリしてコールを静的にバインドします。少なくとも、それは私がほぼ同じ状況で私に起こった1つの可能性です。しかし、asmに基づいて、私は最初の可能性があなたのケースで発生している可能性があると確信しています。

    +0

    Cスタイルのキャストは 'static_cast'を最初に試してみるべきです。だから私はここで何が起こっているのか考えません。 –

    0

    を、私はこの問題を持っていたし、私にとっての問題は、私はクラスのメンバ変数に格納されたということでした。私はそれをポインタに変更し、新規/削除を伴うと、子クラスとその関数を正常に登録しました。

    関連する問題