[PATCH] D51652: lld-link: Write an empty "repro" debug directory entry if /Brepro is passed

[Reid Kleckner via Phabricator via llvm-commits]

rnk accepted this revision.
rnk added a comment.

lgtm

I think @pcc's concerns are addressed, too.



================
Comment at: llvm/lib/Object/COFFObjectFile.cpp:640-641
   DebugDirectoryBegin = reinterpret_cast<const debug_directory *>(IntPtr);
-  if (std::error_code EC = getRvaPtr(
-          DataEntry->RelativeVirtualAddress + DataEntry->Size, IntPtr))
-    return EC;
-  DebugDirectoryEnd = reinterpret_cast<const debug_directory *>(IntPtr);
+  DebugDirectoryEnd = reinterpret_cast<const debug_directory *>(
+      IntPtr + DataEntry->Size);
   return std::error_code();
----------------
thakis wrote:
> rnk wrote:
> > This seems like it loses the error check that the debug directory is in bounds. Is there a better idiom for ensuring that a range is inside the object?
> This is a valid point, but the check that was here just checked that the first and the last object were both contained in some section. If the start was in one section and the end in another, the range would still be invalid and pass the check. What we really want here I think is to retrieve the section for the start and then check that it's large enough to also contain the end in the same section. (Same in initBaseRelocPtr.) That seems maybe out of scope for this patch? I added a FIXME.
Sounds good. Designing safe APIs is hard. =/


https://reviews.llvm.org/D51652