[PATCH] D47047: MC: Remove redundant substr() call

Francis Visoiu Mistrih via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 30 09:14:57 PDT 2018


thegameg added inline comments.


================
Comment at: llvm/trunk/lib/Object/MachOObjectFile.cpp:110
 static const char *getPtr(const MachOObjectFile &O, size_t Offset) {
-  return O.getData().substr(Offset, 1).data();
 }
----------------
sbc100 wrote:
> thegameg wrote:
> > `.substr` never goes past end if `Offset` > `getData().size()`. Maybe add an assert to make sure no callers rely on this?
> Good point, I was trying to figure out what the original intention was and I guess that may have been it.   But it doesn't make a lot of sense to me.. from looking at the StringRef::substr code it looks like it will always return a pointer to the end of the string if you request and out of bounds offset, then the code the receives this pointer will read out of bounds just off the end of the string.  I can't see why this would be useful or desired.    But adding an assert would certainly be an improvement.  I will do that.
> 
> Do you think this change could be responsible for the build break you are seeing?  I can't really see how it could break anything that wasn't already very broken.  But I could be missing something.
> 
Thanks! For the build, nothing obvious comes to mind, once I'll be able to reproduce locally I'll let you know.


Repository:
  rL LLVM

https://reviews.llvm.org/D47047





More information about the llvm-commits mailing list