Fix an undefined behavior when storing an empty StringRef.

[hokein]

Summary

Passing a nullptr to memcpy is UB.

[ioeric]

lib/Support/StringSaver.cpp

14 ↗

(On Diff #161463)

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

[hokein]

Correct the fix.

[hokein]

lib/Support/StringSaver.cpp

14 ↗

(On Diff #161463)

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

[ioeric]

lg

lib/Support/StringSaver.cpp

14 ↗

(On Diff #161463)

Hmm, that seems strange but out of the scope.

[joerg]

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);
...

llvm/trunk/lib/Support/StringSaver.cpp

 //===-- StringSaver.cpp ---------------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/StringSaver.h"
 
 using namespace llvm;
 
 StringRef StringSaver::save(StringRef S) {
   char *P = Alloc.Allocate<char>(S.size() + 1);
+  if (!S.empty())
     memcpy(P, S.data(), S.size());
   P[S.size()] = '\0';
   return StringRef(P, S.size());
 }
 
 StringRef UniqueStringSaver::save(StringRef S) {
   auto R = Unique.insert(S);
   if (R.second)                 // cache miss, need to actually save the string
     *R.first = Strings.save(S); // safe replacement with equal value
   return *R.first;
 }