Skip to content

Commit 4ab214c

Browse files
Andrew AyersCopilot
authored andcommitted
JIT: fix profile likelihood and overflow UB in optimizebools
Fix two issues in optOptimizeBoolsUpdateTrees and GetIntersection: 1. The !sameTarget branch likelihood formula was computing (1-p1) + p1*p2_false = 1 - p1*p2_true, but the correct probability of reaching B2's true target is (1-p1) * p2_true (B1 falls through, then B2 jumps). Also fixed the misleading comment. 2. The normalize lambda in GetIntersection could overflow when cns == SSIZE_T_MAX and cmp == GT_GT (signed overflow is UB in C++). Added an explicit guard and made the lambda return bool to signal failure. 3. Updated a stale comment referencing the nonexistent GTF_BOOLEAN flag in optIsBoolComp. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d751cbe commit 4ab214c

1 file changed

Lines changed: 16 additions & 9 deletions

File tree

src/coreclr/jit/optimizebools.cpp

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -448,23 +448,30 @@ static bool GetIntersection(var_types type,
448448
}
449449

450450
// Convert to a canonical form with GT_GE or GT_LE (inclusive).
451-
auto normalize = [](genTreeOps* cmp, ssize_t* cns) {
451+
auto normalize = [](genTreeOps* cmp, ssize_t* cns) -> bool {
452452
if (*cmp == GT_GT)
453453
{
454454
// "X > cns" -> "X >= cns + 1"
455+
if (*cns == SSIZE_T_MAX)
456+
{
457+
return false;
458+
}
455459
*cns = *cns + 1;
456460
*cmp = GT_GE;
457461
}
458462
if (*cmp == GT_LT)
459463
{
460464
// "X < cns" -> "X <= cns - 1"
465+
// cns is non-negative (checked above), so cns - 1 >= -1 and cannot underflow.
461466
*cns = *cns - 1;
462467
*cmp = GT_LE;
463468
}
464-
// whether these overflow or not is checked below.
469+
return true;
465470
};
466-
normalize(&cmp1, &cns1);
467-
normalize(&cmp2, &cns2);
471+
if (!normalize(&cmp1, &cns1) || !normalize(&cmp2, &cns2))
472+
{
473+
return false;
474+
}
468475

469476
if (cmp1 == cmp2)
470477
{
@@ -1234,8 +1241,8 @@ void OptBoolsDsc::optOptimizeBoolsUpdateTrees()
12341241
}
12351242
else
12361243
{
1237-
// We originally reached B2's true target via
1238-
// B1 false OR B1 true B2 false.
1244+
// We originally reached B2's true target only via
1245+
// B1 false, then B2 true.
12391246
//
12401247
// We will now reach via B1 true.
12411248
// Modify flow for true side of B1
@@ -1244,7 +1251,7 @@ void OptBoolsDsc::optOptimizeBoolsUpdateTrees()
12441251
origB1TrueEdge->setHeuristicBased(origB2TrueEdge->isHeuristicBased());
12451252

12461253
newB1TrueLikelihood =
1247-
(1.0 - origB1TrueLikelihood) + origB1TrueLikelihood * origB2FalseEdge->getLikelihood();
1254+
(1.0 - origB1TrueLikelihood) * origB2TrueEdge->getLikelihood();
12481255
}
12491256

12501257
// Fix B1 true edge likelihood
@@ -1412,9 +1419,9 @@ GenTree* OptBoolsDsc::optIsBoolComp(OptTestInfo* pOptTest)
14121419
ssize_t ival2 = opr2->AsIntCon()->gtIconVal;
14131420

14141421
// Is the value a boolean?
1415-
// We can either have a boolean expression (marked GTF_BOOLEAN) or a constant 0/1.
1422+
// Currently we only recognize constant 0/1.
14161423

1417-
if (opr1->OperIs(GT_CNS_INT) && (opr1->IsIntegralConst(0) || opr1->IsIntegralConst(1)))
1424+
if (opr1->IsIntegralConst(0) || opr1->IsIntegralConst(1))
14181425
{
14191426
pOptTest->isBool = true;
14201427
}

0 commit comments

Comments
 (0)