Add LIFETIMEBOUND to CScript where needed#22278
Conversation
|
Interesting, I wouldn't have considered using utACK fabcaf39cd1516923c818b382ee3478b340f3db7. My understanding is that this cannot introduce bugs if it compiles with issues, as the attribute only adds warning/errors. |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
|
@theuni want to take a look here? |
|
utACK |
|
Concept ACK/utACK. I guess ideally we'd annotate anywhere we return references to I tried switching the return type from The one it turned up is |
Correct. I think where
Thanks, added a commit. Can be tested with this snippet: |
9e429f0 to
fa7e6c5
Compare
|
utACK fa7e6c5. |
jonatack
left a comment
There was a problem hiding this comment.
Light ACK fa7e6c5 debug build with clang 13, reproduced the example compiler warning in the pull description, and briefly looked at clang::lifetimebound support in earlier versions of clang; it is in clang 7 (https://releases.llvm.org/7.0.0/tools/clang/docs/AttributeReference.html#lifetimebound-clang-lifetimebound), did not see references to it in earlier docs
fa7e6c5 Add LIFETIMEBOUND to InitializeChainstate (MarcoFalke) fa5c896 Add LIFETIMEBOUND to CScript where needed (MarcoFalke) Pull request description: Without this, stack-use-after-scope can only be detected at runtime with ASan or code review, both of which are expensive. Use `LIFETIMEBOUND` to turn this error into a compile warning. See https://releases.llvm.org/12.0.0/tools/clang/docs/AttributeReference.html#lifetimebound Example: ```cpp const CScript a{WITH_LOCK(::cs_main, return CScript{} << OP_0 << OP_1)}; ``` Before: (no warning) After: ``` warning: returning reference to local temporary object [-Wreturn-stack-address] const CScript a{WITH_LOCK(::cs_main, return CScript{} << OP_0 << OP_1)}; ^~~~~~~~~ ./sync.h:276:65: note: expanded from macro 'WITH_LOCK' #define WITH_LOCK(cs, code) [&]() -> decltype(auto) { LOCK(cs); code; }() ^~~~ ACKs for top commit: theuni: utACK fa7e6c5. jonatack: Light ACK fa7e6c5 debug build with clang 13, reproduced the example compiler warning in the pull description, and briefly looked at `clang::lifetimebound` support in earlier versions of clang; it is in clang 7 (https://releases.llvm.org/7.0.0/tools/clang/docs/AttributeReference.html#lifetimebound-clang-lifetimebound), did not see references to it in earlier docs Tree-SHA512: e915acdc4532445205b7703fab61a5d682231ace78ecfb274cb8523ca2bddefd85828f50ac047cfb1afaff92a331f5f7b5a1472539f999e30f7cf8ac8c3222f3
…)::start_time 4cb9d21 blockstorage: add LIFETIMEBOUND to GetFirstStoredBlock()::start_time (Jon Atack) Pull request description: Suggested in #25016 (comment), the lifetimebound attribute here indicates that a resource owned by the `start_block` param of `CBlockIndex* BlockManager::GetFirstStoredBlock()` can be retained by the method's return value, which enables detecting the use of out-of-scope stack memory (ASan `stack-use-after-scope`) at compile time. See https://releases.llvm.org/12.0.0/tools/clang/docs/AttributeReference.html#lifetimebound and #22278 for related discussion, and #25040 for a similar example. ACKs for top commit: MarcoFalke: review ACK 4cb9d21 Tree-SHA512: a3f5ef83ebb6f08555d7c89f2437a682071b4ad77a7aa3326b6d2282c909bf9fcf4dac6bf05ee1d9931f2102cad4a02df5468bde1cf377d7126e84e8541604dc
…dBlock()::start_time 4cb9d21 blockstorage: add LIFETIMEBOUND to GetFirstStoredBlock()::start_time (Jon Atack) Pull request description: Suggested in bitcoin#25016 (comment), the lifetimebound attribute here indicates that a resource owned by the `start_block` param of `CBlockIndex* BlockManager::GetFirstStoredBlock()` can be retained by the method's return value, which enables detecting the use of out-of-scope stack memory (ASan `stack-use-after-scope`) at compile time. See https://releases.llvm.org/12.0.0/tools/clang/docs/AttributeReference.html#lifetimebound and bitcoin#22278 for related discussion, and bitcoin#25040 for a similar example. ACKs for top commit: MarcoFalke: review ACK 4cb9d21 Tree-SHA512: a3f5ef83ebb6f08555d7c89f2437a682071b4ad77a7aa3326b6d2282c909bf9fcf4dac6bf05ee1d9931f2102cad4a02df5468bde1cf377d7126e84e8541604dc
Without this, stack-use-after-scope can only be detected at runtime with ASan or code review, both of which are expensive.
Use
LIFETIMEBOUNDto turn this error into a compile warning.See https://releases.llvm.org/12.0.0/tools/clang/docs/AttributeReference.html#lifetimebound
Example:
Before: (no warning)
After: