[PATCH] D77924: ld128 demangle: allow space for 'L' suffix.

[Elliott Hughes via Phabricator via llvm-commits]

enh marked an inline comment as done.
enh added a comment.

In D77924#1977737 <https://reviews.llvm.org/D77924#1977737>, @ldionne wrote:

> Is it possible to add a test to always trigger this bug?


only if you run with hwasan.

In D77924#1976208 <https://reviews.llvm.org/D77924#1976208>, @erik.pilkington wrote:

> Can you add your test to libcxxabi/test/test_demangle.pass.cpp? You can put it in invalid_cases if you expect it to fail to demangle.


no, it's a valid demangle (for systems where `long double` is binary128), and the current code doesn't crash anywhere i know of --- it just overflows the buffer.

i'll send a new patch with 42 (and a comment explaining where that came from) when i've worked out how to get phabricator to do that...



================
Comment at: libcxxabi/src/demangle/ItaniumDemangle.h:5177
 #endif
-    static const size_t max_demangled_size = 40;
+    static const size_t max_demangled_size = 41;
     static constexpr const char *spec = "%LaL";
----------------
erik.pilkington wrote:
> Do you know to get the actual upper bound of this? Is it actually 41? Its only used one place for a stack buffer so it doesn't really matter if its too large. It'd also be nice if we were more defensive in the call to snprintf, we should've been able to handle this more gracefully. 
no, i think it's actually 42.

i think `-0x1.ffffffffffffffffffffffffffffp+16383` is the longest string, which is 40, plus 1 for 'L', plus 1 for '\0' == 42.

the `0x1.` and `p+` are fixed, positives are a character shorter, and the 28 'f's == 28*4 bits == 112 bits, which is the number of bits explicitly stored in an IEEE binary128. (because there's always an explicit sign after `p`, the -16382 smallest exponent is the same length as the +16383 largest exponent.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77924