CComBSTR::CComBSTR()のSysAllocStringで確保済みのメモリが上書きされる

Question

###環境
Windows10 64bit / Visual Studio 2019
C++17
###知りたいこと
CComBSTR のコンストラクタ内の SysAllocString にて、確保済みのメモリが上書きされる事象が発生しました。
この事象が発生する原因を知りたいです。
###コード

CComBSTR test(const CComSafeArray<VARIANT>& var) {
	return CComBSTR(L"ABC");
}

上述のコードでは、CComSafeArray<VARIANT> 型配列 var の一番最初の要素が L"ABC" に書き換わります。
CComBSTR の該当のコンストラクタは以下の通りです。

CComBSTR(_In_opt_z_ LPCOLESTR pSrc)
    {
        if (pSrc == NULL)
        {
            m_str = NULL;
        }
        else
        {
            m_str = ::SysAllocString(pSrc);
            if (!*this)
            {
                AtlThrow(E_OUTOFMEMORY);
            }
        }
    }

m_str = ::SysAllocString(pSrc); の箇所で m_str に充てられるアドレスが上のコードの var の一番最初の要素のアドレスとなっており、結果、元の値が pSrc で上書きされる様な結果となります。
###補足
CComBSTR test() の引数 var を const にしなくとも同様の事象が発生します。
また、参照渡しでなく値渡しにした場合はこの事象は発生しません。

先日投稿した質問も同じ様な事象が発生しています。
Invoke や SysAllocString の内部で実行される共通の処理が原因ではないか、と思っています。
しかし、自力ではそれを確認する手段がなく、ご助力願いたく質問させて頂きました。

本件、および先日の質問につきまして、
何か思い当たる点のある方は、些細なことでも構いませんので是非ご意見下さい。

Comments

[dodox86]

確証はないですが、使い方が悪いのではないかと思います。
以下の提示のコード、test()は、質問者さんの自作の関数なのですよね。
> CComBSTR test(const CComSafeArray<VARIANT>& var) {
> return CComBSTR(L"ABC");
> }

test()関数をどう使っているか分かりませんが、CComBSTRをスタック上で生成したのをreturnで返しているので、それを呼び出したもので=で受け取るようなことをしていませんか？
以下をみると、CComBSTRクラスでは=演算子をオーバーロードしているはずですが、その辺りの扱いに間違いはないか確認してみてください。

https://docs.microsoft.com/ja-jp/cpp/atl/reference/ccombstr-class?view=msvc-160#operator_eq

[Bull]

当方で適当にコードを作成しテストしてみましたが、現象は確認できませんでした。
現象を再現できる、コンパイル・実行可能なコードを提示することは可能ですか？

[subka]

dodox86 様、Bull 様、ありがとうございます。

ご指摘通り、test()関数は自作関数です。

なお、呼び出し元では以下の様に BSTR 型変数に = で戻り値を渡しており、
CComBSTR クラスの = 演算子のオーバーロードは経由していないと考えています。

CComVariant params;
params.vt = VT_BSTR;
params.bstrVal = test(var);

C++は不慣れなもので恐縮ですが、
最終的には他サイトでも見掛ける以下の形になっていると考えております。

params.bstrVal = CComBSTR(L"ABC");

この認識には相違ないでしょうか。
それとも return で値を返した場合はまた違った処理になってくるのでしょうか。

また、Bull 様のご指摘通り、
提示したコードでは当該事象は発生しませんでした。
大変失礼いたしました。

後日、事象が発生する実行可能なコードを質問本文に記載します。
なお、被疑箇所の特定やそれを含めたコードの作成に少々お時間を頂きます。

[dodox86]

> なお、呼び出し元では以下の様に BSTR 型変数に = で戻り値を渡しており、
> CComBSTR クラスの = 演算子のオーバーロードは経由していないと考えています。

