[PATCH] D50966: Fix an undefined behavior when storing an empty StringRef.

2018-08-20 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: ioeric. Herald added a subscriber: llvm-commits. Passing a nullptr to memcpy is UB. Repository: rL LLVM https://reviews.llvm.org/D50966 Files: lib/Support/StringSaver.cpp Index: lib/Support/StringSaver.cpp ===

[PATCH] D50966: Fix an undefined behavior when storing an empty StringRef.

2018-08-20 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: lib/Support/StringSaver.cpp:14 StringRef StringSaver::save(StringRef S) { + if (S.empty()) Is it possible to reuse `StringRef::copy(Allocator &)` here? Repository: rL LLVM https://reviews.llvm.org/D50966 _

[PATCH] D50966: Fix an undefined behavior when storing an empty StringRef.

2018-08-20 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 161467. hokein added a comment. Correct the fix. Repository: rL LLVM https://reviews.llvm.org/D50966 Files: lib/Support/StringSaver.cpp Index: lib/Support/StringSaver.cpp === --- lib/Supp

[PATCH] D50966: Fix an undefined behavior when storing an empty StringRef.

2018-08-20 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: lib/Support/StringSaver.cpp:14 StringRef StringSaver::save(StringRef S) { + if (S.empty()) ioeric wrote: > Is it possible to reuse `StringRef::copy(Allocator &)` here? Unfortunately, not, `StringRef::copy` will change

[PATCH] D50966: Fix an undefined behavior when storing an empty StringRef.

2018-08-20 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lg Comment at: lib/Support/StringSaver.cpp:14 StringRef StringSaver::save(StringRef S) { + if (S.empty()) hokein wrote: > ioeric wrote: > > Is it possibl

[PATCH] D50966: Fix an undefined behavior when storing an empty StringRef.

2018-08-20 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL340170: Fix an undefined behavior when storing an empty StringRef. (authored by hokein, committed by ). Repository: rL LLVM https://reviews.llvm.org/D50966 Files: llvm/trunk/lib/Support/StringSaver.

[PATCH] D50966: Fix an undefined behavior when storing an empty StringRef.

2018-08-20 Thread Joerg Sonnenberger via Phabricator via cfe-commits
joerg added a comment. 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); ... Repository: rL LLVM https://reviews.llvm.org/D50966 ___ cfe-commits mailing list cfe-