[PATCH] D14711: fix lldb crash by changing const char * with StringRef

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 16 09:54:13 PST 2015


On Mon, Nov 16, 2015 at 9:47 AM, Eugene Kosov <claprix at yandex.ru> wrote:

> Hello.
>
> Here is the description of a crash
> http://lists.llvm.org/pipermail/llvm-dev/2015-November/092429.html
>
> I thinks its testable. Option::ArgStr, Option::HelpStr, Option::ValueStr
> must not be nullptr. We could make them private and put assertions in
> setter function. And test it. StringRef can not be created from nullptr
> string.
>

Ah, so it's a matter of the current code observes a null/value initialized
const char* whereas the new code will observe a default constructed
StringRef which represents the empty string, not null?

Could you include a test case in your patch? It's helpful to have test
coverage in LLVM for LLVM features that our users (including in-project
uses in places like Clang and LLDB) depend on.

I'm not quite following if/why this is a valid state (or whether changing
to StringRef is just working around a situation and demoting a crash to
some other unexpected but non-crashy behavior), though - some more detail
might be helpful, but I'm not sure.


>
> 16.11.2015, 20:38, "David Blaikie" <dblaikie at gmail.com>:
> > What's the actual bug in LLDB's usage & how does this address it? Is it
> testable in LLVM? (unit tests?)
> >
> > On Mon, Nov 16, 2015 at 9:26 AM, Eugene via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> >> kevgs created this revision.
> >> kevgs added a reviewer: llvm-commits.
> >> kevgs added a subscriber: llvm-commits.
> >> kevgs set the repository for this revision to rL LLVM.
> >>
> >> Description:
> http://lists.llvm.org/pipermail/llvm-dev/2015-November/092429.html
> >>
> >> Repository:
> >>   rL LLVM
> >>
> >> http://reviews.llvm.org/D14711
> >>
> >> Files:
> >>   include/llvm/Support/CommandLine.h
> >>   lib/Support/CommandLine.cpp
> >>   unittests/Support/CommandLineTest.cpp
> >>   utils/FileCheck/FileCheck.cpp
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
> --
> Eugene
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151116/22605414/attachment.html>


More information about the llvm-commits mailing list