[PATCH] D149675: [Demangle] convert itaniumDemangle and nonMicrosoftDemangle to use std::string_view

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 2 15:57:16 PDT 2023


MaskRay added a comment.

In D149675#4313962 <https://reviews.llvm.org/D149675#4313962>, @efriedma wrote:

> LGTM
>
> In D149675#4313944 <https://reviews.llvm.org/D149675#4313944>, @nickdesaulniers wrote:
>
>> In D149675#4313933 <https://reviews.llvm.org/D149675#4313933>, @efriedma wrote:
>>
>>> Maybe it makes sense to revert D149104 <https://reviews.llvm.org/D149104> while you're fixing the various demanglers to correctly respect the input length?
>>
>> This should fix-forwards the test. Sorry it took me so long to root cause.
>
> If I understand the situation correctly, all the other demanglers read past the end of the buffer too; we just get lucky that none of the tests on the buildbots use the API in a way which exposes that.  I'd prefer to revert to known working, then fix each demangler, then fix the generic demangle() API.

Sounds good. So we need two commits

- Revert D149104 <https://reviews.llvm.org/D149104>. The description of this patch can be used by the revert commit.
- Combine D149104 <https://reviews.llvm.org/D149104> and this patch

Both commits need very sophisticated testing (including `check-lldb check-bolt`)...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149675



More information about the llvm-commits mailing list