[PATCH] MC: Support larger COFF string tables

Rafael Ávila de Espíndola rafael.espindola at gmail.com
Mon Apr 15 07:25:55 PDT 2013


  The implementation looks fine with just some nits.

  Just wait for Bigcheese to comment on the format itself.

  Thanks!


================
Comment at: lib/Object/COFFObjectFile.cpp:77
@@ +76,3 @@
+    // invalid.
+    if (CharVal >= 64)
+      return true;
----------------
You just computed CharVal. You can just assert that it is < 64, no?

================
Comment at: lib/Object/COFFObjectFile.cpp:85
@@ +84,3 @@
+    // Check for overflow by shifting back and seeing if bits were lost.
+    if (Result/64 < PrevResult)
+      return true;
----------------
Why not do the math with an uint64_t? With that you assert this too, no? Since you have an assert on the Str size being <= 6, you get at most 6 * log2(64) bits (36). That way you only need one check outside the loop if the result must fit in 32 bits.




http://llvm-reviews.chandlerc.com/D667



More information about the llvm-commits mailing list