nickdesaulniers added inline comments.
================
Comment at: llvm/lib/Demangle/Demangle.cpp:49
bool llvm::nonMicrosoftDemangle(const char *MangledName, std::string &Result) {
+ if (!MangledName)
+ return false;
----------------
MaskRay wrote:
> nickdesaulniers wrote:
> > nickdesaulniers wrote:
> > > MaskRay wrote:
> > > > Why is this change? The original contract is that `MangledName` must be
> > > > non-null.
> > > https://fosstodon.org/@llvm/110397650834821908
> > >
> > > It's insidious; implicitly constructing a `std::string_view` from a
> > > `char*` which is `nullptr` invokes a call to `strlen` upon construction,
> > > which segfaults on my system. Therefor, all of the `nullptr` checks need
> > > to be hoisted into the callers or stated another way exist at the
> > > boundary of `char*` to `std::string_view` API boundaries (such as right
> > > here).
> > This is also why the `nullptr` input test case must be removed from
> > llvm/unittests/Demangle/DLangDemangleTest.cpp below.
> It's usually code smell if a function taking a C string argument additionally
> accepts nullptr.
> Previously, passing `nullptr` to `nonMicrosoftDemangle` will crash as
> `MangledName[0]` is accessed. We should retain this strictness.
>
> `ninja check-llvm-demangle` passes if I remove the 2 lines.
>
> If future refactoring will possibly pass `nullptr` to `nonMicrosoftDemangle`,
> I think we should fix those call sites not to do that...
> We should retain this strictness.
> If future refactoring will possibly pass nullptr to nonMicrosoftDemangle, I
> think we should fix those call sites not to do that...
Ok, I will drop it then, and add nullptr checks to callers.
> ninja check-llvm-demangle passes if I remove the 2 lines.
Then `check-llvm-demangle` must not be running
llvm/unittests/Demangle/DLangDemangleTest.cpp, or you tested with my change to
llvm/unittests/Demangle/DLangDemangleTest.cpp from below applied.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D151003/new/
https://reviews.llvm.org/D151003
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
