C++ スコープ外になったアドレスの有効性について

Question

下記のように一度変数を保持させたあとにスコープ外にいった場合
x.xの値を参照するのは危険な行為になるのでしょうか？

class X
{
public:
    int* x;
};

using std::cout;

int main()
{
    X x;
    {
        int n = 111;
        x.x = &n;

        cout << x.x << "\n";
    }

    cout << x.x << "\n";

    getchar();
}

Answer 1

int n = 111; ラインは、変数を一時的なスタックにアサインします。この変数は、仮想メモリ上のアドレスを持ち、ポインターの値として、x.xに保存されます。一方、このスタックは　{で始まるラインから、｝で終わるラインまでが有効で、ライン19ではもう存在しません。そのため、ライン１９は意味を持たないポインターの値をプリントすることになってしまいます。コンパイラーは間違った使い方をしていると警告を出すのでしょう。

Answer 2

危険だと思いますが、実際のところ、どうなるか、コンパイラ次第と思いますが、
スタックメモリ上のローカル変数ですが、関数内に新たなローカル変数があるからといって、新たにスタックメモリを確保するのではなく、関数の頭で一括で確保し関数を抜けるところで一括で返します。
よって実際のところ、スコープを抜けたところでも関数を抜けるまでは、そのエリアは確保されていますが仕様ではありませんので保障はありません。で、一応動く。
コンパイラ次第なんて危なくて使う気しないと思います。一度大丈夫でも最適化などで変わってしまう可能性もある。

Answer 3

Cppcheck - A tool for static C/C++ code analysis という、静的解析(static analysis)を行うツールがあります。これで質問文にあるソースコードを解析してみると、以下の様なエラー(invalidLifetime)が報告されます。

$ cppcheck invalid_life_time.cc

invalid_life_time.cc:19:11: error: Using object that points to local variable 'n' that is out of scope. [invalidLifetime]
  cout << x.x << "\n";
          ^
invalid_life_time.cc:16:11: note: Address of variable taken here.
    x.x = &n;
          ^
invalid_life_time.cc:15:9: note: Variable created here.
    int n = 111;
        ^

また、スコープ外から参照する対象をポインタアドレスではなく実体の値に変更してから gcc の Address Sanitizer でチェックすると、stack-use-after-scope | Microsoft Learnというエラーが検出されることが判ります。

  cout << *x.x << "\n";
  //cout << x.x << "\n";
  getchar();

$ g++ --version
g++ (Ubuntu 15.2.0-4ubuntu4) 15.2.0

$ g++ -fsanitize=address -Wall -Wextra -g invalid_life_time.cc -o invalid_life_time && ../invalid_life_time

==569735==ERROR: AddressSanitizer: stack-use-after-scope on address 0x72b421200030 at pc 0x5770c8ae03f8 bp 0x7fffb42ba030 sp 0x7fffb42ba020
READ of size 4 at 0x72b421200030 thread T0
    #0 0x5770c8ae03f7 in main invalid_life_time.cc:19
    #1 0x76b42342a574 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #2 0x76b42342a627 in __libc_start_main_impl ../csu/libc-start.c:360
    #3 0x5770c8ae01e4 in _start (invalid_life_time+0x11e4) (BuildId: 282ae10eecaab9190cb0f4ec5c1d7b5458b25ac3)

Address 0x72b421200030 is located in stack of thread T0 at offset 48 in frame
    #0 0x5770c8ae02b8 in main invalid_life_time.cc:12

  This frame has 2 object(s):
    [48, 52) 'n' (line 15) <== Memory access at offset 48 is inside this variable
    [64, 72) 'x' (line 13)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-use-after-scope invalid_life_time.cc:19 in main

Comments

[apa]

返信ありがとうございます。
そちらのツールは存じ上げていないのですが
エラーが出たということは構文的に正しくないという認識ですかね？

[melian]

構文的には問題ないのですけれども、潜在的なリスク(stack-use-after-scope)が存在します。回答に示した Microsoft Learn の記事の "Example 4 - temporaries" を参照してみてください。

Answer 4

寿命を終えたオブジェクトを指していたポインタは無効 (invalid) であり、無効なポインタを使用した効果は未定義です。 たとえポインタが指す先のオブジェクトにアクセスしなくてもポインタを使おうとすること自体が未定義の効果であると解されます。

Comments

[apa]

返信ありがとうございます。
かなり使い勝手が悪い印象ですね...
以前下記のようなコードを書いていましたが、
一時オブジェクトを登録する際のデータの受け渡しが非常に難解な気がします
この場合だと一時オブジェクトのdataがhit()処理後に未定義になるから
registで登録されているけど未定義ってことですもんね

struct damagedata{
actor* attacker;
actor* deffencer;
float damage;
}

class dmagemanager
{
void regist(damagedeta& data) {Data.Add(data)}
TArray<damagedata> Data;
}

void Sphere::hit()
{
damagedata data;
data.attacker = attacker;
data.deffencer = deffencer;
data.damage = 999;

damagemanager* mgr = getter();
mgr->regist(data);
}

[Manabu]

始めから値渡しでは駄目なのでしょうか

void regist(damagedeta data) {Data.Add(data)};
TArray<damagedata> Data;

void Sphere::hit()
{
damagemanager* mgr = getter();

mgr->regist(
damagedata{attacker,defencer,999}
);
};

安全性が担保される範囲でなら始めからポインタでもいいでしょう

void Sphere::hit()
{
struct damagedata *data=new damagedata;
data->attacker = attacker;
data->deffencer = deffencer;
data->damage = 999;
damagemanager* mgr = getter();
mgr->regist(*data);
}

スコープ外に移るということはスタックを解放して呼び出し元のアドレスに戻るということなのでdataに対する参照がダングリングと化します
スタックなので解放された領域には新しい関数のローカル変数などがセットされます
この状態のdataは不定値と変わらないので大変危険です

[apa]

mgr->regist(
damagedata{attacker,defencer,999}
);
};

上記でも問題なさそうですね
強いて言えば渡したいパラメータが増えるほど横にながくなってしまうくらいでしょう？

またポインタでnewしておくのも一つの解決案ですね

業務で普通に下記のような書き方していたので気を付けないとですね...
damagedata data;
data.attacker = attacker;
data.deffencer = deffencer;
data.damage = 999;

damagemanager* mgr = getter();
mgr->regist(data);

Answer 5

xは初期化しておいたほうがよいでしょう。

X x = {};

n自体はスコープを外れると参照できなくなりますが、その111が入っているアドレスはx.xに代入されるので、問題無いと思います。

Answer 6

たくさんのコメントありがとうございました。
コンパイルエラーや動作に問題がありませんでしたので気にしていませんでしたが
よろしくない書き方ということが分かりました。