UseListOrder: Fix undefined behaviour
authorDuncan P. N. Exon Smith <dexonsmith@apple.com>
Wed, 30 Jul 2014 01:20:26 +0000 (01:20 +0000)
committerDuncan P. N. Exon Smith <dexonsmith@apple.com>
Wed, 30 Jul 2014 01:20:26 +0000 (01:20 +0000)
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.

llvm-svn: 214270

llvm/lib/Bitcode/Writer/ValueEnumerator.cpp

index 5624e02..144e8fb 100644 (file)
@@ -34,6 +34,11 @@ struct OrderMap {
   std::pair<unsigned, bool> lookup(const Value *V) const {
     return IDs.lookup(V);
   }
+  void index(const Value *V) {
+    // Explicitly sequence get-size and insert-value operations to avoid UB.
+    unsigned ID = IDs.size() + 1;
+    IDs[V].first = ID;
+  }
 };
 }
 
@@ -48,8 +53,8 @@ static void orderValue(const Value *V, OrderMap &OM) {
           orderValue(Op, OM);
 
   // Note: we cannot cache this lookup above, since inserting into the map
-  // changes the map's size, and thus affects the ID.
-  OM[V].first = OM.size() + 1;
+  // changes the map's size, and thus affects the other IDs.
+  OM.index(V);
 }
 
 static OrderMap orderModule(const Module *M) {