Page MenuHomePhabricator

[analyzer] Disable constructor inlining when lifetime extension through fields occurs.
ClosedPublic

Authored by NoQ on Feb 23 2018, 11:21 AM.

Details

Summary

As i mentioned in D43497, automatic destructors are missing in the CFG in situations like

const int &x = C().x;

For now i'd rather disable construction inlining, because inlining constructors while doing nothing on destructors is very bad.

Diff Detail

Repository
rC Clang

Event Timeline

NoQ created this revision.Feb 23 2018, 11:21 AM
dcoughlin accepted this revision.Feb 24 2018, 11:08 AM
dcoughlin added inline comments.
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
70

Would you be willing to add a tiny code example to the comment? I.e., const int &x = C().x

lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
177

I *think* this is safe. But it seems like it would be more direct to use skipRValueSubobjectAdjustments() and check for sub-object adjustments since ultimately that is what you care about, right?

This revision is now accepted and ready to land.Feb 24 2018, 11:08 AM
NoQ added inline comments.Feb 24 2018, 2:01 PM
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
177

skipRValueSubobjectAdjustments()

This function doesn't do anything anymore (since rC288563) (in most cases, at least). We now have lvalue sub-objects adjustments that are outside of MaterializeTemporaryExpr, and i'm trying to see if there are any by comparing the variable type with the temporary type. If there are still any rvalue sub-object adjustments present in the AST, then i'd have to skip them, as an additional step.

NoQ added inline comments.Feb 27 2018, 12:16 PM
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
70

Fixed in the commit.

This revision was automatically updated to reflect the committed changes.
chh added a subscriber: chh.Apr 18 2018, 10:24 AM

This change caused an assertion failure in ExprEngineCXX.cpp:
https://bugs.llvm.org/show_bug.cgi?id=37166