Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CUDA] Fix a misuse of std::move: address of stack memory associated with temporary object of type std::lock_guard<std::mutex> returned to caller #3502

Merged
merged 2 commits into from Nov 15, 2021

Conversation

PragmaTwice
Copy link
Contributor

@PragmaTwice PragmaTwice commented Nov 14, 2021

Related issue = #

In the original code, the returned rvalue reference is obviously a dangling reference, and the scoped mutex has no effect (destructed immediately after construction).

…associated with temporary object of type 'std::lock_guard<std::mutex>' returned to caller
@CLAassistant
Copy link

CLAassistant commented Nov 14, 2021

CLA assistant check
All committers have signed the CLA.

@netlify
Copy link

netlify bot commented Nov 14, 2021

✔️ Deploy Preview for jovial-fermat-aa59dc canceled.

🔨 Explore the source changes: 9ed343d

🔍 Inspect the deploy log: https://app.netlify.com/sites/jovial-fermat-aa59dc/deploys/6190a915f97b5e000721c295

@PragmaTwice PragmaTwice changed the title [CUDA] Fix an obvious misusage of std::move: address of stack memory associated with temporary object of type 'std::lock_guard<std::mutex>' returned to caller [CUDA] Fix an obvious misusage of std::move: address of stack memory associated with temporary object of type std::lock_guard<std::mutex> returned to caller Nov 14, 2021
@PragmaTwice PragmaTwice changed the title [CUDA] Fix an obvious misusage of std::move: address of stack memory associated with temporary object of type std::lock_guard<std::mutex> returned to caller [CUDA] Fix an obvious misusage of std::move: address of stack memory associated with temporary object of type std::lock_guard<std::mutex> returned to caller Nov 14, 2021
@PragmaTwice PragmaTwice changed the title [CUDA] Fix an obvious misusage of std::move: address of stack memory associated with temporary object of type std::lock_guard<std::mutex> returned to caller [CUDA] Fix a misuse of std::move: address of stack memory associated with temporary object of type std::lock_guard<std::mutex> returned to caller Nov 14, 2021
Copy link
Member

@k-ye k-ye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM && welcome to the community!

@k-ye k-ye requested a review from sjwsl November 14, 2021 11:09
@PragmaTwice
Copy link
Contributor Author

PragmaTwice commented Nov 14, 2021

Some fact to help understand it:

  • std::move is implicit and mandatory while a local variable is returned (in the standard, the overload resolution will select a move ctor T(T&&) first, as treat the lvalue as an rvalue, since c++11, addressed in ISO/IEC 14882:2011 12.8/32)
  • RVO (but not NRVO) is a mandatory optimization since c++17, so even move ctor will not be called here. Using std::move here will prevent RVO. (of course, the return type can never be a reference type while you want to return a local variable (with auto storage duration))

Copy link
Contributor

@ailzhang ailzhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! Thank you!

@ailzhang ailzhang merged commit d4b4818 into taichi-dev:master Nov 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants