ヒープの崩壊について

Question

###前提・実現したいこと
unsigned short型の画像を読み込み、検出フィルター、大津の2値化、白黒反転、ラベリング、ラベルの面積・円形度の計算を経て2値化した画像とラベルを色付けしたものを出力するプログラムを書いているのですが、
roundnessという関数の一つ目のfor文の途中で強制終了してしまいます。
エラー文は以下の通りです。
また、ステップオーバーで1文づつ確認していったところ、そこでエラーは出ず、
2値化した画像を書き込もうとしたときに、
image_write_unchar(outname, w, h, binary);
の中で
「ヒープが壊れています」
とエラーが出て、また*fpを参照すると
「メモリが読み取れません」
と表示されます。
また、freeで配列を解放した時にも同じエラーが出ます。
どこかで配列の扱い方が間違っているのだとは思いますが、どこが間違っているのか私にはわかりません。
お力を貸していただけないでしょうか。
よろしくお願いします。
###発生している問題・エラーメッセージ
①コマンドプロンプトで実行した場合
②ステップオーバー（デバッグモード）で実行した場合

①Unhandled exception at 0x0F5B205E (msvcr120d.dll) in fjt8.exe: 0xC0000005: Access violation reading location 0x40319E42.

②ハンドルされない例外が 0x77769D11 (ntdll.dll) で発生しました(fjt8.exe 内): 0xC0000374: ヒープは壊れています。 (パラメーター: 0x7779D8D0)。

###該当のソースコード

void removefilter(unsigned char *a, int w, int h){
	int i, j, k, l;
	int *s;  //ラベルの面積
	s = (int *)calloc(labelcnt, sizeof(int *));

	printf("labelcnt = %d\n", labelcnt);

	for (k = 0; k < labelcnt; k++){  //面積を計算
		for (i = 0; i < h; i++){
			for (j = 0; j < w; j++){
				if (a[i*w + j] == k){  //もし注目座標のラベルがkなら
					s[k]++;
				}

			}
		}
	}
	s[0] = 0;
	for (k = 0; k < labelcnt; k++){  //面積の出力
		printf("ラベル%dの面積 = %d\n", k, s[k]);
	}
	printf("面積が極端に小さい、また極端に大きいラベルを削除します(100以下または3000以上)\n");
	for (k = 0; k < labelcnt; k++){
		if (s[k] < 100 || 3000 < s[k]){  //もし面積が極端に小さい、また極端に大きいなら
			for (i = 0; i < h; i++){
				for (j = 0; j < w; j++){
					if (a[i*w + j] == k){  //もし該当するラベルの座標なら
						a[i*w + j] = 0;  //ラベルを取り消す
					}

				}
			}
		}
	}
	printf("削除工程を経て残ったものを表示します\n");
	initarray(s, labelcnt);
	for (k = 1; k < labelcnt; k++){  //もう一度面積を計算（残ったもの）
		for (i = 0; i < h; i++){
			for (j = 0; j < w; j++){
				if (a[i*w + j] == k){
					s[k]++;
				}

			}
		}
	}
	s[0] = 0;
	for (k = 0; k < labelcnt; k++){  //改めて面積の出力
		if (s[k] != 0){
			printf("ラベル%dの面積 = %d\n", k, s[k]);
		}
	}

	printf("これで面積の値が極端なものを削除することができました\n");
	printf("次に、円形度による判定を行います。\n");
	
	roundness(a, w, h, s, labelcnt);
	printf("面積の初期化\n");
	initarray(s, labelcnt);
	printf("面積の再計算\n");
	for (k = 0; k < labelcnt; k++){  //もう一度面積を計算（残ったもの）
		for (i = 0; i < h; i++){
			for (j = 0; j < w; j++){
				if (a[i*w + j] == k){
					s[k]++;
				}

			}
		}
	}
	s[0] = 0;
	for (k = 0; k < labelcnt; k++){  //面積の出力
		if (s[k] != 0){
			printf("ラベル%dの面積 = %d\n", k, s[k]);
		}

	}
	printf("残ったラベルは以上です。\n");

	free(s);
	
}
void roundness(unsigned char *a, int w, int h, int *s, int labelcnt){
	int i, j, k, l;

	double *r;  //ラベルの仮想の円の半径
	r = (double *)calloc(labelcnt, sizeof(double *));

	printf("円形度を求めるために仮想的な半径を求めます。\n");
	for (k = 0; k < labelcnt; k++){
		if (s[k] != 0){
			r[k] = sqrt((double)s[k] / pi);  //r*r*pi = sより
			printf("%d:r = %lf, s = %d\n", k, r[k],s[k]);
		}
	}
	printf("次に、図形の中心となる座標を求めます。\n");
	int max_x = 0;
	int min_x = 1024;
	int max_y = 0;
	int min_y = 1024;
	int x[100],y[100];  //ラベルの中心の座標
	initarray(x, 100);
	initarray(y, 100);

	printf("探索開始\n");
	for (k = 0; k < labelcnt; k++){
		if (s[k] != 0){
			max_x = 0;
			min_x = 1024;
			max_y = 0;
			min_y = 1024;
			//printf("ラベル%d:面積あり,%d\n", k,s[k]);
			for (i = 0; i < h; i++){
				for (j = 0; j < w; j++){
					if (a[i*w + j] == k){  //もし注目座標のラベルがkなら
						if (j > max_x){
							max_x = j;
						}
						if (j < min_x){
							min_x = j;
						}

						if (i > max_y){
							max_y = i;
						}
						if (i < min_y){
							min_y = i;
						}
					}
				}
			}
			//printf("ラベル%d:探索終了\n", k);
			x[k] = min_x + (max_x - min_x) / 2;
			y[k] = min_y + (max_y - min_y) / 2;
			printf("label %d: (x,y) = (%d,%d)\n", k, x[k], y[k]);  //中心座標の出力
			//終わったら次のラベルへ
		}
	}
	printf("探索終了\n");
	
	printf("仮想の円と実際のラベルが一致している面積を求め、円形度を求める\n");

	int match_s[100];
	initarray(match_s, 100);
	
	for (k = 0; k < labelcnt; k++){
		if (s[k] != 0){
			for (i = 0; i < h; i++){
				for (j = 0; j < w; j++){
					if ((double)(abs(j - x[k])*abs(j - x[k]) + abs(i - y[k])*abs(i - y[k])) < r[k] * r[k]){  //もし注目画素の座標が仮想の円の中なら
						if (a[i*w + j] == k){  //注目画素の座標がラベルを持っている(=一致しているなら)
							match_s[k]++;
						}
					}
				}
			}
			printf("label %dの一致した面積: %d\n         元の面積: %d\n", k, match_s[k], s[k]);
		}
	}
	printf("各ラベルの円形度を表示します。\n");
	for (k = 0; k < labelcnt; k++){
		if (s[k] != 0){
			printf("label %dの円形度: %lf\n", k, (double)match_s[k] / (double)s[k]);
		}
	}
	printf("円形度が0.8以下のものを削除します\n");
	for (k = 0; k < labelcnt; k++){
		if (((double)match_s[k] / (double)s[k]) < 0.8){  //もし円形度が0.8以下なら
			for (i = 0; i < h; i++){
				for (j = 0; j < w; j++){
					if (a[i*w + j] == k){  //もし該当するラベルの座標なら
						a[i*w + j] = 0;  //ラベルを取り消す
					}

				}
			}
		}
	}
	printf("円形度の値によるラベルの削除終了\n");
	
	free(r); 

	printf("free\n");
	

}
void image_write_unchar(char *filename, int w, int h, unsigned char *im)
{
	printf("output %s\n", filename);

	FILE *fp;
	if ((fp = fopen(filename, "wb")) == NULL){
		printf("File Open Error Occured!\n");
		exit(-1);
	}
	fwrite((unsigned char *)im, sizeof(unsigned char), w * h, fp); fclose(fp);

	printf("output completed\n");
}

