Skip to content

Commit c128f3b

Browse files
committed
UseListOrder: Fix undefined behaviour
This commit fixes undefined behaviour that caused the revert in r214249. The problem was two unsequenced operations on a `DenseMap<>`, giving different behaviour in GCC and Clang. This: DenseMap<T*, unsigned> DM; for (auto &X : ...) DM[&X] = DM.size() + 1; should have been: DenseMap<T*, unsigned> DM; for (auto &X : ...) { unsigned Size = DM.size(); DM[&X] = Size + 1; } Until r214242, this difference between compilers didn't matter. In r214242, `OrderMap::LastGlobalValueID` was introduced and compared against IDs, which in GCC were off-by-one my expectations. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@214270 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 8ad2443 commit c128f3b

File tree

1 file changed

+7
-2
lines changed

1 file changed

+7
-2
lines changed

lib/Bitcode/Writer/ValueEnumerator.cpp

+7-2
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ struct OrderMap {
3434
std::pair<unsigned, bool> lookup(const Value *V) const {
3535
return IDs.lookup(V);
3636
}
37+
void index(const Value *V) {
38+
// Explicitly sequence get-size and insert-value operations to avoid UB.
39+
unsigned ID = IDs.size() + 1;
40+
IDs[V].first = ID;
41+
}
3742
};
3843
}
3944

@@ -48,8 +53,8 @@ static void orderValue(const Value *V, OrderMap &OM) {
4853
orderValue(Op, OM);
4954

5055
// Note: we cannot cache this lookup above, since inserting into the map
51-
// changes the map's size, and thus affects the ID.
52-
OM[V].first = OM.size() + 1;
56+
// changes the map's size, and thus affects the other IDs.
57+
OM.index(V);
5358
}
5459

5560
static OrderMap orderModule(const Module *M) {

0 commit comments

Comments
 (0)