[PATCH] D124524: [demangler] Avoid nullptr UB

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D124524#3478021 <https://reviews.llvm.org/D124524#3478021>, @urnathan wrote:

> In D124524#3477537 <https://reviews.llvm.org/D124524#3477537>, @dblaikie wrote:
>
>> Hmm - why does this need an allocation, though? A `StringPiece(nullptr, nullptr)` sounds valid to me? Is something using a distinction between StringPiece that's zero-length but non-null and StringPiece that's null?
>
> The UB is that we end up at `memcpy (dst, SV.begin(), SV.size())`, even though that's copying zero bytes, we have a null SV.begin().  C++20's string_view cannot be over a null pointer, and ISTM that the demangler's StringView (should) have the same restriction.  StringView is a pair of pointers, along with a FIXME to move to std::string_view when C++17 is the implementation language.  Hm, perhaps the assert belongs inside StringView?
>
> This seemed the least-worst way of addressing it.  Alternatively I guess we could unconditionally allocate in OutputBuffer's ctor, but that seemed less pleasant.

Ah, OK - thanks for walking me through it. The other alternative could be to wrap the memcpy in "if (SV.data() != nullptr)"? That feels more direct/explicit/intentional about the issue, to me at least.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124524/new/

https://reviews.llvm.org/D124524