[PATCH] D103641: [scudo] Rework Vector/String

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad added a comment.

In D103641#2797247 <https://reviews.llvm.org/D103641#2797247>, @hctim wrote:

>> CanGrow templated parameter that for now is
>> always true but would be set to false on Trusty.
>
> Are we seeing OOMs on Trusty due to ScopedString? I had a quick look and it seemed like there's no non-ephemeral buffers (and really, this shouldn't be called very often at all), and it concerns me a little that `push_back` can silently fail on a ScopedErrorReport and truncate the text and it seems hard to test "all failure conditions log successfully".

Some Trusty apps have a heap of 1 page (yup), and all maps end up in the sbrk space. So in it's current shape, a ScopedString is taking way too much space in the heap.
`push_back` is only used in tests so far, `formatString` failing might be a little more problematic. I think there will be some truncation for larger strings, but more of the simple ones will go through, and that's probably the price to pay when you have such a small heap.



================
Comment at: compiler-rt/lib/scudo/standalone/string_utils.h:22
+  explicit ScopedString(uptr InitialSize = 0) : String(InitialSize) {
     String[0] = '\0';
   }
----------------
hctim wrote:
> With `InitialSize == 0`, isn't this OOB?
Well we always have at least the local buffer worth of space, so it won't OOB, but I see how this isn't  obvious.
I am going to add a check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103641