[clang] remove dereferencing of invalid pointer

[ashay-github]

Summary

A line in the demangling code for float literals dereferences the
.end() iterator, which causes the Windows debug build of llvm-cxxfilt
to crash. The failure can be reproduced by passing the string
_Z5dummyIXtl8wrapper1IdEtlNS1_Ut_Edi9RightNametlNS2_Ut_ELd405ec00000000000EEEEEEvv
to llvm-cxxfilt -n.

This patch rewrites the code to use the .size() member of the
string_view type to avoid dereferencing past the buffer.

[DavidSpickett]

Seems fine to me, that is what size is for after all :). Just one comment.

llvm/include/llvm/Demangle/ItaniumDemangle.h

2332–2333

Might as well merge this into the const char* t = line below.

[ashay-github]

inlined the definition of first into its two uses

[DavidSpickett]

LGTM, thanks.

[nickdesaulniers]

Thanks for the patch! It would have been good to include your test case though.

[ashay-github]

In D149061#4292974, @nickdesaulniers wrote:

It would have been good to include your test case though.

Ah, I forgot to mention that the error was also reproducible using an existing test (the one in clang/test/CodeGenCXX/mangle-nttp-anon-union.cpp). The specific case that I mentioned ("_Z5dummyIXtl8wrapper1IdEtlNS1_Ut_Edi9RightNametlNS2_Ut_ELd405ec00000000000EEEEEEvv") is just a subset of the mangle-nttp-anon-union.cpp test, specifically the lines below:

dummy<wrapper1<double>{123.0}>();
// CHECK: call void @_Z5dummyIXtl8wrapper1IdEtlNS1_Ut_Edi9RightNametlNS2_Ut_ELd405ec00000000000EEEEEEvv
// DEMANGLED: call void @void dummy<wrapper1<double>{wrapper1<double>::'unnamed'{.RightName = wrapper1<double>::'unnamed'::'unnamed'{0x1.ec{{.*}}p+6}}}>()()

[nickdesaulniers]

In D149061#4297011, @ashay-github wrote:

In D149061#4292974, @nickdesaulniers wrote:

It would have been good to include your test case though.

Ah, I forgot to mention that the error was also reproducible using an existing test (the one in clang/test/CodeGenCXX/mangle-nttp-anon-union.cpp). The specific case that I mentioned ("_Z5dummyIXtl8wrapper1IdEtlNS1_Ut_Edi9RightNametlNS2_Ut_ELd405ec00000000000EEEEEEvv") is just a subset of the mangle-nttp-anon-union.cpp test, specifically the lines below:

dummy<wrapper1<double>{123.0}>();
// CHECK: call void @_Z5dummyIXtl8wrapper1IdEtlNS1_Ut_Edi9RightNametlNS2_Ut_ELd405ec00000000000EEEEEEvv
// DEMANGLED: call void @void dummy<wrapper1<double>{wrapper1<double>::'unnamed'{.RightName = wrapper1<double>::'unnamed'::'unnamed'{0x1.ec{{.*}}p+6}}}>()()

Got it. Any ideas why I'm not able to reproduce on a non-windows host (and AFAIK that specific test case hasn't been flagged by any build bots)? I would think we should still be able to test the windows demangling scheme on any host.

[nickdesaulniers]

Also, llvm/include/llvm/Demangle/ is the downstream copy from libcxxabi/. I'll have to copy it back in https://reviews.llvm.org/D148566.

[ashay-github]

Any ideas why I'm not able to reproduce on a non-windows host (and AFAIK that specific test case hasn't been flagged by any build bots)? I would think we should still be able to test the windows demangling scheme on any host.

My apologies for dropping this. I was out of town when your message arrived.

My hypothesis is that you may have build LLVM in Release configuration (and perhaps the bots build in Release configuration as well). Our Windows Release builds were passing but the Debug builds failed. When I looked through the standard library functions in MSVC header files, I found that a vast number of them contained #ifdef statements that were predicated on whether the compiler is called with debug flags, with the debug builds containing far more conservative checks.

llvm/include/llvm/Demangle/ItaniumDemangle.h

static constexpr Kind KindForClass =
       float_literal_impl::getFloatLiteralKind((Float *)nullptr);
 
 public:
   FloatLiteralImpl(std::string_view Contents_)
       : Node(KindForClass), Contents(Contents_) {}
 
   template<typename Fn> void match(Fn F) const { F(Contents); }
 
   void printLeft(OutputBuffer &OB) const override {
-    const char *first = &*Contents.begin();
-    const char *last = &*Contents.end() + 1;
-
     const size_t N = FloatData<Float>::mangled_size;
-    if (static_cast<std::size_t>(last - first) > N) {
-      last = first + N;
+    if (Contents.size() >= N) {
       union {
         Float value;
         char buf[sizeof(Float)];
       };
-      const char *t = first;
+      const char *t = &*Contents.begin();
+      const char *last = t + N;
       char *e = buf;
       for (; t != last; ++t, ++e) {
         unsigned d1 = isdigit(*t) ? static_cast<unsigned>(*t - '0')
                                   : static_cast<unsigned>(*t - 'a' + 10);
         ++t;
         unsigned d0 = isdigit(*t) ? static_cast<unsigned>(*t - '0')
                                   : static_cast<unsigned>(*t - 'a' + 10);
         *e = static_cast<char>((d1 << 4) + d0);