This is an archive of the discontinued LLVM Phabricator instance.

Fix an undefined behavior when storing an empty StringRef.
ClosedPublic

Authored by hokein on Aug 20 2018, 5:21 AM.

Diff Detail

Event Timeline

hokein created this revision.Aug 20 2018, 5:21 AM
ioeric added inline comments.Aug 20 2018, 5:29 AM
lib/Support/StringSaver.cpp
14

Is it possible to reuse StringRef::copy(Allocator &) here?

hokein updated this revision to Diff 161467.Aug 20 2018, 6:01 AM

Correct the fix.

hokein added inline comments.Aug 20 2018, 6:02 AM
lib/Support/StringSaver.cpp
14

Unfortunately, not, StringRef::copy will change the behavior. The StringRef returned here is null-terminated.

ioeric accepted this revision.Aug 20 2018, 6:09 AM

lg

lib/Support/StringSaver.cpp
14

Hmm, that seems strange but out of the scope.

This revision is now accepted and ready to land.Aug 20 2018, 6:09 AM
This revision was automatically updated to reflect the committed changes.
joerg added a subscriber: joerg.Aug 20 2018, 6:28 AM

Why do we need to allocate memory in this case at all? I.e. why can't this just be:

if (S.empty())
  return StringRef("", 0);
...