[PATCH] COFFDumper: Dump data directory entries.

Rui Ueyama ruiu at google.com
Tue Jul 16 09:38:30 PDT 2013



================
Comment at: lib/Object/COFFObjectFile.cpp:46
@@ +45,3 @@
+  uintptr_t Addr = uintptr_t(Ptr);
+  if (Addr + Size < Addr ||
+      Addr + Size < Size ||
----------------
Rafael Ávila de Espíndola wrote:
> To test that [addr, addr + Size] is in M, you only need to check
> 
> Addr >= M->getBufferStart() && Addr+Size <= M->getBufferEnd(), no?
> 
> Can't you assert that it is true instead of returning an error?
> 
Size may be from an input file so it can be a very large number. I think the extra condition is to check for integer overflow. We shouldn't use assert() here because if it fails it means the input file is broken and it's not a bug of this code.

================
Comment at: lib/Object/COFFObjectFile.cpp:613
@@ +612,3 @@
+                                            const data_directory *&Res) const {
+  // Error if if there's no data directory or the index is out of range.
+  if (!DataDirectory || index > PE32Header->NumberOfRvaAndSize)
----------------
Rafael Ávila de Espíndola wrote:
> Just returing a data_directory* (possibly null) would be a simpler interface.
All the other get* functions have the same return type. I think it's nice to keep it consistent with others.


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

BRANCH
  readobj

ARCANIST PROJECT
  llvm



More information about the llvm-commits mailing list