2010-12-30 13 views
0

私は、固定サイズの配列とその配列へのインデックスとして実装されたカスタムFastStackクラスを持っています。new演算子が既存のオブジェクトを上書きする

私のコピーコンストラクタでは、私は配列を割り当て、コピーの配列から各オブジェクトを新しい配列に割り当てます。スタック上のオブジェクトにはいくつかのrefcountingがあるため、単純なコピーではなく代入が使用されます。

問題は、配列を割り当てるときに、他のスタックの配列の一部を上書きすることがあることです。予想されるように、これは、そのデータが逆参照されると最終的なセグメンテーションフォールトにつながる。

class FastStack { 
private: 
    int m_size, m_ptr; 
    ObjectRef* m_stack; 

public: 
    FastStack(int size) : m_size(size), m_ptr(-1) { 
     m_stack = new ObjectRef[m_size]; 
    } 

    FastStack(const FastStack& copy) : m_size(copy.m_size), m_ptr(copy.m_ptr) { 
     long a = (long)copy.m_stack[0]; 

     m_stack = new ObjectRef[m_size]; 

     if ((long)copy.m_stack[0] != a) 
      fprintf(stderr, "\nWe have a serious problem!\n\n"); 

     for (int i = 0; i <= m_ptr; i++) 
      m_stack[i] = copy.m_stack[i]; 
    } 

    ~FastStack() { 
     delete[] m_stack; 
    } 
}; 

class ObjectRef { 
private: 
    DataObj* m_obj; 

public: 
    ObjectRef() : m_obj(0) { } 

    ObjectRef(DataObj* obj) : m_obj(obj) { 
     if (m_obj) m_obj->addRef(); 
    } 

    ObjectRef(const ObjectRef& obj) : m_obj(obj.m_obj) { 
     if (m_obj) m_obj->addRef(); 
    } 

    ~ObjectRef() { 
     if (m_obj) m_obj->delRef(); 
    } 

    ObjectRef& operator=(DataObj* obj) { 
     if (obj) obj->addRef(); 
     if (m_obj) m_obj->delRef(); 
     m_obj = obj; 
     return *this; 
    } 

    ObjectRef& operator=(const ObjectRef& obj) { 
     if (obj.m_obj) obj.m_obj->addRef(); 
     if (m_obj) m_obj->delRef(); 
     m_obj = obj.m_obj; 
     return *this; 
    } 
}; 

「私たちには深刻な問題があります。 segfaultの直前にある行を調べて、gdbを実行すると、newによって作成されたObjectRefの1つが他のスタックの配列と同じアドレスを持つことがわかります。

私の最初の本能は、新しいものがすでに使用中のメモリを割り当ててはならないと言うことですが、明らかにここにあるようですが、何ができるのかについて完全な犠牲を払っています。

を追加しました:私はこれが起こる見る時、でm_size = 2とm_ptr = 0

このプログラムの完全なコードはhttps://github.com/dvpdiner2/pycdcでgithubの上にあるが、かなり複雑とフォローするのは困難です。

+0

コンパイラのバージョンは?サードパーティのライブラリを使用していますか?あなたのビルドで 'sizeof(long)== sizeof(ObjectRef *)'が確実でしょうか?メモリを割り当てる他のプログラムの何千(何百万という)にも影響しないバグを「new operator」に発見したかもしれません。または、私たちに表示されていない他のコードにポインタや配列境界エラーがある可能性があります... – Blastfurnace

+3

'FastStack :: operator ='が見つからず、 'FastStack: :〜Fastack'私たちが本当に必要とするのは、実行時に問題を示す**コンパイル可能な例**です。この**のようなすべてのコードスニペットと同様に、あなたは十分なコード**を見逃しています。 –

+1

'すでに使用されているメモリは絶対に割り当ててはいけませんが、明らかにここにあるようです。 '絶対的なひげ。コードにバグがあります(新規ではありません)。 –

答えて

1
あなたは0 m_ptrから反復ループでは

for (int i = 0; i <= m_ptr; i++) 
    m_stack[i] = copy.m_stack[i]; 

しかし、あなたの配列m_stackは(あなたがそれらを初期化すると仮定して)m_size要素が含まれています。

EDIT:m_stackcopy.m_stackが重複する唯一の方法は、配置new演算子が使用されている場合です。しかし、投稿されたソースによると、あなたはそれをしていませんでした。

+0

ObjectRefコンストラクタは、m_stackが割り当てられたときに呼び出され、配列をデフォルト値で初期化します。 m_sizeは2、m_ptrは0です。 – dpogue

2
時には

newは、すでに他のオブジェクトに割り当てられたメモリを与えることが知られており、それは良いC++のコードを発生したとき

NO ...他の人と共有されていないメモリを有するのを期待して割り当てを繰り返す必要があります!

newはあなたのプログラムがバグです。この100回を黒板に書いてください。

new、すなわち使用、それが指していたオブジェクトが割り当て解除された後、ポインタを逆参照、限界の外にアクセスする(前にあなたは何も悪いしなかった場合、他の誰かしかしに与えられたあなたのメモリを与えることはありません無効化されたイテレータや他の可能性のある「UB」違反の数千件)、newは特別な許可を得て、あなたの鼻孔からデーモンを出すなど、好きなことをすることができます。

あなたのコードは非常に容疑者を表示されますが、私は(あまりにも多くのコードが正確にどのようにそれが宣言されてOBJREFとは何か例えば、隠されている)ことを確認エラーので何も表示されません。しかし、私が見ているところでは、新しいC++実装の違反が多いため(例えば、表示されたクラスにはコンストラクタとコピーコンストラクタがありますが、代入演算子もデストラクタもありません) 。

ただし、表示されているコードの最大の問題は、平らなstd :: vectorのサブセットを模倣するハーフバックやバグのようなものです。しかし、問題が何であるかを正確に言うには、より多くのコンテキストが必要です...これは悪いコードですが、となります。この小さなコードでさえ、何かにアクセスする方法がないので変更され、縮小されていることも明らかです。友人やデータメンバーはプライベートではありません(したがって、これらのオブジェクトで何かを行うことは基本的に不可能です)。

1

std::vectorを使用してメモリを管理し、インデックス(m_ptr)を明示的に管理してください。これにより、これらの問題はすべて解決されます。

しかし、私は真剣にあなたのコードで何が間違っているか見ることができません。オプティマイザと考えて、すぐにacopy.m_stack[0]に注目してください。

long a = (long)copy.m_stack[0]; 

m_stack = new ObjectRef[m_size]; 

if ((long)copy.m_stack[0] != a) 



long a = (long)copy.m_stack[0]; 

if ((long)copy.m_stack[0] != a) 


if (copy.m_stack[0] != copy.m_stack[0]) 

その条件がtrueの場合、あなたは、ヒープが破損し、おそらくしかしにくいスタックしている必要があります。ヒープの破損は、明らかに完全に無関係のコードでは、障害のコードに噛み付く可能性があります。どちらか、それともスレッドで悩んでいて、それについて私たちに教えてくれません。 (そして間違っている)。

もちろん、operator =とdestructorのようないくつかの重要な機能が欠けていますが、ベクトルを使用した場合は自分で実装する必要があります。

+0

メモリ割り当てを手動で管理するのではなく、std :: vectorを使用することを強くお勧めします。 – bcsanches

+0

私は、彼が複数のスレッドで作業しているという疑いがあります。特にエラーはたまにしか発生しないので... – mmmmmmmm

0

私にとっては、コピーコンストラクタへの入力が最も可能性が高いと思われます。

しかし、私はこのコードでは十分な検証が行われておらず、構築されている方法では、バグを見つけたり、誤って使用したりして、途中でクラッシュするまではないと言います。このように書かれたクラスは、あなたが今見ているように、バグを見つけにくいでしょう。あなたは最低で行う必要があります

シンプルなもの: 1.あなただけのm_stackを使用している場合でも、配列の中にあなたのインデックスの前に例外 2.チェックを投げ境界新しいハンドル[0] 3.確認することコピーコンストラクタへの入力は有効です(別名初期化)

ああ、このようなメモリエラーを追跡する最も簡単な方法は、それを評価することです。驚くべきことに、メモリの問題をすばやく洗い流すことができます。

また、問題を単純な再現可能な形式で分離したコードを組み込んだ場合、これは非常に簡単にデバッグできます。

0

あなたは、このことにより、投稿全体のコード置き換えることができます。

class FastStack { 
public: 

protected: 
    std::vector<boost::shared_ptr<DataObj> > m_Stack; 
}; 

を(そして、あなたは、ベクターは、そのメモリを事前に割り当てたことを確認するコンストラクタを追加する必要がある場合)。

他の人がすでに行っているクラスを実装することで、あなたがここでやっているような不必要な作業をしているとき(このケースでは、他の誰かがスマートな人たちです)あなたが解決する以上の問題。

std :: vectorは、割り当てられた配列よりもはるかに優れたコレクションになり、boost::shared_ptr(tr1 :: shared_ptrまたはstd :: shared_ptrがtr1/C++ 0x互換コンパイラを使用している場合)あなたのObjectRefクラスが何をしているのか(ただし、より良い)。あなたのコードからは、DataObjに自身のrefcountが含まれているかのように見えますが、代わりにshared_ptrsを使用するとスクラップすることもできます。

利用可能なツールを使用してください。書くべきコードが少ないほど、バグを書く機会が少なくなります。コードを上記のものに置き換えても問題は残っていれば、あなたのエラーは別の場所にあります。

0

コードが複数のスレッドで動作することがありますか?

新しいものは、すでに割り当てられているメモリを返すことはないので、チェックメッセージを受け取る唯一の方法は、別のスレッドがコピーインスタンスに対して何かをしていることです。

この疑惑は、それが時々起こるという事実によっても引き起こされます。これは一般的にマルチスレッドバグ(時々発生する特別なタイミングが必要なため)です。