こちらでも別途確認しました。その使い方ですと=演算子のオーバーロードではなくてBSTRへのキャストになりますね。ですので、左辺値がCComBSTRクラスへの=演算子オーバーロードは経由していないというお考えは合っています。当方の見誤りと確認不足で混乱させてしまった点があれば大変失礼しました。
ですが、test()関数がreturn CComSTR(L"hoge"); で返すコード（＜このようなコードの書き方はしたことが無かったので、見誤りました）になっていることと、その後段の質問者さんのコードで、結果的に不適切な使い方になっていると推察します。subkaさんの回答への[2021/02/27 10:39]のコメントでradianさんが指摘されていますが、私も同意見です。

Answer 1

事象が発生するコードの検証中に原因が分かった為、こちらに記載します。

###原因
他サイトから引用した以下の様なコードが原因でした。
Qiita 等でも記事にされているメジャーな使い方の様です。

BSTR str = CComBSTR(L"ABC");

###原因詳細
上述のコードの処理は以下の通りとなります。
①CComBSTR クラスのコンストラクタが呼び出され、L"ABC"のメモリが確保される。
②str に L"ABC" の参照が渡される。
③CComBSTR クラスのデストラクタが呼び出され、L"ABC"のメモリが解放される。

つきまして、str は見掛け上 L"ABC" が確保されたアドレスを参照していますが、
COM 内部処理的には L"ABC" のアドレスが解放された状態となります。

質問本文では CComBSTR の呼び出しが一度のみのコードを記載しておりましたが、
該当のコードの引数である var の値を設定する際に CComBSTR による値設定を行っていたことがそもそもの原因となります。

以下サンプルの様に、上記の処理を複数回実行すると同様の事象が発生します。

###サンプル

int main() {
	BSTR abc = SysAllocString(L"ABC");
	SysFreeString(abc);
	BSTR def = SysAllocString(L"DEF");
	SysFreeString(def);
}

3行目（SysFreeString(abc);）の時点で abc は解放されていますが、
4行目でも abc の参照先は変わらず、参照先には L"ABC" が残っています。

これは COM 内部のメモリ管理をしている機能に起因しています。
COM API が確保するメモリはスタックでもヒープでもなく、COM 管理下のメモリです。
COM が自管理下にあるメモリを参照しているか否かの判断は、実際に参照されているかではなく、
COM 内部のメモリ管理をする機能が参照しているか否かで判断されます。
SysFreeString は、この機能上での参照を解除します。

そして5行目で再度 L"DEF" を確保する際、確保する先は先程解放した L"ABC" が格納されているアドレスです。
つまり、この時点でまだ L"ABC" のアドレスを参照している abc の値も変わります。

ちなみに以下でも同様です。

int main() {
	BSTR abc = CComBSTR(L"ABC");
	BSTR def = CComBSTR(L"DEF");
}

###その他
同関数内のスコープの異なる位置でメモリ確保をしたり、質問本文の様にルーチンを分離して実行してみたり、BSTR と CComBSTR を混在させたり、などなど検証している内に事象が発生しないパターンが存在することもわかりました。

検証が雑だったせいかもしれませんが、
時により二度目のメモリ確保の際に、一度目に解放したメモリ以外を確保するパターンが確認できました。

問題となっている事象の原因が既に分かっていたのと、条件がきれいにまとめられなくなりそうだったので細かい検証は見送りましたが、タイミングや環境如何では不都合なく動く可能性もある様です。
###解決策
CComBSTR クラスのコンストラクタを利用した BSTR および CComBSTR の初期化をするべきでない。
事前に CComBSTR クラスを用意しているのであれば CComBSTR::Copy を利用する。
そうでなければ SysAllocString を利用して、値の受け取り側がラッパークラスでなければ各要素で SysFreeString を明示する。
###宜しければご教示下さい
素人の検証・見解ですので、内容に相違がある場合はご指摘をお願いします。

Comments

[退会済みユーザー]

> CComBSTR クラスのコンストラクタを利用した BSTR および CComBSTR の初期化をするべきでない。
CComBSTR のコンストラクタでの初期化は問題ないと思います。

