[llvm] r214270 - UseListOrder: Fix undefined behaviour

Duncan P. N. Exon Smith dexonsmith at apple.com
Tue Jul 29 18:20:26 PDT 2014


Author: dexonsmith
Date: Tue Jul 29 20:20:26 2014
New Revision: 214270

URL: http://llvm.org/viewvc/llvm-project?rev=214270&view=rev
Log:
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.

Modified:
    llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp

Modified: llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp?rev=214270&r1=214269&r2=214270&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp (original)
+++ llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp Tue Jul 29 20:20:26 2014
@@ -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, O
           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) {





More information about the llvm-commits mailing list