Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> {
void VisitUnaryOperator(const UnaryOperator *UO);
void VisitReturnStmt(const ReturnStmt *RS);
void VisitBinaryOperator(const BinaryOperator *BO);
void VisitConditionalOperator(const ConditionalOperator *CO);
void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE);
void VisitCXXFunctionalCastExpr(const CXXFunctionalCastExpr *FCE);
void VisitInitListExpr(const InitListExpr *ILE);
Expand Down
9 changes: 9 additions & 0 deletions clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,15 @@ void FactsGenerator::VisitBinaryOperator(const BinaryOperator *BO) {
handleAssignment(BO->getLHS(), BO->getRHS());
}

void FactsGenerator::VisitConditionalOperator(const ConditionalOperator *CO) {
if (hasOrigin(CO)) {
// Merge origins from both branches of the conditional operator.
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe comment why we only kill for the true branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we kill to clear the initial state and merge (flow) both origins into it.

// We kill to clear the initial state and merge both origins into it.
killAndFlowOrigin(*CO, *CO->getTrueExpr());
flowOrigin(*CO, *CO->getFalseExpr());
}
}

void FactsGenerator::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) {
// Assignment operators have special "kill-then-propagate" semantics
// and are handled separately.
Expand Down
17 changes: 17 additions & 0 deletions clang/test/Sema/warn-lifetime-safety-dataflow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -414,3 +414,20 @@ void test_use_lifetimebound_call() {
// CHECK: Expire ([[L_Y]] (Path: y))
// CHECK: Expire ([[L_X]] (Path: x))
}
// CHECK-LABEL: Function: test_conditional_operator
void test_conditional_operator(bool cond) {
MyObj x, y;
MyObj *p = cond ? &x : &y;
// CHECK: Block B{{[0-9]+}}:
// CHECK: Issue ([[L_X:[0-9]+]] (Path: x), ToOrigin: [[O_DRE_X:[0-9]+]] (Expr: DeclRefExpr))
// CHECK: OriginFlow (Dest: [[O_ADDR_X:[0-9]+]] (Expr: UnaryOperator), Src: [[O_DRE_X]] (Expr: DeclRefExpr))
// CHECK: Block B{{[0-9]+}}:
// CHECK: Issue ([[L_Y:[0-9]+]] (Path: y), ToOrigin: [[O_DRE_Y:[0-9]+]] (Expr: DeclRefExpr))
// CHECK: OriginFlow (Dest: [[O_ADDR_Y:[0-9]+]] (Expr: UnaryOperator), Src: [[O_DRE_Y]] (Expr: DeclRefExpr))
// CHECK: Block B{{[0-9]+}}:
// CHECK: OriginFlow (Dest: [[O_COND_OP:[0-9]+]] (Expr: ConditionalOperator), Src: [[O_ADDR_X]] (Expr: UnaryOperator))
// CHECK: OriginFlow (Dest: [[O_COND_OP]] (Expr: ConditionalOperator), Src: [[O_ADDR_Y]] (Expr: UnaryOperator), Merge)
// CHECK: OriginFlow (Dest: [[O_P:[0-9]+]] (Decl: p), Src: [[O_COND_OP]] (Expr: ConditionalOperator))
// CHECK: Expire ([[L_Y]] (Path: y))
// CHECK: Expire ([[L_X]] (Path: x))
}
73 changes: 73 additions & 0 deletions clang/test/Sema/warn-lifetime-safety.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ void no_error_loan_from_current_iteration(bool cond) {
//===----------------------------------------------------------------------===//

View Identity(View v [[clang::lifetimebound]]);
MyObj* Identity(MyObj* v [[clang::lifetimebound]]);
View Choose(bool cond, View a [[clang::lifetimebound]], View b [[clang::lifetimebound]]);
MyObj* GetPointer(const MyObj& obj [[clang::lifetimebound]]);

Expand Down Expand Up @@ -582,3 +583,75 @@ void lifetimebound_ctor() {
}
(void)v;
}

// Conditional operator.
void conditional_operator_one_unsafe_branch(bool cond) {
MyObj safe;
MyObj* p = &safe;
{
MyObj temp;
p = cond ? &temp // expected-warning {{object whose reference is captured may not live long enough}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this trigger in all modes or only pessimitic/conservative? I vaguely remember discussing this but can't remember where we landed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This triggers in only strict mode. Confidence is driven by liveness now and p is not live on all paths from here so this is maybe case.

: &safe;
} // expected-note {{destroyed here}}

// This is not a use-after-free for any value of `cond` but the analysis
// cannot reason this and marks the above as a false positive. This
// ensures safety regardless of cond's value.
if (cond)
p = &safe;
(void)*p; // expected-note {{later used here}}
}

void conditional_operator_two_unsafe_branches(bool cond) {
MyObj* p;
{
MyObj a, b;
p = cond ? &a // expected-warning {{object whose reference is captured does not live long enough}}
: &b; // expected-warning {{object whose reference is captured does not live long enough}}
} // expected-note 2 {{destroyed here}}
(void)*p; // expected-note 2 {{later used here}}
}

void conditional_operator_nested(bool cond) {
MyObj* p;
{
MyObj a, b, c, d;
p = cond ? cond ? &a // expected-warning {{object whose reference is captured does not live long enough}}.
: &b // expected-warning {{object whose reference is captured does not live long enough}}.
: cond ? &c // expected-warning {{object whose reference is captured does not live long enough}}.
: &d; // expected-warning {{object whose reference is captured does not live long enough}}.
} // expected-note 4 {{destroyed here}}
(void)*p; // expected-note 4 {{later used here}}
}

void conditional_operator_lifetimebound(bool cond) {
MyObj* p;
{
MyObj a, b;
p = Identity(cond ? &a // expected-warning {{object whose reference is captured does not live long enough}}
: &b); // expected-warning {{object whose reference is captured does not live long enough}}
} // expected-note 2 {{destroyed here}}
(void)*p; // expected-note 2 {{later used here}}
}

void conditional_operator_lifetimebound_nested(bool cond) {
MyObj* p;
{
MyObj a, b;
p = Identity(cond ? Identity(&a) // expected-warning {{object whose reference is captured does not live long enough}}
: Identity(&b)); // expected-warning {{object whose reference is captured does not live long enough}}
} // expected-note 2 {{destroyed here}}
(void)*p; // expected-note 2 {{later used here}}
}

void conditional_operator_lifetimebound_nested_deep(bool cond) {
MyObj* p;
{
MyObj a, b, c, d;
p = Identity(cond ? Identity(cond ? &a // expected-warning {{object whose reference is captured does not live long enough}}
: &b) // expected-warning {{object whose reference is captured does not live long enough}}
: Identity(cond ? &c // expected-warning {{object whose reference is captured does not live long enough}}
: &d)); // expected-warning {{object whose reference is captured does not live long enough}}
} // expected-note 4 {{destroyed here}}
(void)*p; // expected-note 4 {{later used here}}
}
3 changes: 1 addition & 2 deletions clang/unittests/Analysis/LifetimeSafetyTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,6 @@ TEST_F(LifetimeAnalysisTest, GslPointerConstructFromView) {
EXPECT_THAT(Origin("q"), HasLoansTo({"a"}, "p1"));
}

// FIXME: Handle loans in ternary operator!
TEST_F(LifetimeAnalysisTest, GslPointerInConditionalOperator) {
SetupTest(R"(
void target(bool cond) {
Expand All @@ -698,7 +697,7 @@ TEST_F(LifetimeAnalysisTest, GslPointerInConditionalOperator) {
POINT(p1);
}
)");
EXPECT_THAT(Origin("v"), HasLoansTo({}, "p1"));
EXPECT_THAT(Origin("v"), HasLoansTo({"a", "b"}, "p1"));
}

// FIXME: Handle temporaries.
Expand Down
Loading