[clang][dataflow] Handle more glvalue cases of the ConditionalOperator transfer #168994
+153
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…r transfer In the dataflow framework, the builtin transfer function currently only handles the GLValue result case of ConditionalOperator when the true and false expression StorageLocations are exactly the same. Ideally / we have wanted to introduce alias sets to handle when the Locs are different. However, that is a larger change to the framework (and we may need to introduce weak updates). For now, do something simpler to at least handle when the GLValue is immediately cast to an RValue, by making up a distinct StorageLocation that holds the join of the true and false expression values (when not a record). This seems like the most common case, so seems worth covering. The case when an LValue is needed and can be updated later (and thus needs a link to the original storage locations) seems more rare, and we currently do not handle such updates either, so this intermediate step is no different (for that case).
🐧 Linux x64 Test Results
|
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang Author: Jan Voung (jvoung) ChangesIn the dataflow framework, the builtin transfer function currently only Ideally / we have wanted to introduce alias sets to handle when the Locs For now, do something simpler to at least handle when the GLValue is Full diff: https://github.com/llvm/llvm-project/pull/168994.diff 2 Files Affected:
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 06f12784aa82d..28c7fe3a5e1bc 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -769,8 +769,29 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
StorageLocation *TrueLoc = TrueEnv->getStorageLocation(*S->getTrueExpr());
StorageLocation *FalseLoc =
FalseEnv->getStorageLocation(*S->getFalseExpr());
- if (TrueLoc == FalseLoc && TrueLoc != nullptr)
+ if (TrueLoc == FalseLoc && TrueLoc != nullptr) {
Env.setStorageLocation(*S, *TrueLoc);
+ } else if (!S->getType()->isRecordType()) {
+ // Ideally, we would have something like an "alias set" to say that the
+ // result StorageLocation can be either of the locations from the
+ // TrueEnv or FalseEnv. Then, when this ConditionalOperator is
+ // (a) used in an LValueToRValue cast, the value is the join of all of
+ // the values in the alias set.
+ // (b) or, used in an assignment to the resulting LValue, the assignment
+ // *may* update all of the locations in the alias set.
+ // For now, we do the simpler thing of creating a new StorageLocation
+ // and joining the values right away, handling only case (a).
+ // Otherwise, the dataflow framework needs to be updated be able to
+ // represent alias sets and weak updates (for the "may").
+ StorageLocation &Loc = Env.createStorageLocation(*S);
+ Env.setStorageLocation(*S, Loc);
+ if (Value *Val = Environment::joinValues(
+ S->getType(), TrueEnv->getValue(*S->getTrueExpr()), *TrueEnv,
+ FalseEnv->getValue(*S->getFalseExpr()), *FalseEnv, Env,
+ Model)) {
+ Env.setValue(Loc, *Val);
+ }
+ }
} else if (!S->getType()->isRecordType()) {
// The conditional operator can evaluate to either of the values of the
// two branches. To model this, join these two values together to yield
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 66b3bba594fc9..14ac7ec4e39fe 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -17,6 +17,7 @@
#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/Formula.h"
#include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
#include "clang/Analysis/FlowSensitive/NoopLattice.h"
#include "clang/Analysis/FlowSensitive/RecordOps.h"
@@ -4382,6 +4383,40 @@ TEST(TransferTest, VarDeclInitAssignConditionalOperator) {
});
}
+TEST(TransferTest, VarDeclInitReferenceAssignConditionalOperator) {
+ std::string Code = R"(
+ struct A {
+ int i;
+ };
+
+ void target(A Foo, A Bar, bool Cond) {
+ A &Baz = Cond ? Foo : Bar;
+ // Make sure A::i is modeled.
+ Baz.i;
+ /*[[p]]*/
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+ auto *FooIVal = cast<IntegerValue>(getFieldValue(
+ &getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "Foo"), "i",
+ ASTCtx, Env));
+ auto *BarIVal = cast<IntegerValue>(getFieldValue(
+ &getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "Bar"), "i",
+ ASTCtx, Env));
+ auto *BazIVal = cast<IntegerValue>(getFieldValue(
+ &getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "Baz"), "i",
+ ASTCtx, Env));
+
+ EXPECT_NE(BazIVal, FooIVal);
+ EXPECT_NE(BazIVal, BarIVal);
+ });
+}
+
TEST(TransferTest, VarDeclInDoWhile) {
std::string Code = R"(
void target(int *Foo) {
@@ -6177,6 +6212,102 @@ TEST(TransferTest, ConditionalOperatorLocation) {
});
}
+TEST(TransferTest, ConditionalOperatorValuesTested) {
+ // Even though the LHS and the RHS of the conditional operator have different
+ // StorageLocations, we get constraints from the condition that the values
+ // must be equal to B1 for JoinDifferentIsB1, or B2 for JoinDifferentIsB2.
+ std::string Code = R"(
+ void target(bool B1, bool B2) {
+ bool JoinDifferentIsB1 = (B1 == B2) ? B2 : B1;
+ bool JoinDifferentIsB2 = (B1 == B2) ? B1 : B2;
+ // [[p]]
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ Environment Env = getEnvironmentAtAnnotation(Results, "p").fork();
+
+ auto &B1 = getValueForDecl<BoolValue>(ASTCtx, Env, "B1");
+ auto &B2 = getValueForDecl<BoolValue>(ASTCtx, Env, "B2");
+ auto &JoinDifferentIsB1 =
+ getValueForDecl<BoolValue>(ASTCtx, Env, "JoinDifferentIsB1");
+ auto &JoinDifferentIsB2 =
+ getValueForDecl<BoolValue>(ASTCtx, Env, "JoinDifferentIsB2");
+
+ const Formula &JoinDifferentIsB1EqB1 =
+ Env.arena().makeEquals(JoinDifferentIsB1.formula(), B1.formula());
+ EXPECT_TRUE(Env.allows(JoinDifferentIsB1EqB1));
+ EXPECT_TRUE(Env.proves(JoinDifferentIsB1EqB1));
+
+ const Formula &JoinDifferentIsB2EqB2 =
+ Env.arena().makeEquals(JoinDifferentIsB2.formula(), B2.formula());
+ EXPECT_TRUE(Env.allows(JoinDifferentIsB2EqB2));
+ EXPECT_TRUE(Env.proves(JoinDifferentIsB2EqB2));
+ });
+}
+
+TEST(TransferTest, ConditionalOperatorLocationUpdatedAfter) {
+ // We don't currently model a Conditional Operator with an LValue result
+ // as having aliases to the LHS and RHS (if it isn't just the same LValue
+ // on both sides). We also don't model that the update "may" happen
+ // (a weak update). So, we don't consider the LHS and RHS as being weakly
+ // updated at [[after_diff]].
+ std::string Code = R"(
+ void target(bool Cond, bool B1, bool B2) {
+ (void)0;
+ // [[before_same]]
+ (Cond ? B1 : B1) = !B1;
+ // [[after_same]]
+ (Cond ? B1 : B2) = !B1;
+ // [[after_diff]]
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ Environment BeforeSameEnv =
+ getEnvironmentAtAnnotation(Results, "before_same").fork();
+ Environment AfterSameEnv =
+ getEnvironmentAtAnnotation(Results, "after_same").fork();
+ Environment AfterDiffEnv =
+ getEnvironmentAtAnnotation(Results, "after_diff").fork();
+
+ auto &BeforeSameB1 =
+ getValueForDecl<BoolValue>(ASTCtx, BeforeSameEnv, "B1");
+ auto &AfterSameB1 =
+ getValueForDecl<BoolValue>(ASTCtx, AfterSameEnv, "B1");
+ auto &AfterDiffB1 =
+ getValueForDecl<BoolValue>(ASTCtx, AfterDiffEnv, "B1");
+
+ EXPECT_NE(&BeforeSameB1, &AfterSameB1);
+ EXPECT_NE(&BeforeSameB1, &AfterDiffB1);
+ // FIXME: The formula for AfterSameB1 should be different from
+ // AfterDiffB1 to reflect that B1 may be updated.
+ EXPECT_EQ(&AfterSameB1, &AfterDiffB1);
+
+ // The value of B1 is definitely different from before_same vs
+ // after_same.
+ const Formula &B1ChangedForSame =
+ AfterSameEnv.arena().makeNot(AfterSameEnv.arena().makeEquals(
+ AfterSameB1.formula(), BeforeSameB1.formula()));
+ EXPECT_TRUE(AfterSameEnv.allows(B1ChangedForSame));
+ EXPECT_TRUE(AfterSameEnv.proves(B1ChangedForSame));
+
+ // FIXME: It should be possible that B1 *may* be updated, so it may be
+ // that AfterSameB1 != AfterDiffB1 or AfterSameB1 == AfterDiffB1.
+ const Formula &B1ChangedForDiff =
+ AfterDiffEnv.arena().makeNot(AfterDiffEnv.arena().makeEquals(
+ AfterDiffB1.formula(), AfterSameB1.formula()));
+ EXPECT_FALSE(AfterSameEnv.allows(B1ChangedForDiff));
+ // proves() should be false, since B1 may or may not have changed
+ // depending on `Cond`.
+ EXPECT_FALSE(AfterSameEnv.proves(B1ChangedForDiff));
+ });
+}
+
TEST(TransferTest, ConditionalOperatorOnConstantExpr) {
// This is a regression test: We used to crash when a `ConstantExpr` was used
// in the branches of a conditional operator.
|
Member
|
@llvm/pr-subscribers-clang-analysis Author: Jan Voung (jvoung) ChangesIn the dataflow framework, the builtin transfer function currently only Ideally / we have wanted to introduce alias sets to handle when the Locs For now, do something simpler to at least handle when the GLValue is Full diff: https://github.com/llvm/llvm-project/pull/168994.diff 2 Files Affected:
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 06f12784aa82d..28c7fe3a5e1bc 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -769,8 +769,29 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
StorageLocation *TrueLoc = TrueEnv->getStorageLocation(*S->getTrueExpr());
StorageLocation *FalseLoc =
FalseEnv->getStorageLocation(*S->getFalseExpr());
- if (TrueLoc == FalseLoc && TrueLoc != nullptr)
+ if (TrueLoc == FalseLoc && TrueLoc != nullptr) {
Env.setStorageLocation(*S, *TrueLoc);
+ } else if (!S->getType()->isRecordType()) {
+ // Ideally, we would have something like an "alias set" to say that the
+ // result StorageLocation can be either of the locations from the
+ // TrueEnv or FalseEnv. Then, when this ConditionalOperator is
+ // (a) used in an LValueToRValue cast, the value is the join of all of
+ // the values in the alias set.
+ // (b) or, used in an assignment to the resulting LValue, the assignment
+ // *may* update all of the locations in the alias set.
+ // For now, we do the simpler thing of creating a new StorageLocation
+ // and joining the values right away, handling only case (a).
+ // Otherwise, the dataflow framework needs to be updated be able to
+ // represent alias sets and weak updates (for the "may").
+ StorageLocation &Loc = Env.createStorageLocation(*S);
+ Env.setStorageLocation(*S, Loc);
+ if (Value *Val = Environment::joinValues(
+ S->getType(), TrueEnv->getValue(*S->getTrueExpr()), *TrueEnv,
+ FalseEnv->getValue(*S->getFalseExpr()), *FalseEnv, Env,
+ Model)) {
+ Env.setValue(Loc, *Val);
+ }
+ }
} else if (!S->getType()->isRecordType()) {
// The conditional operator can evaluate to either of the values of the
// two branches. To model this, join these two values together to yield
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 66b3bba594fc9..14ac7ec4e39fe 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -17,6 +17,7 @@
#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/Formula.h"
#include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
#include "clang/Analysis/FlowSensitive/NoopLattice.h"
#include "clang/Analysis/FlowSensitive/RecordOps.h"
@@ -4382,6 +4383,40 @@ TEST(TransferTest, VarDeclInitAssignConditionalOperator) {
});
}
+TEST(TransferTest, VarDeclInitReferenceAssignConditionalOperator) {
+ std::string Code = R"(
+ struct A {
+ int i;
+ };
+
+ void target(A Foo, A Bar, bool Cond) {
+ A &Baz = Cond ? Foo : Bar;
+ // Make sure A::i is modeled.
+ Baz.i;
+ /*[[p]]*/
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+ auto *FooIVal = cast<IntegerValue>(getFieldValue(
+ &getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "Foo"), "i",
+ ASTCtx, Env));
+ auto *BarIVal = cast<IntegerValue>(getFieldValue(
+ &getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "Bar"), "i",
+ ASTCtx, Env));
+ auto *BazIVal = cast<IntegerValue>(getFieldValue(
+ &getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "Baz"), "i",
+ ASTCtx, Env));
+
+ EXPECT_NE(BazIVal, FooIVal);
+ EXPECT_NE(BazIVal, BarIVal);
+ });
+}
+
TEST(TransferTest, VarDeclInDoWhile) {
std::string Code = R"(
void target(int *Foo) {
@@ -6177,6 +6212,102 @@ TEST(TransferTest, ConditionalOperatorLocation) {
});
}
+TEST(TransferTest, ConditionalOperatorValuesTested) {
+ // Even though the LHS and the RHS of the conditional operator have different
+ // StorageLocations, we get constraints from the condition that the values
+ // must be equal to B1 for JoinDifferentIsB1, or B2 for JoinDifferentIsB2.
+ std::string Code = R"(
+ void target(bool B1, bool B2) {
+ bool JoinDifferentIsB1 = (B1 == B2) ? B2 : B1;
+ bool JoinDifferentIsB2 = (B1 == B2) ? B1 : B2;
+ // [[p]]
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ Environment Env = getEnvironmentAtAnnotation(Results, "p").fork();
+
+ auto &B1 = getValueForDecl<BoolValue>(ASTCtx, Env, "B1");
+ auto &B2 = getValueForDecl<BoolValue>(ASTCtx, Env, "B2");
+ auto &JoinDifferentIsB1 =
+ getValueForDecl<BoolValue>(ASTCtx, Env, "JoinDifferentIsB1");
+ auto &JoinDifferentIsB2 =
+ getValueForDecl<BoolValue>(ASTCtx, Env, "JoinDifferentIsB2");
+
+ const Formula &JoinDifferentIsB1EqB1 =
+ Env.arena().makeEquals(JoinDifferentIsB1.formula(), B1.formula());
+ EXPECT_TRUE(Env.allows(JoinDifferentIsB1EqB1));
+ EXPECT_TRUE(Env.proves(JoinDifferentIsB1EqB1));
+
+ const Formula &JoinDifferentIsB2EqB2 =
+ Env.arena().makeEquals(JoinDifferentIsB2.formula(), B2.formula());
+ EXPECT_TRUE(Env.allows(JoinDifferentIsB2EqB2));
+ EXPECT_TRUE(Env.proves(JoinDifferentIsB2EqB2));
+ });
+}
+
+TEST(TransferTest, ConditionalOperatorLocationUpdatedAfter) {
+ // We don't currently model a Conditional Operator with an LValue result
+ // as having aliases to the LHS and RHS (if it isn't just the same LValue
+ // on both sides). We also don't model that the update "may" happen
+ // (a weak update). So, we don't consider the LHS and RHS as being weakly
+ // updated at [[after_diff]].
+ std::string Code = R"(
+ void target(bool Cond, bool B1, bool B2) {
+ (void)0;
+ // [[before_same]]
+ (Cond ? B1 : B1) = !B1;
+ // [[after_same]]
+ (Cond ? B1 : B2) = !B1;
+ // [[after_diff]]
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ Environment BeforeSameEnv =
+ getEnvironmentAtAnnotation(Results, "before_same").fork();
+ Environment AfterSameEnv =
+ getEnvironmentAtAnnotation(Results, "after_same").fork();
+ Environment AfterDiffEnv =
+ getEnvironmentAtAnnotation(Results, "after_diff").fork();
+
+ auto &BeforeSameB1 =
+ getValueForDecl<BoolValue>(ASTCtx, BeforeSameEnv, "B1");
+ auto &AfterSameB1 =
+ getValueForDecl<BoolValue>(ASTCtx, AfterSameEnv, "B1");
+ auto &AfterDiffB1 =
+ getValueForDecl<BoolValue>(ASTCtx, AfterDiffEnv, "B1");
+
+ EXPECT_NE(&BeforeSameB1, &AfterSameB1);
+ EXPECT_NE(&BeforeSameB1, &AfterDiffB1);
+ // FIXME: The formula for AfterSameB1 should be different from
+ // AfterDiffB1 to reflect that B1 may be updated.
+ EXPECT_EQ(&AfterSameB1, &AfterDiffB1);
+
+ // The value of B1 is definitely different from before_same vs
+ // after_same.
+ const Formula &B1ChangedForSame =
+ AfterSameEnv.arena().makeNot(AfterSameEnv.arena().makeEquals(
+ AfterSameB1.formula(), BeforeSameB1.formula()));
+ EXPECT_TRUE(AfterSameEnv.allows(B1ChangedForSame));
+ EXPECT_TRUE(AfterSameEnv.proves(B1ChangedForSame));
+
+ // FIXME: It should be possible that B1 *may* be updated, so it may be
+ // that AfterSameB1 != AfterDiffB1 or AfterSameB1 == AfterDiffB1.
+ const Formula &B1ChangedForDiff =
+ AfterDiffEnv.arena().makeNot(AfterDiffEnv.arena().makeEquals(
+ AfterDiffB1.formula(), AfterSameB1.formula()));
+ EXPECT_FALSE(AfterSameEnv.allows(B1ChangedForDiff));
+ // proves() should be false, since B1 may or may not have changed
+ // depending on `Cond`.
+ EXPECT_FALSE(AfterSameEnv.proves(B1ChangedForDiff));
+ });
+}
+
TEST(TransferTest, ConditionalOperatorOnConstantExpr) {
// This is a regression test: We used to crash when a `ConstantExpr` was used
// in the branches of a conditional operator.
|
ymand
reviewed
Nov 21, 2025
| // represent alias sets and weak updates (for the "may"). | ||
| StorageLocation &Loc = Env.createStorageLocation(*S); | ||
| Env.setStorageLocation(*S, Loc); | ||
| if (Value *Val = Environment::joinValues( |
Collaborator
There was a problem hiding this comment.
If the join fails (Val == nullptr), do we still want the newly created Loc to be associated with *S? Or, should line 787 be grouped with line 792?
bazuzi
reviewed
Nov 21, 2025
| EXPECT_TRUE(AfterSameEnv.allows(B1ChangedForSame)); | ||
| EXPECT_TRUE(AfterSameEnv.proves(B1ChangedForSame)); | ||
|
|
||
| // FIXME: It should be possible that B1 *may* be updated, so it may be |
Contributor
There was a problem hiding this comment.
It would more clear to me that only the allows expectation needs to change if this comment were moved to precede that line, after this decl.
ymand
approved these changes
Nov 21, 2025
|
|
||
| TEST(TransferTest, ConditionalOperatorValuesTested) { | ||
| // Even though the LHS and the RHS of the conditional operator have different | ||
| // StorageLocations, we get constraints from the condition that the values |
Collaborator
There was a problem hiding this comment.
I don't follow this text. Could you rephrase?
| const Formula &JoinDifferentIsB1EqB1 = | ||
| Env.arena().makeEquals(JoinDifferentIsB1.formula(), B1.formula()); | ||
| EXPECT_TRUE(Env.allows(JoinDifferentIsB1EqB1)); | ||
| EXPECT_TRUE(Env.proves(JoinDifferentIsB1EqB1)); |
Collaborator
There was a problem hiding this comment.
why both? If "proves" is true, then "allows" is definitely true.
| auto &JoinDifferentIsB2 = | ||
| getValueForDecl<BoolValue>(ASTCtx, Env, "JoinDifferentIsB2"); | ||
|
|
||
| const Formula &JoinDifferentIsB1EqB1 = |
Collaborator
There was a problem hiding this comment.
Add comment explaining the intent here? The name is a little hard to parse.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the dataflow framework, the builtin transfer function currently only
handles the GLValue result case of ConditionalOperator when the
true and false expression StorageLocations are exactly the same.
Ideally / we have wanted to introduce alias sets to handle when the Locs
are different. However, that is a larger change to the framework
(and we may need to introduce weak updates).
For now, do something simpler to at least handle when the GLValue is
immediately cast to an RValue, by making up a distinct StorageLocation
that holds the join of the true and false expression values (when not a
record). This seems like the most common case, so seems worth covering.
The case when an LValue is needed and can be updated later (and
thus needs a link to the original storage locations) seems more rare,
and we currently do not handle such updates either, so this intermediate
step is no different (for that case).