そもそも、
BSTR str = CComBSTR(L"ABC");
このコード自体が危険な匂いがするのですが。CComBSTRのインスタンスを保持していなければ、CComBSTRのデストラクタが走った時点で、strは無効なメモリを参照する事になるのでは。

[Bull]

> Qiita 等でも記事にされているメジャーな使い方の様です。
> BSTR str = CComBSTR(L"ABC");
これは本当のことでしょうか。でしたらちょっと怖いです。
すでに指摘されているように、この使い方は間違っているんじゃないかと。
参考 : https://makandat.wordpress.com/2018/02/25/c-%E3%81%A7%E4%B8%80%E6%99%82%E3%82%AA%E3%83%96%E3%82%B8%E3%82%A7%E3%82%AF%E3%83%88%E3%81%AE%E5%AF%BF%E5%91%BD%E3%81%AF%E8%B6%85%E7%9F%AD%E5%91%BD%EF%BC%81/

BSTR str = CComBSTR(L"ABC").Copy();
なら問題ないですけど。

BSTR と CComBSTR、VARIANT と CComVariant などを混ぜて使う時は、細心の注意が必要になります。
以前、VARIANT から CComVariant にコピーして、メモリーリークしていた事例をみたことがあります。

[Bull]

Qiita の記事って、https://qiita.com/tadnakam/items/d4d01850490a09af590c でしょうか？
確かに
BSTR bstr1 = CComBSTR(L"Hello");
と書いてありますが、その後のサンプルプログラムでは、
BSTR bstr1 = CComBSTR(L"Hello").Copy();
になっていますね。

[退会済みユーザー]

Qiitaのコメントでもかなり問題点が指摘されてますね。思った事がほぼ書いてありました。

[Bull]

BSTR str = CComBSTR(L"ABC").Copy();
でも、いろいろと問題はありますね。
やはり、CComBSTR のまま使うのがいいんでしょうね。
とはいえ、BSTR を必要とするケースもあるわけで、悩ましい問題です。

[subka]

radian 様、Bull 様、コメントありがとうございます。

CComBSTR::Copy は良くない様ですね。
元からインスタンスを保持していている場合はそれでも良いのではとも思いますが、
BSTR 型変数の初期化のために一時変数として利用する場合は無駄が多いわけですね。

一時変数として利用するのであれば Qiita のコメントに記載されていた CComBSTR::Detach が最適でしょうか。
BSTR 型の値を return して、CComBSTR が保持している m_str を NULL にする様です。

事象の原因としては概ね見解に相違ない様ですので、
こちらの自己回答を以って解決としたいと思います。

ご協力頂いた皆様、本当にありがとうございました。

[dodox86]

> 一時変数として利用するのであれば Qiita のコメントに記載されていた CComBSTR::Detach が最適でしょうか。

Attach/Detachメンバー関数はWindowsのATL/MFCで提供されるクラスにしばしば見られる名前のメンバー関数で、Windowsのプリミティブ(?)なオブジェクトを扱うクラスに実装されていることが多いです。(MFCのCWnd, CHandle等）既に存在するハンドル、ポインタ等をAttachし、そのクラスのインスタンスで操作できるようにします。Detachで切り離し、そのハンドルやポインタとクラスのインスタンスオブジェクトとの関係を断ちます。CComBSTRクラスのそれもおおむねそういった用途に使うはずです。subkaさんが最終的にどういった使い方をされるかは分からないので最適かどうかは私では判断付きませんが、例えばComBSTRのオブジェクトを生成してあれこれ操作し、DetachでBSTRなポインタを切り離し、そのポインタを後で使うようなことを想定されているとして、それらの生存（有効）期間、スコープをきちんと意識できるのであれば使えるのではないかな、と思います。幸い、Visual StudioのエディションによってはCComBSTRクラスの実装ソースファイルは確認できることですし。