[PATCH] D38020: [COFF] Adjust secrel limit assertion

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added inline comments.


================
Comment at: COFF/Chunks.cpp:65
   uint64_t SecRel = S - OS->getRVA();
-  assert(SecRel < INT32_MAX && "overflow in SECREL relocation");
+  assert(SecRel < UINT32_MAX && "overflow in SECREL relocation");
   add32(Off, SecRel);
----------------
ruiu wrote:
> Can you use error() instead of assert()? If you can intentionally trigger an assertion, it shouldn't be an assert() but should be handled by error().
I thought about that, but I think an assertion is actually the right thing here. D38005 makes it so that a section larger than 4 GiB will trigger an error, which means relocation processing can assume that as a precondition, and a `SecRel` larger than 4 GiB implies a section larger than 4 GiB, which implies a precondition violation. Does that seem reasonable?

On the other hand, since D38005 is using `error`, I suppose it's possible to enter here with a section larger than 4 GiB which you've already issued an error for previously (but processing continued after issuing that error), so that makes it slightly less clear.


https://reviews.llvm.org/D38020