[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