###試したこと
配列を宣言するときにmallocとcalloc、静的確保と動的確保、両方試してみました。

###補足情報(言語/FW/ツール等のバージョンなど)
そこのプログラムを実行すると変数:labelcntは88になります。
また上に乗せたソースはエラーが発生した関数のみとなります。

Answer 1

VSは不勉強でよく分かってないのですが、デバッガでスタックトレースできないですか？落ちたところからスタックを見ればエラー箇所が分かると思います。
Chironianさんも書かれていますが、sizeof(～*)はポインタのサイズ（３２ビット環境ではたいてい４バイト）です。従ってsizeof(int*)はたまたま同じサイズで問題は起こらないと思いますが、修正をした方がいいと思います。また、重箱の隅をつつくようですが100以下といった場合≦100となります。（提示されたコードでは100より小さいが正解と思います、）3000も同様に3000より大きいが正解です。

Answer 2

labelcnt の値はいくつでしょう？
ざっと眺めた感じでは

int x[100],y[100];  //ラベルの中心の座標
int match_s[100];

```としているので labelcnt が 100 以上の値になると破壊されませんか？

Answer 3

こんにちは。

r = (double *)calloc(labelcnt, sizeof(double *));

もし、32bitビルドしているのであれば、sizeof(double*)は4です。
しかし、doubleは通常は8バイトですので、領域が足りていないと思います。
double型の領域をlabelcnt分獲得するのですから、sizeof(double)が正しいと思います。

ところで、コードを見る限りC言語ですが、C++のタグで正しいのでしょうか?
C++を使うのでしたら、callocよりはnewを使った方がこのようなミスは発生しにくいと思いますよ。

Comments

[matchdas3333]

回答ありがとうございます。
おっしゃる通りだとは思いますが、変更して実行してみたところまた強制終了してしまいました…
C++であっています。newですね、アドバイス、ありがとうございます。

[Chironian]

なるほど、他にもまだ不明メモリ・アクセスしているところがあるということですね。
配列に値を設定しているところを、暫定的にコメント・アウトしてみて落ちるかどうか確認してみてはどうでしょうか?
異常動作しない程度の初期化はしておかないといけませんので、初期化部は要注意です。
その結果、落ちなくなったら、コメントアウトしたどれかが原因ですので、前の方から１つ1つもどして行けば原因に辿り着くと思います。

[yohhoy]

C++を使うなら std::vector や std::unique_ptr でメモリリソース管理をすべきかと思います。cmalloc/freeをnew/deleteに置き換えただけでは何も解決しませんし...

追記：言われてみれば malloc -> newでsizeof(型名)のミスは減りますね。とはいえ最大限newは避けた方がいいですけど。

[Chironian]

ああ、これはsizeof(double*)でdouble用メモリ獲得するミスをし難くなることを狙った提案でした。
でも、折角C++を使うのであれば、new/deleteを使わないで済む時は使わない方が良いですね。