[lldb-dev] running lldb-mi with LLDB_DISABLE_PYTHON

2016-09-20 Thread Chunseok Lee via lldb-dev
Dear lldb dev team,

I am working on running lldb-mi on arm32 device(like rpi) for remote
debugging usage.
Is there any way to run lldb-mi with python disabled ?
When building lldb with LLDB_DISABLE_PYTHON, it seems that dataformatter
initialization is failed due to the following code in
MICmnLLDBDebugger.cpp:59

//++
//
// MI summary helper routines
static inline bool MI_add_summary(lldb::SBTypeCategory category,
  const char *typeName,
  lldb::SBTypeSummary::FormatCallback cb,
  uint32_t options, bool regex = false) {
#if defined(LLDB_DISABLE_PYTHON)
  return false;
#else
  lldb::SBTypeSummary summary =
  lldb::SBTypeSummary::CreateWithCallback(cb, options);
  return summary.IsValid()
 ? category.AddTypeSummary(
   lldb::SBTypeNameSpecifier(typeName, regex), summary)
 : false;
#endif
}

Thank you.

BR,
Chunseok Lee

-- 
Where Do We come from? What Are We? Where Are We Going?
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] running lldb-mi with LLDB_DISABLE_PYTHON

2016-09-20 Thread Ilia K via lldb-dev
Hi!

Please see possible workarounds in
https://llvm.org/bugs/show_bug.cgi?id=28253.

On Tue, Sep 20, 2016 at 10:57 AM, Chunseok Lee via lldb-dev <
lldb-dev@lists.llvm.org> wrote:

> Dear lldb dev team,
>
> I am working on running lldb-mi on arm32 device(like rpi) for remote
> debugging usage.
> Is there any way to run lldb-mi with python disabled ?
> When building lldb with LLDB_DISABLE_PYTHON, it seems that dataformatter
> initialization is failed due to the following code in
> MICmnLLDBDebugger.cpp:59
>
> //++
> //--
> --
> // MI summary helper routines
> static inline bool MI_add_summary(lldb::SBTypeCategory category,
>   const char *typeName,
>   lldb::SBTypeSummary::FormatCallback cb,
>   uint32_t options, bool regex = false) {
> #if defined(LLDB_DISABLE_PYTHON)
>   return false;
> #else
>   lldb::SBTypeSummary summary =
>   lldb::SBTypeSummary::CreateWithCallback(cb, options);
>   return summary.IsValid()
>  ? category.AddTypeSummary(
>lldb::SBTypeNameSpecifier(typeName, regex), summary)
>  : false;
> #endif
> }
>
> Thank you.
>
> BR,
> Chunseok Lee
>
> --
> Where Do We come from? What Are We? Where Are We Going?
>
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
>


-- 
- Ilia
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Greg Clayton via lldb-dev
But StringRef is a smart string wrapper and it is there for exactly this 
reason: to make string handling correct. So let us let it be smart and not 
crash if you make it with null and call .size() on it...
> On Sep 19, 2016, at 2:37 PM, Zachary Turner  wrote:
> 
> FWIW, strlen(nullptr) will also crash just as easily as StringRef(nullptr) 
> will crash, so that one isn't a particularly convincing example of poorly 
> used asserts, since the onus on the developer is exactly the same as with 
> strlen.  That said, I still know where you're coming from :)
> 
> On Mon, Sep 19, 2016 at 2:34 PM Greg Clayton  wrote:
> 
> > On Sep 19, 2016, at 2:20 PM, Mehdi Amini  wrote:
> >
> >>
> >> On Sep 19, 2016, at 2:02 PM, Greg Clayton via lldb-dev 
> >>  wrote:
> >>
> >>
> >>> On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev 
> >>>  wrote:
> >>>
> >>> Following up with Kate's post from a few weeks ago, I think the dust has 
> >>> settled on the code reformat and it went over pretty smoothly for the 
> >>> most part.  So I thought it might be worth throwing out some ideas for 
> >>> where we go from here.  I have a large list of ideas (more ideas than 
> >>> time, sadly) that I've been collecting over the past few weeks, so I 
> >>> figured I would throw them out in the open for discussion.
> >>>
> >>> I’ve grouped the areas for improvement into 3 high level categories.
> >>>
> >>> • De-inventing the wheel - We should use more code from LLVM, and 
> >>> delete code in LLDB where LLVM provides a solution.  In cases where there 
> >>> is an LLVM thing that is *similar* to what we need, we should extend the 
> >>> LLVM thing to support what we need, and then use it.  Following are some 
> >>> areas I've identified.  This list is by no means complete.  For each one, 
> >>> I've given a personal assessment of how likely it is to cause some 
> >>> (temporary) hiccups, how much it would help us in the long run, and how 
> >>> difficult it would be to do.  Without further ado:
> >>> • Use llvm::Regex instead of lldb::Regex
> >>> • llvm::Regex doesn’t support enhanced mode.  Could 
> >>> we add support for this to llvm::Regex?
> >>> • Risk: 6
> >>> • Impact: 3
> >>> • Difficulty / Effort: 3  (5 if we have to add 
> >>> enhanced mode support)
> >>
> >> As long as it supports all of the features, then this is fine. Otherwise 
> >> we will need to modify the regular expressions to work with the LLVM 
> >> version or back port any advances regex features we need into the LLVM 
> >> regex parser.
> >>
> >>> • Use llvm streams instead of lldb::StreamString
> >>> • Supports output re-targeting (stderr, stdout, 
> >>> std::string, etc), printf style formatting, and type-safe streaming 
> >>> operators.
> >>> • Interoperates nicely with many existing llvm 
> >>> utility classes
> >>> • Risk: 4
> >>> • Impact: 5
> >>> • Difficulty / Effort: 7
> >>
> >> I have worked extensively with both C++ streams and with printf. I don't 
> >> find the type safe streaming operators to be readable and a great way to 
> >> place things into streams. Part of my objection to this will be quelled if 
> >> the LLVM streams are not stateful like C++ streams are (add an extra 
> >> "std::hex" into the stream and suddenly other things down stream start 
> >> coming out in hex). As long as printf functionality is in the LLVM streams 
> >> I am ok with it.
> >>
> >>> • Use llvm::Error instead of lldb::Error
> >>> • llvm::Error is an error class that *requires* you 
> >>> to check whether it succeeded or it will assert.  In a way, it's similar 
> >>> to a C++ exception, except that it doesn't come with the performance hit 
> >>> associated with exceptions.  It's extensible, and can be easily extended 
> >>> to support the various ways LLDB needs to construct errors and error 
> >>> messages.
> >>> • Would need to first rename lldb::Error to LLDBError 
> >>> so that te conversion from LLDBError to llvm::Error could be done 
> >>> incrementally.
> >>> • Risk: 7
> >>> • Impact: 7
> >>> • Difficulty / Effort: 8
> >>
> >> We must be very careful here. Nothing can crash LLDB and adding something 
> >> that will be known to crash in some cases can only be changed if there is 
> >> a test that can guarantee that it won't crash. assert() calls in a shared 
> >> library like LLDB are not OK and shouldn't fire. Even if they do, when 
> >> asserts are off, then it shouldn't crash LLDB. We have made efforts to 
> >> only use asserts in LLDB for things that really can't happen, but for 
> >> things like constructing a StringRef with NULL is one example of why I 
> >> don't want to use certain classes in LLVM. If we can remove the crash 
> >> threat, then lets

Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Zachary Turner via lldb-dev
FWIW I added a function to StringRef the other day that looks like this:

static StringRef withNullAsEmpty(const char *Str) {
  return StringRef(Str ? Str : "");
}

I've been using this code in converting existing uses of const char * over
to StringRef.  It's not 100% what you want, but at least it's better than
nothing.

On Tue, Sep 20, 2016 at 10:26 AM Greg Clayton  wrote:

> But StringRef is a smart string wrapper and it is there for exactly this
> reason: to make string handling correct. So let us let it be smart and not
> crash if you make it with null and call .size() on it...
> > On Sep 19, 2016, at 2:37 PM, Zachary Turner  wrote:
> >
> > FWIW, strlen(nullptr) will also crash just as easily as
> StringRef(nullptr) will crash, so that one isn't a particularly convincing
> example of poorly used asserts, since the onus on the developer is exactly
> the same as with strlen.  That said, I still know where you're coming from
> :)
> >
> > On Mon, Sep 19, 2016 at 2:34 PM Greg Clayton  wrote:
> >
> > > On Sep 19, 2016, at 2:20 PM, Mehdi Amini 
> wrote:
> > >
> > >>
> > >> On Sep 19, 2016, at 2:02 PM, Greg Clayton via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> > >>
> > >>
> > >>> On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> > >>>
> > >>> Following up with Kate's post from a few weeks ago, I think the dust
> has settled on the code reformat and it went over pretty smoothly for the
> most part.  So I thought it might be worth throwing out some ideas for
> where we go from here.  I have a large list of ideas (more ideas than time,
> sadly) that I've been collecting over the past few weeks, so I figured I
> would throw them out in the open for discussion.
> > >>>
> > >>> I’ve grouped the areas for improvement into 3 high level categories.
> > >>>
> > >>> • De-inventing the wheel - We should use more code from LLVM,
> and delete code in LLDB where LLVM provides a solution.  In cases where
> there is an LLVM thing that is *similar* to what we need, we should extend
> the LLVM thing to support what we need, and then use it.  Following are
> some areas I've identified.  This list is by no means complete.  For each
> one, I've given a personal assessment of how likely it is to cause some
> (temporary) hiccups, how much it would help us in the long run, and how
> difficult it would be to do.  Without further ado:
> > >>> • Use llvm::Regex instead of lldb::Regex
> > >>> • llvm::Regex doesn’t support enhanced mode.
> Could we add support for this to llvm::Regex?
> > >>> • Risk: 6
> > >>> • Impact: 3
> > >>> • Difficulty / Effort: 3  (5 if we have to add
> enhanced mode support)
> > >>
> > >> As long as it supports all of the features, then this is fine.
> Otherwise we will need to modify the regular expressions to work with the
> LLVM version or back port any advances regex features we need into the LLVM
> regex parser.
> > >>
> > >>> • Use llvm streams instead of lldb::StreamString
> > >>> • Supports output re-targeting (stderr, stdout,
> std::string, etc), printf style formatting, and type-safe streaming
> operators.
> > >>> • Interoperates nicely with many existing llvm
> utility classes
> > >>> • Risk: 4
> > >>> • Impact: 5
> > >>> • Difficulty / Effort: 7
> > >>
> > >> I have worked extensively with both C++ streams and with printf. I
> don't find the type safe streaming operators to be readable and a great way
> to place things into streams. Part of my objection to this will be quelled
> if the LLVM streams are not stateful like C++ streams are (add an extra
> "std::hex" into the stream and suddenly other things down stream start
> coming out in hex). As long as printf functionality is in the LLVM streams
> I am ok with it.
> > >>
> > >>> • Use llvm::Error instead of lldb::Error
> > >>> • llvm::Error is an error class that *requires*
> you to check whether it succeeded or it will assert.  In a way, it's
> similar to a C++ exception, except that it doesn't come with the
> performance hit associated with exceptions.  It's extensible, and can be
> easily extended to support the various ways LLDB needs to construct errors
> and error messages.
> > >>> • Would need to first rename lldb::Error to
> LLDBError so that te conversion from LLDBError to llvm::Error could be done
> incrementally.
> > >>> • Risk: 7
> > >>> • Impact: 7
> > >>> • Difficulty / Effort: 8
> > >>
> > >> We must be very careful here. Nothing can crash LLDB and adding
> something that will be known to crash in some cases can only be changed if
> there is a test that can guarantee that it won't crash. assert() calls in a
> shared library like LLDB are not OK and shouldn't fire. Even if 

Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Greg Clayton via lldb-dev

> On Sep 19, 2016, at 2:46 PM, Mehdi Amini  wrote:
> 
>> 
>> On Sep 19, 2016, at 2:34 PM, Greg Clayton  wrote:
>> 
>>> 
>>> On Sep 19, 2016, at 2:20 PM, Mehdi Amini  wrote:
>>> 
 
 On Sep 19, 2016, at 2:02 PM, Greg Clayton via lldb-dev 
  wrote:
 
 
> On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev 
>  wrote:
> 
> Following up with Kate's post from a few weeks ago, I think the dust has 
> settled on the code reformat and it went over pretty smoothly for the 
> most part.  So I thought it might be worth throwing out some ideas for 
> where we go from here.  I have a large list of ideas (more ideas than 
> time, sadly) that I've been collecting over the past few weeks, so I 
> figured I would throw them out in the open for discussion.
> 
> I’ve grouped the areas for improvement into 3 high level categories.
> 
>   • De-inventing the wheel - We should use more code from LLVM, and 
> delete code in LLDB where LLVM provides a solution.  In cases where there 
> is an LLVM thing that is *similar* to what we need, we should extend the 
> LLVM thing to support what we need, and then use it.  Following are some 
> areas I've identified.  This list is by no means complete.  For each one, 
> I've given a personal assessment of how likely it is to cause some 
> (temporary) hiccups, how much it would help us in the long run, and how 
> difficult it would be to do.  Without further ado:
>   • Use llvm::Regex instead of lldb::Regex
>   • llvm::Regex doesn’t support enhanced mode.  Could we 
> add support for this to llvm::Regex?
>   • Risk: 6
>   • Impact: 3
>   • Difficulty / Effort: 3  (5 if we have to add enhanced 
> mode support)
 
 As long as it supports all of the features, then this is fine. Otherwise 
 we will need to modify the regular expressions to work with the LLVM 
 version or back port any advances regex features we need into the LLVM 
 regex parser. 
 
>   • Use llvm streams instead of lldb::StreamString
>   • Supports output re-targeting (stderr, stdout, 
> std::string, etc), printf style formatting, and type-safe streaming 
> operators.
>   • Interoperates nicely with many existing llvm utility 
> classes
>   • Risk: 4
>   • Impact: 5
>   • Difficulty / Effort: 7
 
 I have worked extensively with both C++ streams and with printf. I don't 
 find the type safe streaming operators to be readable and a great way to 
 place things into streams. Part of my objection to this will be quelled if 
 the LLVM streams are not stateful like C++ streams are (add an extra 
 "std::hex" into the stream and suddenly other things down stream start 
 coming out in hex). As long as printf functionality is in the LLVM streams 
 I am ok with it.
 
>   • Use llvm::Error instead of lldb::Error
>   • llvm::Error is an error class that *requires* you to 
> check whether it succeeded or it will assert.  In a way, it's similar to 
> a C++ exception, except that it doesn't come with the performance hit 
> associated with exceptions.  It's extensible, and can be easily extended 
> to support the various ways LLDB needs to construct errors and error 
> messages.
>   • Would need to first rename lldb::Error to LLDBError 
> so that te conversion from LLDBError to llvm::Error could be done 
> incrementally.
>   • Risk: 7
>   • Impact: 7
>   • Difficulty / Effort: 8
 
 We must be very careful here. Nothing can crash LLDB and adding something 
 that will be known to crash in some cases can only be changed if there is 
 a test that can guarantee that it won't crash. assert() calls in a shared 
 library like LLDB are not OK and shouldn't fire. Even if they do, when 
 asserts are off, then it shouldn't crash LLDB. We have made efforts to 
 only use asserts in LLDB for things that really can't happen, but for 
 things like constructing a StringRef with NULL is one example of why I 
 don't want to use certain classes in LLVM. If we can remove the crash 
 threat, then lets use them. But many people firmly believe that asserts 
 belong in llvm and clang code and I don't agree.
>>> 
>>> I’m surprise by your aversion to assertions, what is your suggested 
>>> alternative? Are you expecting to check and handle every possible 
>>> invariants everywhere and recover (or signal an error) properly? That does 
>>> not seem practical, and it seems to me that it'd lead to just involving UB 
>>> (or breaking design invariant) without actually noticing it.
>> 
>> We have to as a debugger. We ge

Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Zachary Turner via lldb-dev
This kind of philisophical debate is probably worthy of a separate thread
:)  That being said, I think asserts are typically used in places where the
assert firing means "You're going to crash *anyway*"

It's like trying to handle a bad alloc.  What are you going to do anyway?
You can crash now or you can crash later.

It's for guaranteeing the invariants of a function.  If you violate the
invariants of a function, then you might as well be passing null to
strlen(), or aliasing a value through a union, or dereferencing a null
pointer, or making a signed overflow, or accessing an array out of bounds.
It's all equally undefined behavior, so crashing as soon as possible at
least alerts you to the source of the undefined behavior, rather than
trying to hold on as long as possible and then try to reconstruct the crash
dynamics from the crime scene after the fact, so to speak.

The example you gave about
assert(p);
if (p)
  p->crash();

I disagree with.  There are two ways to look at this.  One way to look at
it is "I made my code safer by being defensive.  I don't know why p might
be null, but if it is, at least we won't crash".  Another way to look at it
is "I made my code more complex.  Now it's harder to reason about, harder
to analyze, and harder to test".

Which one is better?  We're in subjective territory, but peraonslly I lean
towards simple = better.  Simple code is easier to test because the are
fewer code paths, and I believe that if a code path isn't tested, then it's
broken.

On Tue, Sep 20, 2016 at 10:33 AM Greg Clayton  wrote:

>
> > On Sep 19, 2016, at 2:46 PM, Mehdi Amini  wrote:
> >
> >>
> >> On Sep 19, 2016, at 2:34 PM, Greg Clayton  wrote:
> >>
> >>>
> >>> On Sep 19, 2016, at 2:20 PM, Mehdi Amini 
> wrote:
> >>>
> 
>  On Sep 19, 2016, at 2:02 PM, Greg Clayton via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> 
> 
> > On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >
> > Following up with Kate's post from a few weeks ago, I think the dust
> has settled on the code reformat and it went over pretty smoothly for the
> most part.  So I thought it might be worth throwing out some ideas for
> where we go from here.  I have a large list of ideas (more ideas than time,
> sadly) that I've been collecting over the past few weeks, so I figured I
> would throw them out in the open for discussion.
> >
> > I’ve grouped the areas for improvement into 3 high level categories.
> >
> >   • De-inventing the wheel - We should use more code from LLVM, and
> delete code in LLDB where LLVM provides a solution.  In cases where there
> is an LLVM thing that is *similar* to what we need, we should extend the
> LLVM thing to support what we need, and then use it.  Following are some
> areas I've identified.  This list is by no means complete.  For each one,
> I've given a personal assessment of how likely it is to cause some
> (temporary) hiccups, how much it would help us in the long run, and how
> difficult it would be to do.  Without further ado:
> >   • Use llvm::Regex instead of lldb::Regex
> >   • llvm::Regex doesn’t support enhanced mode.
> Could we add support for this to llvm::Regex?
> >   • Risk: 6
> >   • Impact: 3
> >   • Difficulty / Effort: 3  (5 if we have to add
> enhanced mode support)
> 
>  As long as it supports all of the features, then this is fine.
> Otherwise we will need to modify the regular expressions to work with the
> LLVM version or back port any advances regex features we need into the LLVM
> regex parser.
> 
> >   • Use llvm streams instead of lldb::StreamString
> >   • Supports output re-targeting (stderr, stdout,
> std::string, etc), printf style formatting, and type-safe streaming
> operators.
> >   • Interoperates nicely with many existing llvm
> utility classes
> >   • Risk: 4
> >   • Impact: 5
> >   • Difficulty / Effort: 7
> 
>  I have worked extensively with both C++ streams and with printf. I
> don't find the type safe streaming operators to be readable and a great way
> to place things into streams. Part of my objection to this will be quelled
> if the LLVM streams are not stateful like C++ streams are (add an extra
> "std::hex" into the stream and suddenly other things down stream start
> coming out in hex). As long as printf functionality is in the LLVM streams
> I am ok with it.
> 
> >   • Use llvm::Error instead of lldb::Error
> >   • llvm::Error is an error class that *requires*
> you to check whether it succeeded or it will assert.  In a way, it's
> similar to a C++ exception, except that it doesn't come with the
> performance hit associated with exceptions.  It's extensible, and can be
> easily extended to support the various 

Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Mehdi Amini via lldb-dev

> On Sep 20, 2016, at 10:33 AM, Greg Clayton  wrote:
> 
>> 
>> On Sep 19, 2016, at 2:46 PM, Mehdi Amini  wrote:
>> 
>>> 
>>> On Sep 19, 2016, at 2:34 PM, Greg Clayton  wrote:
>>> 
 
 On Sep 19, 2016, at 2:20 PM, Mehdi Amini  wrote:
 
> 
> On Sep 19, 2016, at 2:02 PM, Greg Clayton via lldb-dev 
>  wrote:
> 
> 
>> On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev 
>>  wrote:
>> 
>> Following up with Kate's post from a few weeks ago, I think the dust has 
>> settled on the code reformat and it went over pretty smoothly for the 
>> most part.  So I thought it might be worth throwing out some ideas for 
>> where we go from here.  I have a large list of ideas (more ideas than 
>> time, sadly) that I've been collecting over the past few weeks, so I 
>> figured I would throw them out in the open for discussion.
>> 
>> I’ve grouped the areas for improvement into 3 high level categories.
>> 
>>  • De-inventing the wheel - We should use more code from LLVM, and 
>> delete code in LLDB where LLVM provides a solution.  In cases where 
>> there is an LLVM thing that is *similar* to what we need, we should 
>> extend the LLVM thing to support what we need, and then use it.  
>> Following are some areas I've identified.  This list is by no means 
>> complete.  For each one, I've given a personal assessment of how likely 
>> it is to cause some (temporary) hiccups, how much it would help us in 
>> the long run, and how difficult it would be to do.  Without further ado:
>>  • Use llvm::Regex instead of lldb::Regex
>>  • llvm::Regex doesn’t support enhanced mode.  Could we 
>> add support for this to llvm::Regex?
>>  • Risk: 6
>>  • Impact: 3
>>  • Difficulty / Effort: 3  (5 if we have to add enhanced 
>> mode support)
> 
> As long as it supports all of the features, then this is fine. Otherwise 
> we will need to modify the regular expressions to work with the LLVM 
> version or back port any advances regex features we need into the LLVM 
> regex parser. 
> 
>>  • Use llvm streams instead of lldb::StreamString
>>  • Supports output re-targeting (stderr, stdout, 
>> std::string, etc), printf style formatting, and type-safe streaming 
>> operators.
>>  • Interoperates nicely with many existing llvm utility 
>> classes
>>  • Risk: 4
>>  • Impact: 5
>>  • Difficulty / Effort: 7
> 
> I have worked extensively with both C++ streams and with printf. I don't 
> find the type safe streaming operators to be readable and a great way to 
> place things into streams. Part of my objection to this will be quelled 
> if the LLVM streams are not stateful like C++ streams are (add an extra 
> "std::hex" into the stream and suddenly other things down stream start 
> coming out in hex). As long as printf functionality is in the LLVM 
> streams I am ok with it.
> 
>>  • Use llvm::Error instead of lldb::Error
>>  • llvm::Error is an error class that *requires* you to 
>> check whether it succeeded or it will assert.  In a way, it's similar to 
>> a C++ exception, except that it doesn't come with the performance hit 
>> associated with exceptions.  It's extensible, and can be easily extended 
>> to support the various ways LLDB needs to construct errors and error 
>> messages.
>>  • Would need to first rename lldb::Error to LLDBError 
>> so that te conversion from LLDBError to llvm::Error could be done 
>> incrementally.
>>  • Risk: 7
>>  • Impact: 7
>>  • Difficulty / Effort: 8
> 
> We must be very careful here. Nothing can crash LLDB and adding something 
> that will be known to crash in some cases can only be changed if there is 
> a test that can guarantee that it won't crash. assert() calls in a shared 
> library like LLDB are not OK and shouldn't fire. Even if they do, when 
> asserts are off, then it shouldn't crash LLDB. We have made efforts to 
> only use asserts in LLDB for things that really can't happen, but for 
> things like constructing a StringRef with NULL is one example of why I 
> don't want to use certain classes in LLVM. If we can remove the crash 
> threat, then lets use them. But many people firmly believe that asserts 
> belong in llvm and clang code and I don't agree.
 
 I’m surprise by your aversion to assertions, what is your suggested 
 alternative? Are you expecting to check and handle every possible 
 invariants everywhere and recover (or signal an error) properly? That does 
 not seem practical, and it seems to me that 

Re: [lldb-dev] running lldb-mi with LLDB_DISABLE_PYTHON

2016-09-20 Thread Jim Ingham via lldb-dev
If you use the MI, that assumes that the server process is the one doing most 
of the work.  lldb is not meant to be a light-weight program, and if you start 
feeding it a lot of symbol information you may find it taking up too many 
resources to really want to run it on your device.   For device debugging, it 
is more efficient to run lldb-server or some other monitor that speaks the 
gdb-remote protocol on the device and then run lldb on the host.  The 
gdb-remote servers can be much more efficient since they don't directly deal 
with debug or symbol information.  We've done a lot of work both for iOS and 
for Android to make lldb's handling of this way of debugging efficient.  The 
changes we made to the gdb-remote protocol to support this work are documented 
in docs/lldb-gdb-remote.txt, though lldb will also run correctly with a vanilla 
gdb-remote server.

Jim

> On Sep 20, 2016, at 12:57 AM, Chunseok Lee via lldb-dev 
>  wrote:
> 
> Dear lldb dev team,
> 
> I am working on running lldb-mi on arm32 device(like rpi) for remote 
> debugging usage.
> Is there any way to run lldb-mi with python disabled ?
> When building lldb with LLDB_DISABLE_PYTHON, it seems that dataformatter 
> initialization is failed due to the following code in 
> MICmnLLDBDebugger.cpp:59   
> 
> //++
> //
> // MI summary helper routines
> static inline bool MI_add_summary(lldb::SBTypeCategory category,
>   const char *typeName,
>   lldb::SBTypeSummary::FormatCallback cb,
>   uint32_t options, bool regex = false) {
> #if defined(LLDB_DISABLE_PYTHON)
>   return false;
> #else
>   lldb::SBTypeSummary summary =
>   lldb::SBTypeSummary::CreateWithCallback(cb, options);
>   return summary.IsValid()
>  ? category.AddTypeSummary(
>lldb::SBTypeNameSpecifier(typeName, regex), summary)
>  : false;
> #endif
> }
> 
> Thank you.
> 
> BR,
> Chunseok Lee
> 
> -- 
> Where Do We come from? What Are We? Where Are We Going?
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Greg Clayton via lldb-dev

> On Sep 20, 2016, at 10:46 AM, Zachary Turner  wrote:
> 
> This kind of philisophical debate is probably worthy of a separate thread :)  
> That being said, I think asserts are typically used in places where the 
> assert firing means "You're going to crash *anyway*"
> 
> It's like trying to handle a bad alloc.  What are you going to do anyway?  
> You can crash now or you can crash later.  

With a StringRef being init'ed with NULL? You don't need to crash for that. In 
case where this is the only thing that you can do, that might be ok, but there 
are many many places where crashing isn't necessary.

> It's for guaranteeing the invariants of a function.  If you violate the 
> invariants of a function, then you might as well be passing null to strlen(), 
> or aliasing a value through a union, or dereferencing a null pointer, or 
> making a signed overflow, or accessing an array out of bounds.  It's all 
> equally undefined behavior, so crashing as soon as possible at least alerts 
> you to the source of the undefined behavior, rather than trying to hold on as 
> long as possible and then try to reconstruct the crash dynamics from the 
> crime scene after the fact, so to speak.
> 
> The example you gave about
> assert(p);
> if (p)
>   p->crash();
> 
> I disagree with.  There are two ways to look at this.  One way to look at it 
> is "I made my code safer by being defensive.  I don't know why p might be 
> null, but if it is, at least we won't crash".  Another way to look at it is 
> "I made my code more complex.  Now it's harder to reason about, harder to 
> analyze, and harder to test".
> 
> Which one is better?  We're in subjective territory, but peraonslly I lean 
> towards simple = better.  Simple code is easier to test because the are fewer 
> code paths, and I believe that if a code path isn't tested, then it's broken.

My only point is: if you are a library, you can't crash because you are unhappy 
with what you have. What that really means is take it out of process otherwise 
you will crash. I think the code should be as resilient as possible. I just 
moves the onus onto users of your library to figure out what makes you assert 
before you can possibly assert. And if there are rare cases that don't happen 
often, you find out about them when your users crash as they do something that 
causes the problem to be seen for the first time which doesn't make for a great 
experience. If all assertions were documented, then that would be one thing. 
But they aren't. You just run into them and must decipher what it means. 
> 
> On Tue, Sep 20, 2016 at 10:33 AM Greg Clayton  wrote:
> 
> > On Sep 19, 2016, at 2:46 PM, Mehdi Amini  wrote:
> >
> >>
> >> On Sep 19, 2016, at 2:34 PM, Greg Clayton  wrote:
> >>
> >>>
> >>> On Sep 19, 2016, at 2:20 PM, Mehdi Amini  wrote:
> >>>
> 
>  On Sep 19, 2016, at 2:02 PM, Greg Clayton via lldb-dev 
>   wrote:
> 
> 
> > On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev 
> >  wrote:
> >
> > Following up with Kate's post from a few weeks ago, I think the dust 
> > has settled on the code reformat and it went over pretty smoothly for 
> > the most part.  So I thought it might be worth throwing out some ideas 
> > for where we go from here.  I have a large list of ideas (more ideas 
> > than time, sadly) that I've been collecting over the past few weeks, so 
> > I figured I would throw them out in the open for discussion.
> >
> > I’ve grouped the areas for improvement into 3 high level categories.
> >
> >   • De-inventing the wheel - We should use more code from LLVM, and 
> > delete code in LLDB where LLVM provides a solution.  In cases where 
> > there is an LLVM thing that is *similar* to what we need, we should 
> > extend the LLVM thing to support what we need, and then use it.  
> > Following are some areas I've identified.  This list is by no means 
> > complete.  For each one, I've given a personal assessment of how likely 
> > it is to cause some (temporary) hiccups, how much it would help us in 
> > the long run, and how difficult it would be to do.  Without further ado:
> >   • Use llvm::Regex instead of lldb::Regex
> >   • llvm::Regex doesn’t support enhanced mode.  Could 
> > we add support for this to llvm::Regex?
> >   • Risk: 6
> >   • Impact: 3
> >   • Difficulty / Effort: 3  (5 if we have to add 
> > enhanced mode support)
> 
>  As long as it supports all of the features, then this is fine. Otherwise 
>  we will need to modify the regular expressions to work with the LLVM 
>  version or back port any advances regex features we need into the LLVM 
>  regex parser.
> 
> >   • Use llvm streams instead of lldb::StreamString
> >   • Supports output re-targeting (stderr, stdout, 
> > std::string, etc), pri

Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Greg Clayton via lldb-dev

> On Sep 20, 2016, at 10:47 AM, Mehdi Amini  wrote:
> 
>> 
>> On Sep 20, 2016, at 10:33 AM, Greg Clayton  wrote:
>> 
>>> 
>>> On Sep 19, 2016, at 2:46 PM, Mehdi Amini  wrote:
>>> 
 
 On Sep 19, 2016, at 2:34 PM, Greg Clayton  wrote:
 
> 
> On Sep 19, 2016, at 2:20 PM, Mehdi Amini  wrote:
> 
>> 
>> On Sep 19, 2016, at 2:02 PM, Greg Clayton via lldb-dev 
>>  wrote:
>> 
>> 
>>> On Sep 19, 2016, at 1:18 PM, Zachary Turner via lldb-dev 
>>>  wrote:
>>> 
>>> Following up with Kate's post from a few weeks ago, I think the dust 
>>> has settled on the code reformat and it went over pretty smoothly for 
>>> the most part.  So I thought it might be worth throwing out some ideas 
>>> for where we go from here.  I have a large list of ideas (more ideas 
>>> than time, sadly) that I've been collecting over the past few weeks, so 
>>> I figured I would throw them out in the open for discussion.
>>> 
>>> I’ve grouped the areas for improvement into 3 high level categories.
>>> 
>>> • De-inventing the wheel - We should use more code from LLVM, 
>>> and delete code in LLDB where LLVM provides a solution.  In cases where 
>>> there is an LLVM thing that is *similar* to what we need, we should 
>>> extend the LLVM thing to support what we need, and then use it.  
>>> Following are some areas I've identified.  This list is by no means 
>>> complete.  For each one, I've given a personal assessment of how likely 
>>> it is to cause some (temporary) hiccups, how much it would help us in 
>>> the long run, and how difficult it would be to do.  Without further ado:
>>> • Use llvm::Regex instead of lldb::Regex
>>> • llvm::Regex doesn’t support enhanced mode.  
>>> Could we add support for this to llvm::Regex?
>>> • Risk: 6
>>> • Impact: 3
>>> • Difficulty / Effort: 3  (5 if we have to add 
>>> enhanced mode support)
>> 
>> As long as it supports all of the features, then this is fine. Otherwise 
>> we will need to modify the regular expressions to work with the LLVM 
>> version or back port any advances regex features we need into the LLVM 
>> regex parser. 
>> 
>>> • Use llvm streams instead of lldb::StreamString
>>> • Supports output re-targeting (stderr, stdout, 
>>> std::string, etc), printf style formatting, and type-safe streaming 
>>> operators.
>>> • Interoperates nicely with many existing llvm 
>>> utility classes
>>> • Risk: 4
>>> • Impact: 5
>>> • Difficulty / Effort: 7
>> 
>> I have worked extensively with both C++ streams and with printf. I don't 
>> find the type safe streaming operators to be readable and a great way to 
>> place things into streams. Part of my objection to this will be quelled 
>> if the LLVM streams are not stateful like C++ streams are (add an extra 
>> "std::hex" into the stream and suddenly other things down stream start 
>> coming out in hex). As long as printf functionality is in the LLVM 
>> streams I am ok with it.
>> 
>>> • Use llvm::Error instead of lldb::Error
>>> • llvm::Error is an error class that *requires* 
>>> you to check whether it succeeded or it will assert.  In a way, it's 
>>> similar to a C++ exception, except that it doesn't come with the 
>>> performance hit associated with exceptions.  It's extensible, and can 
>>> be easily extended to support the various ways LLDB needs to construct 
>>> errors and error messages.
>>> • Would need to first rename lldb::Error to 
>>> LLDBError so that te conversion from LLDBError to llvm::Error could be 
>>> done incrementally.
>>> • Risk: 7
>>> • Impact: 7
>>> • Difficulty / Effort: 8
>> 
>> We must be very careful here. Nothing can crash LLDB and adding 
>> something that will be known to crash in some cases can only be changed 
>> if there is a test that can guarantee that it won't crash. assert() 
>> calls in a shared library like LLDB are not OK and shouldn't fire. Even 
>> if they do, when asserts are off, then it shouldn't crash LLDB. We have 
>> made efforts to only use asserts in LLDB for things that really can't 
>> happen, but for things like constructing a StringRef with NULL is one 
>> example of why I don't want to use certain classes in LLVM. If we can 
>> remove the crash threat, then lets use them. But many people firmly 
>> believe that asserts belong in llvm and clang code and I don't agree.
> 

Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Ted Woodward via lldb-dev

From: lldb-dev [mailto:lldb-dev-boun...@lists.llvm.org] On Behalf Of Zachary 
Turner via lldb-dev
Sent: Tuesday, September 20, 2016 12:47 PM

> This kind of philisophical debate is probably worthy of a separate thread :)  
> That being said, I think asserts are typically used in places where the 
> assert firing means "You're going to crash *anyway*"

This is emphatically NOT the case.

One of the first tasks I was given when I started at Qualcomm was to fix the 
disassembler for Hexagon. It was a mess - it would assert if the disassembly 
tables couldn't identify an opcode. Maybe that's fine for an assembler, assert 
if you can't generate an opcode, but an unidentified opcode should print a 
warning and move on. It's not a fatal error to disassemble data!

There are other instances of this in llvm. I agree with Greg - libraries 
shouldn't assert.  Send an error back to the caller, and let the caller handle 
it. A typo in my expression that lldb sends to clang shouldn't crash my debug 
session.

--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project


___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Nico Weber via lldb-dev
On Tue, Sep 20, 2016 at 4:37 PM, Ted Woodward via lldb-dev <
lldb-dev@lists.llvm.org> wrote:

>
> From: lldb-dev [mailto:lldb-dev-boun...@lists.llvm.org] On Behalf Of
> Zachary Turner via lldb-dev
> Sent: Tuesday, September 20, 2016 12:47 PM
>
> > This kind of philisophical debate is probably worthy of a separate
> thread :)  That being said, I think asserts are typically used in places
> where the assert firing means "You're going to crash *anyway*"
>
> This is emphatically NOT the case.
>
> One of the first tasks I was given when I started at Qualcomm was to fix
> the disassembler for Hexagon. It was a mess - it would assert if the
> disassembly tables couldn't identify an opcode. Maybe that's fine for an
> assembler, assert if you can't generate an opcode, but an unidentified
> opcode should print a warning and move on. It's not a fatal error to
> disassemble data!
>
> There are other instances of this in llvm. I agree with Greg - libraries
> shouldn't assert.  Send an error back to the caller, and let the caller
> handle it. A typo in my expression that lldb sends to clang shouldn't crash
> my debug session.
>

It depends on what you use the assert for, right? If it's used to document
invariants that you control, then having them crash is ok, since those
should always be true. You shouldn't assert on input you don't control.

So in a public api, you'd do:

my_err my_public_api(int p) {
  if (p < 0 || p > 10) {
return make_err("p must be between 0 and 10");
  internal_fun(p);
}

But then in internal_fun you can do

void internal_fun(int p) {
  assert((p >= 0 && p <= 10) && "must pass correct p");
}


>
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> Linux Foundation Collaborative Project
>
>
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Zachary Turner via lldb-dev
I do agree that asserts are sometimes used improperly.  But who's to say
that the bug was the assert, and not the surrounding code?  For example,
consider this code:

assert(p);
int x = *p;

Should this assert also not be here in library code?  I mean it's obvious
that the program is about to crash if p is invalid.  Asserts should mean
"you're about to invoke undefined behavior", and a crash is *better* than
undefined behavior.  It surfaces the problem so that you can't let it slip
under the radar, and it also alerts you to the point that the UB is
invoked, rather than later.

What about this assert?

assert(ptr);
int x = strlen(ptr);

Surely that assert is ok right?  Do we need to check whether ptr is valid
EVERY SINGLE TIME we invoke strlen, or any other function for that matter?
The code would be a disastrous mess.

If you think you've found an improper assert, the thing to do is raise it
on the proper mailing list and present your case of why you think it's an
improper assert.  asserting is not inherently wrong, but like anything, you
have to use it right.

On Tue, Sep 20, 2016 at 1:37 PM Ted Woodward 
wrote:

>
> From: lldb-dev [mailto:lldb-dev-boun...@lists.llvm.org] On Behalf Of
> Zachary Turner via lldb-dev
> Sent: Tuesday, September 20, 2016 12:47 PM
>
> > This kind of philisophical debate is probably worthy of a separate
> thread :)  That being said, I think asserts are typically used in places
> where the assert firing means "You're going to crash *anyway*"
>
> This is emphatically NOT the case.
>
> One of the first tasks I was given when I started at Qualcomm was to fix
> the disassembler for Hexagon. It was a mess - it would assert if the
> disassembly tables couldn't identify an opcode. Maybe that's fine for an
> assembler, assert if you can't generate an opcode, but an unidentified
> opcode should print a warning and move on. It's not a fatal error to
> disassemble data!
>
> There are other instances of this in llvm. I agree with Greg - libraries
> shouldn't assert.  Send an error back to the caller, and let the caller
> handle it. A typo in my expression that lldb sends to clang shouldn't crash
> my debug session.
>
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> Linux Foundation Collaborative Project
>
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Greg Clayton via lldb-dev

> On Sep 20, 2016, at 1:45 PM, Zachary Turner  wrote:
> 
> I do agree that asserts are sometimes used improperly.  But who's to say that 
> the bug was the assert, and not the surrounding code?  For example, consider 
> this code:
> 
> assert(p);
> int x = *p;

Should be written as:

assert(p);
if (!p)
do_something_correct();
else
int x = *p;

> 
> Should this assert also not be here in library code?  I mean it's obvious 
> that the program is about to crash if p is invalid.  Asserts should mean 
> "you're about to invoke undefined behavior", and a crash is *better* than 
> undefined behavior.  It surfaces the problem so that you can't let it slip 
> under the radar, and it also alerts you to the point that the UB is invoked, 
> rather than later.
> 
> What about this assert?
> 
> assert(ptr);
> int x = strlen(ptr);
> 
> Surely that assert is ok right?  Do we need to check whether ptr is valid 
> EVERY SINGLE TIME we invoke strlen, or any other function for that matter?  
> The code would be a disastrous mess.

Again, check before you call if this is in a shared library! What is so hard 
about that? It is called software that doesn't crash.

assert(ptr)
int x = ptr ? strlen(ptr) : 0;

> 
> If you think you've found an improper assert, the thing to do is raise it on 
> the proper mailing list and present your case of why you think it's an 
> improper assert.  asserting is not inherently wrong, but like anything, you 
> have to use it right.

The code to strlen() is well documented. All code in llvm and clang is not. 
Asserts fire off all the time for reasons the original author and others close 
to the code know about, but when the llvm and clang APIs are used by others, we 
don't know all of these cases. They aren't documented. The only way to find 
them is to crash when you run into them with questionable input. Not acceptable 
for a library.

> 
> On Tue, Sep 20, 2016 at 1:37 PM Ted Woodward  
> wrote:
> 
> From: lldb-dev [mailto:lldb-dev-boun...@lists.llvm.org] On Behalf Of Zachary 
> Turner via lldb-dev
> Sent: Tuesday, September 20, 2016 12:47 PM
> 
> > This kind of philisophical debate is probably worthy of a separate thread 
> > :)  That being said, I think asserts are typically used in places where the 
> > assert firing means "You're going to crash *anyway*"
> 
> This is emphatically NOT the case.
> 
> One of the first tasks I was given when I started at Qualcomm was to fix the 
> disassembler for Hexagon. It was a mess - it would assert if the disassembly 
> tables couldn't identify an opcode. Maybe that's fine for an assembler, 
> assert if you can't generate an opcode, but an unidentified opcode should 
> print a warning and move on. It's not a fatal error to disassemble data!
> 
> There are other instances of this in llvm. I agree with Greg - libraries 
> shouldn't assert.  Send an error back to the caller, and let the caller 
> handle it. A typo in my expression that lldb sends to clang shouldn't crash 
> my debug session.
> 
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
> Linux Foundation Collaborative Project
> 
> 

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Sean Callanan via lldb-dev
My 2¢:

> assert(p);
> int x = *p;
> …

> assert(ptr);
> int x = strlen(ptr);

Both of these should either check for null, be in a situation where p is 
obviously good (e.g., p is data() from a stack-allocated std::vector), or use 
references.  The assertion to my mind is like an admission "I'm not 100% sure, 
so let me crash if I'm wrong..." – if we're making that admission and not doing 
error handling, we're already a little shady.

Sean
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Zachary Turner via lldb-dev
On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:

>
> > On Sep 20, 2016, at 1:45 PM, Zachary Turner  wrote:
> >
> > I do agree that asserts are sometimes used improperly.  But who's to say
> that the bug was the assert, and not the surrounding code?  For example,
> consider this code:
> >
> > assert(p);
> > int x = *p;
>
> Should be written as:
>
> assert(p);
> if (!p)
> do_something_correct();
> else
> int x = *p;
>
> >
> > Should this assert also not be here in library code?  I mean it's
> obvious that the program is about to crash if p is invalid.  Asserts should
> mean "you're about to invoke undefined behavior", and a crash is *better*
> than undefined behavior.  It surfaces the problem so that you can't let it
> slip under the radar, and it also alerts you to the point that the UB is
> invoked, rather than later.
> >
> > What about this assert?
> >
> > assert(ptr);
> > int x = strlen(ptr);
> >
> > Surely that assert is ok right?  Do we need to check whether ptr is
> valid EVERY SINGLE TIME we invoke strlen, or any other function for that
> matter?  The code would be a disastrous mess.
>
> Again, check before you call if this is in a shared library! What is so
> hard about that? It is called software that doesn't crash.
>
> assert(ptr)
> int x = ptr ? strlen(ptr) : 0;
>

I find it hard to believe that you are arguing that you cannot EVER know
ANYTHING about the state of your program.  :-/

This is like arguing that you should run a full heap integrity check every
time you perform a memory write, just to be sure you aren't about to crash.


If you make a std::vector<>, do we need to verify that its internal pointer
is not null before we write to it?   Probably not, right?  Why not?
Because it has a specification of how it works, and it is documented that
you can construct one, you can use it.

It's ok to document how functions work, and it is ok to assume that
functions work the way they claim to work.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Adrian McCarthy via lldb-dev
My concern about this example:

void do_something(foo *p)
{
assert(p);
if (p)
p->crash();
}

Is that by skipping the operation when the pointer is null is that it's not
clear what it should do if it's precondition isn't met.  Sure, it won't
crash, but it's also not going to "do something."  This can lead to corrupt
state and postpones discovery of the bug.

If do_something is a public API, then, yes, you have to decide, document,
and implement what to do if the caller passes in a null pointer.  If it's
an internal API, then the silent elision of the crash merely hides the bug
possibly corrupts the debugger's state.  A corrupt debugging state seems
(to me) at least as bad as an obvious crash to the user.  Crashes are going
to get complained about and investigated.  Silently doing the wrong thing
just wastes everyone's time.

On Tue, Sep 20, 2016 at 1:59 PM, Zachary Turner via lldb-dev <
lldb-dev@lists.llvm.org> wrote:

>
>
> On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:
>
>>
>> > On Sep 20, 2016, at 1:45 PM, Zachary Turner  wrote:
>> >
>> > I do agree that asserts are sometimes used improperly.  But who's to
>> say that the bug was the assert, and not the surrounding code?  For
>> example, consider this code:
>> >
>> > assert(p);
>> > int x = *p;
>>
>> Should be written as:
>>
>> assert(p);
>> if (!p)
>> do_something_correct();
>> else
>> int x = *p;
>>
>> >
>> > Should this assert also not be here in library code?  I mean it's
>> obvious that the program is about to crash if p is invalid.  Asserts should
>> mean "you're about to invoke undefined behavior", and a crash is *better*
>> than undefined behavior.  It surfaces the problem so that you can't let it
>> slip under the radar, and it also alerts you to the point that the UB is
>> invoked, rather than later.
>> >
>> > What about this assert?
>> >
>> > assert(ptr);
>> > int x = strlen(ptr);
>> >
>> > Surely that assert is ok right?  Do we need to check whether ptr is
>> valid EVERY SINGLE TIME we invoke strlen, or any other function for that
>> matter?  The code would be a disastrous mess.
>>
>> Again, check before you call if this is in a shared library! What is so
>> hard about that? It is called software that doesn't crash.
>>
>> assert(ptr)
>> int x = ptr ? strlen(ptr) : 0;
>>
>
> I find it hard to believe that you are arguing that you cannot EVER know
> ANYTHING about the state of your program.  :-/
>
> This is like arguing that you should run a full heap integrity check every
> time you perform a memory write, just to be sure you aren't about to crash.
>
>
> If you make a std::vector<>, do we need to verify that its internal
> pointer is not null before we write to it?   Probably not, right?  Why
> not?  Because it has a specification of how it works, and it is documented
> that you can construct one, you can use it.
>
> It's ok to document how functions work, and it is ok to assume that
> functions work the way they claim to work.
>
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Greg Clayton via lldb-dev
Again, strlen is a stupid example as it is well documented. All of llvm and 
clang are not.
> On Sep 20, 2016, at 1:59 PM, Zachary Turner  wrote:
> 
> 
> 
> On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:
> 
> > On Sep 20, 2016, at 1:45 PM, Zachary Turner  wrote:
> >
> > I do agree that asserts are sometimes used improperly.  But who's to say 
> > that the bug was the assert, and not the surrounding code?  For example, 
> > consider this code:
> >
> > assert(p);
> > int x = *p;
> 
> Should be written as:
> 
> assert(p);
> if (!p)
> do_something_correct();
> else
> int x = *p;
> 
> >
> > Should this assert also not be here in library code?  I mean it's obvious 
> > that the program is about to crash if p is invalid.  Asserts should mean 
> > "you're about to invoke undefined behavior", and a crash is *better* than 
> > undefined behavior.  It surfaces the problem so that you can't let it slip 
> > under the radar, and it also alerts you to the point that the UB is 
> > invoked, rather than later.
> >
> > What about this assert?
> >
> > assert(ptr);
> > int x = strlen(ptr);
> >
> > Surely that assert is ok right?  Do we need to check whether ptr is valid 
> > EVERY SINGLE TIME we invoke strlen, or any other function for that matter?  
> > The code would be a disastrous mess.
> 
> Again, check before you call if this is in a shared library! What is so hard 
> about that? It is called software that doesn't crash.
> 
> assert(ptr)
> int x = ptr ? strlen(ptr) : 0;
> 
> I find it hard to believe that you are arguing that you cannot EVER know 
> ANYTHING about the state of your program.  :-/
> 
> This is like arguing that you should run a full heap integrity check every 
> time you perform a memory write, just to be sure you aren't about to crash.  
> 
> If you make a std::vector<>, do we need to verify that its internal pointer 
> is not null before we write to it?   Probably not, right?  Why not?  Because 
> it has a specification of how it works, and it is documented that you can 
> construct one, you can use it.
> 
> It's ok to document how functions work, and it is ok to assume that functions 
> work the way they claim to work.

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Ed Maste via lldb-dev
On 19 September 2016 at 16:18, Zachary Turner via lldb-dev
 wrote:
>
> De-inventing the wheel - We should use more code from LLVM, and delete code
> in LLDB where LLVM provides a solution.

Another example of duplicated code is the debug info parsing (LLDB
source/Plugins/SymbolFile/DWARF vs LLVM lib/DebugInfo/DWARF). I took a
look at trying to rationalize the two when I first started working
with LLDB and it looked like a large task then, and they've only
continued to diverge.

However, diffs between the two trees are now at least not cluttered
with whitespace and formatting differences. I'll try to take another
look at these.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Greg Clayton via lldb-dev
A better solution would be to return an error indicating what went wrong with 
llvm::Error. 

I really can't believe people are ok with "well you called me with the wrong 
parameters that aren't documented, I am unhappy and will crash your program" 
mentality. 

> On Sep 20, 2016, at 2:11 PM, Adrian McCarthy  wrote:
> 
> My concern about this example:
> 
> void do_something(foo *p)
> {
> assert(p);
> if (p)
> p->crash();
> }
> 
> Is that by skipping the operation when the pointer is null is that it's not 
> clear what it should do if it's precondition isn't met.  Sure, it won't 
> crash, but it's also not going to "do something."  This can lead to corrupt 
> state and postpones discovery of the bug.
> 
> If do_something is a public API, then, yes, you have to decide, document, and 
> implement what to do if the caller passes in a null pointer.  If it's an 
> internal API, then the silent elision of the crash merely hides the bug 
> possibly corrupts the debugger's state.  A corrupt debugging state seems (to 
> me) at least as bad as an obvious crash to the user.  Crashes are going to 
> get complained about and investigated.  Silently doing the wrong thing just 
> wastes everyone's time.
> 
> On Tue, Sep 20, 2016 at 1:59 PM, Zachary Turner via lldb-dev 
>  wrote:
> 
> 
> On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:
> 
> > On Sep 20, 2016, at 1:45 PM, Zachary Turner  wrote:
> >
> > I do agree that asserts are sometimes used improperly.  But who's to say 
> > that the bug was the assert, and not the surrounding code?  For example, 
> > consider this code:
> >
> > assert(p);
> > int x = *p;
> 
> Should be written as:
> 
> assert(p);
> if (!p)
> do_something_correct();
> else
> int x = *p;
> 
> >
> > Should this assert also not be here in library code?  I mean it's obvious 
> > that the program is about to crash if p is invalid.  Asserts should mean 
> > "you're about to invoke undefined behavior", and a crash is *better* than 
> > undefined behavior.  It surfaces the problem so that you can't let it slip 
> > under the radar, and it also alerts you to the point that the UB is 
> > invoked, rather than later.
> >
> > What about this assert?
> >
> > assert(ptr);
> > int x = strlen(ptr);
> >
> > Surely that assert is ok right?  Do we need to check whether ptr is valid 
> > EVERY SINGLE TIME we invoke strlen, or any other function for that matter?  
> > The code would be a disastrous mess.
> 
> Again, check before you call if this is in a shared library! What is so hard 
> about that? It is called software that doesn't crash.
> 
> assert(ptr)
> int x = ptr ? strlen(ptr) : 0;
> 
> I find it hard to believe that you are arguing that you cannot EVER know 
> ANYTHING about the state of your program.  :-/
> 
> This is like arguing that you should run a full heap integrity check every 
> time you perform a memory write, just to be sure you aren't about to crash.  
> 
> If you make a std::vector<>, do we need to verify that its internal pointer 
> is not null before we write to it?   Probably not, right?  Why not?  Because 
> it has a specification of how it works, and it is documented that you can 
> construct one, you can use it.
> 
> It's ok to document how functions work, and it is ok to assume that functions 
> work the way they claim to work.
> 
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
> 
> 

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Zachary Turner via lldb-dev
Well, but StringRef for example is well documented.  So it seems to me like
there's an example of a perfectly used assert.  It's documented that you
can't use null, and if you do it asserts.  Just like strlen.

The issue I have with "you can't ever assert" is that it brings it into an
absolute when it really shouldn't be.  We already agreed (I think) that
certain things that are well documented can assert.  But when we talk in
absolutes, it tends to sway people that they should always do that thing,
even when it's not the most appropriate solution.  And I think some of that
shows in the LLDB codebase where you've got hugely complicated logic that
is very hard to follow, reason about, or test, because no assumptions are
ever made about any of the inputs.  Even when they are internal inputs that
are entirely controlled by us.

On Tue, Sep 20, 2016 at 2:19 PM Greg Clayton  wrote:

> Again, strlen is a stupid example as it is well documented. All of llvm
> and clang are not.
> > On Sep 20, 2016, at 1:59 PM, Zachary Turner  wrote:
> >
> >
> >
> > On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:
> >
> > > On Sep 20, 2016, at 1:45 PM, Zachary Turner 
> wrote:
> > >
> > > I do agree that asserts are sometimes used improperly.  But who's to
> say that the bug was the assert, and not the surrounding code?  For
> example, consider this code:
> > >
> > > assert(p);
> > > int x = *p;
> >
> > Should be written as:
> >
> > assert(p);
> > if (!p)
> > do_something_correct();
> > else
> > int x = *p;
> >
> > >
> > > Should this assert also not be here in library code?  I mean it's
> obvious that the program is about to crash if p is invalid.  Asserts should
> mean "you're about to invoke undefined behavior", and a crash is *better*
> than undefined behavior.  It surfaces the problem so that you can't let it
> slip under the radar, and it also alerts you to the point that the UB is
> invoked, rather than later.
> > >
> > > What about this assert?
> > >
> > > assert(ptr);
> > > int x = strlen(ptr);
> > >
> > > Surely that assert is ok right?  Do we need to check whether ptr is
> valid EVERY SINGLE TIME we invoke strlen, or any other function for that
> matter?  The code would be a disastrous mess.
> >
> > Again, check before you call if this is in a shared library! What is so
> hard about that? It is called software that doesn't crash.
> >
> > assert(ptr)
> > int x = ptr ? strlen(ptr) : 0;
> >
> > I find it hard to believe that you are arguing that you cannot EVER know
> ANYTHING about the state of your program.  :-/
> >
> > This is like arguing that you should run a full heap integrity check
> every time you perform a memory write, just to be sure you aren't about to
> crash.
> >
> > If you make a std::vector<>, do we need to verify that its internal
> pointer is not null before we write to it?   Probably not, right?  Why
> not?  Because it has a specification of how it works, and it is documented
> that you can construct one, you can use it.
> >
> > It's ok to document how functions work, and it is ok to assume that
> functions work the way they claim to work.
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Greg Clayton via lldb-dev

> On Sep 20, 2016, at 2:21 PM, Ed Maste via lldb-dev  
> wrote:
> 
> On 19 September 2016 at 16:18, Zachary Turner via lldb-dev
>  wrote:
>> 
>> De-inventing the wheel - We should use more code from LLVM, and delete code
>> in LLDB where LLVM provides a solution.
> 
> Another example of duplicated code is the debug info parsing (LLDB
> source/Plugins/SymbolFile/DWARF vs LLVM lib/DebugInfo/DWARF). I took a
> look at trying to rationalize the two when I first started working
> with LLDB and it looked like a large task then, and they've only
> continued to diverge.

Well the DWARF code was copied from LLDB into LLVM. The original person didn't 
update LLDB to use it, so things diverged. 

> However, diffs between the two trees are now at least not cluttered
> with whitespace and formatting differences. I'll try to take another
> look at these.

I am actually in the process of fixing all of this as we speak, so don't do any 
work on the DWARF parser. It will all be fixed in the next month or so!

Greg

> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Zachary Turner via lldb-dev
I don't think anyone is ok with that.  I just think that a better solution
is to document them.  Why handle at runtime what is known about at compile
time?

On Tue, Sep 20, 2016 at 2:24 PM Zachary Turner  wrote:

> Well, but StringRef for example is well documented.  So it seems to me
> like there's an example of a perfectly used assert.  It's documented that
> you can't use null, and if you do it asserts.  Just like strlen.
>
> The issue I have with "you can't ever assert" is that it brings it into an
> absolute when it really shouldn't be.  We already agreed (I think) that
> certain things that are well documented can assert.  But when we talk in
> absolutes, it tends to sway people that they should always do that thing,
> even when it's not the most appropriate solution.  And I think some of that
> shows in the LLDB codebase where you've got hugely complicated logic that
> is very hard to follow, reason about, or test, because no assumptions are
> ever made about any of the inputs.  Even when they are internal inputs that
> are entirely controlled by us.
>
> On Tue, Sep 20, 2016 at 2:19 PM Greg Clayton  wrote:
>
> Again, strlen is a stupid example as it is well documented. All of llvm
> and clang are not.
> > On Sep 20, 2016, at 1:59 PM, Zachary Turner  wrote:
> >
> >
> >
> > On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:
> >
> > > On Sep 20, 2016, at 1:45 PM, Zachary Turner 
> wrote:
> > >
> > > I do agree that asserts are sometimes used improperly.  But who's to
> say that the bug was the assert, and not the surrounding code?  For
> example, consider this code:
> > >
> > > assert(p);
> > > int x = *p;
> >
> > Should be written as:
> >
> > assert(p);
> > if (!p)
> > do_something_correct();
> > else
> > int x = *p;
> >
> > >
> > > Should this assert also not be here in library code?  I mean it's
> obvious that the program is about to crash if p is invalid.  Asserts should
> mean "you're about to invoke undefined behavior", and a crash is *better*
> than undefined behavior.  It surfaces the problem so that you can't let it
> slip under the radar, and it also alerts you to the point that the UB is
> invoked, rather than later.
> > >
> > > What about this assert?
> > >
> > > assert(ptr);
> > > int x = strlen(ptr);
> > >
> > > Surely that assert is ok right?  Do we need to check whether ptr is
> valid EVERY SINGLE TIME we invoke strlen, or any other function for that
> matter?  The code would be a disastrous mess.
> >
> > Again, check before you call if this is in a shared library! What is so
> hard about that? It is called software that doesn't crash.
> >
> > assert(ptr)
> > int x = ptr ? strlen(ptr) : 0;
> >
> > I find it hard to believe that you are arguing that you cannot EVER know
> ANYTHING about the state of your program.  :-/
> >
> > This is like arguing that you should run a full heap integrity check
> every time you perform a memory write, just to be sure you aren't about to
> crash.
> >
> > If you make a std::vector<>, do we need to verify that its internal
> pointer is not null before we write to it?   Probably not, right?  Why
> not?  Because it has a specification of how it works, and it is documented
> that you can construct one, you can use it.
> >
> > It's ok to document how functions work, and it is ok to assume that
> functions work the way they claim to work.
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Ed Maste via lldb-dev
On 20 September 2016 at 17:25, Greg Clayton  wrote:
>
> Well the DWARF code was copied from LLDB into LLVM. The original person 
> didn't update LLDB to use it, so things diverged.

Yeah, sorry I didn't mention that explicitly. I do recall someone
pointed that out when this came up previously.

> I am actually in the process of fixing all of this as we speak, so don't do 
> any work on the DWARF parser. It will all be fixed in the next month or so!

Most excellent, thank you.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Mehdi Amini via lldb-dev

> On Sep 20, 2016, at 2:19 PM, Greg Clayton  wrote:
> 
> Again, strlen is a stupid example as it is well documented. All of llvm and 
> clang are not.

IMO that is:

1) A free claim that is easily defeated (to prove you wrong on *all* of LLVM 
being not document I just have to point you to one example where it is).
2) Not productive or constructive attitude. I’m not sure where you go with that…

—
Mehdi

>> On Sep 20, 2016, at 1:59 PM, Zachary Turner  wrote:
>> 
>> 
>> 
>> On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:
>> 
>>> On Sep 20, 2016, at 1:45 PM, Zachary Turner  wrote:
>>> 
>>> I do agree that asserts are sometimes used improperly.  But who's to say 
>>> that the bug was the assert, and not the surrounding code?  For example, 
>>> consider this code:
>>> 
>>> assert(p);
>>> int x = *p;
>> 
>> Should be written as:
>> 
>> assert(p);
>> if (!p)
>>do_something_correct();
>> else
>>int x = *p;
>> 
>>> 
>>> Should this assert also not be here in library code?  I mean it's obvious 
>>> that the program is about to crash if p is invalid.  Asserts should mean 
>>> "you're about to invoke undefined behavior", and a crash is *better* than 
>>> undefined behavior.  It surfaces the problem so that you can't let it slip 
>>> under the radar, and it also alerts you to the point that the UB is 
>>> invoked, rather than later.
>>> 
>>> What about this assert?
>>> 
>>> assert(ptr);
>>> int x = strlen(ptr);
>>> 
>>> Surely that assert is ok right?  Do we need to check whether ptr is valid 
>>> EVERY SINGLE TIME we invoke strlen, or any other function for that matter?  
>>> The code would be a disastrous mess.
>> 
>> Again, check before you call if this is in a shared library! What is so hard 
>> about that? It is called software that doesn't crash.
>> 
>> assert(ptr)
>> int x = ptr ? strlen(ptr) : 0;
>> 
>> I find it hard to believe that you are arguing that you cannot EVER know 
>> ANYTHING about the state of your program.  :-/
>> 
>> This is like arguing that you should run a full heap integrity check every 
>> time you perform a memory write, just to be sure you aren't about to crash.  
>> 
>> If you make a std::vector<>, do we need to verify that its internal pointer 
>> is not null before we write to it?   Probably not, right?  Why not?  Because 
>> it has a specification of how it works, and it is documented that you can 
>> construct one, you can use it.
>> 
>> It's ok to document how functions work, and it is ok to assume that 
>> functions work the way they claim to work.
> 

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Greg Clayton via lldb-dev
We should avoid crashing if there is a reasonable work around when the input is 
bad. StringRef with NULL is easy, just put NULL and zero length and don't 
crash. Just because it is documented, doesn't mean it needs to stay that way, 
but I am not going to fight that battle.

We should make every effort to not crash if we can. If it is way too difficult, 
document the issue and make sure clients know that this is the way things are. 
StringRef does this and we accept it. Doesn't mean it won't crash us. I just 
hate seeing the crash logs where we have:

StringRef s(die.GetName());

It shouldn't crash IMHO, but we know it does and we now code around it. Yes, we 
put in code like:

StringRef s;
const char *cstr = die.GetName();
if (cstr)
s = cstr;

Is this nice code? I am glad it makes it simple for the LLVM side, but I would 
rather write:

StringRef s(die.GetName());

Maybe I will subclass llvm::StringRef as lldb::StringRef and override the 
constructor.


> On Sep 20, 2016, at 2:24 PM, Zachary Turner  wrote:
> 
> Well, but StringRef for example is well documented.  So it seems to me like 
> there's an example of a perfectly used assert.  It's documented that you 
> can't use null, and if you do it asserts.  Just like strlen.
> 
> The issue I have with "you can't ever assert" is that it brings it into an 
> absolute when it really shouldn't be.  We already agreed (I think) that 
> certain things that are well documented can assert.  But when we talk in 
> absolutes, it tends to sway people that they should always do that thing, 
> even when it's not the most appropriate solution.  And I think some of that 
> shows in the LLDB codebase where you've got hugely complicated logic that is 
> very hard to follow, reason about, or test, because no assumptions are ever 
> made about any of the inputs.  Even when they are internal inputs that are 
> entirely controlled by us.
> 
> On Tue, Sep 20, 2016 at 2:19 PM Greg Clayton  wrote:
> Again, strlen is a stupid example as it is well documented. All of llvm and 
> clang are not.
> > On Sep 20, 2016, at 1:59 PM, Zachary Turner  wrote:
> >
> >
> >
> > On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:
> >
> > > On Sep 20, 2016, at 1:45 PM, Zachary Turner  wrote:
> > >
> > > I do agree that asserts are sometimes used improperly.  But who's to say 
> > > that the bug was the assert, and not the surrounding code?  For example, 
> > > consider this code:
> > >
> > > assert(p);
> > > int x = *p;
> >
> > Should be written as:
> >
> > assert(p);
> > if (!p)
> > do_something_correct();
> > else
> > int x = *p;
> >
> > >
> > > Should this assert also not be here in library code?  I mean it's obvious 
> > > that the program is about to crash if p is invalid.  Asserts should mean 
> > > "you're about to invoke undefined behavior", and a crash is *better* than 
> > > undefined behavior.  It surfaces the problem so that you can't let it 
> > > slip under the radar, and it also alerts you to the point that the UB is 
> > > invoked, rather than later.
> > >
> > > What about this assert?
> > >
> > > assert(ptr);
> > > int x = strlen(ptr);
> > >
> > > Surely that assert is ok right?  Do we need to check whether ptr is valid 
> > > EVERY SINGLE TIME we invoke strlen, or any other function for that 
> > > matter?  The code would be a disastrous mess.
> >
> > Again, check before you call if this is in a shared library! What is so 
> > hard about that? It is called software that doesn't crash.
> >
> > assert(ptr)
> > int x = ptr ? strlen(ptr) : 0;
> >
> > I find it hard to believe that you are arguing that you cannot EVER know 
> > ANYTHING about the state of your program.  :-/
> >
> > This is like arguing that you should run a full heap integrity check every 
> > time you perform a memory write, just to be sure you aren't about to crash.
> >
> > If you make a std::vector<>, do we need to verify that its internal pointer 
> > is not null before we write to it?   Probably not, right?  Why not?  
> > Because it has a specification of how it works, and it is documented that 
> > you can construct one, you can use it.
> >
> > It's ok to document how functions work, and it is ok to assume that 
> > functions work the way they claim to work.
> 

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Zachary Turner via lldb-dev
StringRef has `withNullAsEmpty` which I added a few days ago.  It will
return an empty StringRef.  seems to me that should solve most of those
kinds of problems.

On Tue, Sep 20, 2016 at 2:31 PM Greg Clayton  wrote:

> We should avoid crashing if there is a reasonable work around when the
> input is bad. StringRef with NULL is easy, just put NULL and zero length
> and don't crash. Just because it is documented, doesn't mean it needs to
> stay that way, but I am not going to fight that battle.
>
> We should make every effort to not crash if we can. If it is way too
> difficult, document the issue and make sure clients know that this is the
> way things are. StringRef does this and we accept it. Doesn't mean it won't
> crash us. I just hate seeing the crash logs where we have:
>
> StringRef s(die.GetName());
>
> It shouldn't crash IMHO, but we know it does and we now code around it.
> Yes, we put in code like:
>
> StringRef s;
> const char *cstr = die.GetName();
> if (cstr)
> s = cstr;
>
> Is this nice code? I am glad it makes it simple for the LLVM side, but I
> would rather write:
>
> StringRef s(die.GetName());
>
> Maybe I will subclass llvm::StringRef as lldb::StringRef and override the
> constructor.
>
>
> > On Sep 20, 2016, at 2:24 PM, Zachary Turner  wrote:
> >
> > Well, but StringRef for example is well documented.  So it seems to me
> like there's an example of a perfectly used assert.  It's documented that
> you can't use null, and if you do it asserts.  Just like strlen.
> >
> > The issue I have with "you can't ever assert" is that it brings it into
> an absolute when it really shouldn't be.  We already agreed (I think) that
> certain things that are well documented can assert.  But when we talk in
> absolutes, it tends to sway people that they should always do that thing,
> even when it's not the most appropriate solution.  And I think some of that
> shows in the LLDB codebase where you've got hugely complicated logic that
> is very hard to follow, reason about, or test, because no assumptions are
> ever made about any of the inputs.  Even when they are internal inputs that
> are entirely controlled by us.
> >
> > On Tue, Sep 20, 2016 at 2:19 PM Greg Clayton  wrote:
> > Again, strlen is a stupid example as it is well documented. All of llvm
> and clang are not.
> > > On Sep 20, 2016, at 1:59 PM, Zachary Turner 
> wrote:
> > >
> > >
> > >
> > > On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton 
> wrote:
> > >
> > > > On Sep 20, 2016, at 1:45 PM, Zachary Turner 
> wrote:
> > > >
> > > > I do agree that asserts are sometimes used improperly.  But who's to
> say that the bug was the assert, and not the surrounding code?  For
> example, consider this code:
> > > >
> > > > assert(p);
> > > > int x = *p;
> > >
> > > Should be written as:
> > >
> > > assert(p);
> > > if (!p)
> > > do_something_correct();
> > > else
> > > int x = *p;
> > >
> > > >
> > > > Should this assert also not be here in library code?  I mean it's
> obvious that the program is about to crash if p is invalid.  Asserts should
> mean "you're about to invoke undefined behavior", and a crash is *better*
> than undefined behavior.  It surfaces the problem so that you can't let it
> slip under the radar, and it also alerts you to the point that the UB is
> invoked, rather than later.
> > > >
> > > > What about this assert?
> > > >
> > > > assert(ptr);
> > > > int x = strlen(ptr);
> > > >
> > > > Surely that assert is ok right?  Do we need to check whether ptr is
> valid EVERY SINGLE TIME we invoke strlen, or any other function for that
> matter?  The code would be a disastrous mess.
> > >
> > > Again, check before you call if this is in a shared library! What is
> so hard about that? It is called software that doesn't crash.
> > >
> > > assert(ptr)
> > > int x = ptr ? strlen(ptr) : 0;
> > >
> > > I find it hard to believe that you are arguing that you cannot EVER
> know ANYTHING about the state of your program.  :-/
> > >
> > > This is like arguing that you should run a full heap integrity check
> every time you perform a memory write, just to be sure you aren't about to
> crash.
> > >
> > > If you make a std::vector<>, do we need to verify that its internal
> pointer is not null before we write to it?   Probably not, right?  Why
> not?  Because it has a specification of how it works, and it is documented
> that you can construct one, you can use it.
> > >
> > > It's ok to document how functions work, and it is ok to assume that
> functions work the way they claim to work.
> >
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Zachary Turner via lldb-dev
Also, maybe that code could just return a StringRef.  It's like I mentioned
a few days ago (don't remember if it was this thread or another), but when
you've got StringRefs all the way down, this problem pretty much disappears.

On Tue, Sep 20, 2016 at 2:36 PM Zachary Turner  wrote:

> StringRef has `withNullAsEmpty` which I added a few days ago.  It will
> return an empty StringRef.  seems to me that should solve most of those
> kinds of problems.
>
> On Tue, Sep 20, 2016 at 2:31 PM Greg Clayton  wrote:
>
> We should avoid crashing if there is a reasonable work around when the
> input is bad. StringRef with NULL is easy, just put NULL and zero length
> and don't crash. Just because it is documented, doesn't mean it needs to
> stay that way, but I am not going to fight that battle.
>
> We should make every effort to not crash if we can. If it is way too
> difficult, document the issue and make sure clients know that this is the
> way things are. StringRef does this and we accept it. Doesn't mean it won't
> crash us. I just hate seeing the crash logs where we have:
>
> StringRef s(die.GetName());
>
> It shouldn't crash IMHO, but we know it does and we now code around it.
> Yes, we put in code like:
>
> StringRef s;
> const char *cstr = die.GetName();
> if (cstr)
> s = cstr;
>
> Is this nice code? I am glad it makes it simple for the LLVM side, but I
> would rather write:
>
> StringRef s(die.GetName());
>
> Maybe I will subclass llvm::StringRef as lldb::StringRef and override the
> constructor.
>
>
> > On Sep 20, 2016, at 2:24 PM, Zachary Turner  wrote:
> >
> > Well, but StringRef for example is well documented.  So it seems to me
> like there's an example of a perfectly used assert.  It's documented that
> you can't use null, and if you do it asserts.  Just like strlen.
> >
> > The issue I have with "you can't ever assert" is that it brings it into
> an absolute when it really shouldn't be.  We already agreed (I think) that
> certain things that are well documented can assert.  But when we talk in
> absolutes, it tends to sway people that they should always do that thing,
> even when it's not the most appropriate solution.  And I think some of that
> shows in the LLDB codebase where you've got hugely complicated logic that
> is very hard to follow, reason about, or test, because no assumptions are
> ever made about any of the inputs.  Even when they are internal inputs that
> are entirely controlled by us.
> >
> > On Tue, Sep 20, 2016 at 2:19 PM Greg Clayton  wrote:
> > Again, strlen is a stupid example as it is well documented. All of llvm
> and clang are not.
> > > On Sep 20, 2016, at 1:59 PM, Zachary Turner 
> wrote:
> > >
> > >
> > >
> > > On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton 
> wrote:
> > >
> > > > On Sep 20, 2016, at 1:45 PM, Zachary Turner 
> wrote:
> > > >
> > > > I do agree that asserts are sometimes used improperly.  But who's to
> say that the bug was the assert, and not the surrounding code?  For
> example, consider this code:
> > > >
> > > > assert(p);
> > > > int x = *p;
> > >
> > > Should be written as:
> > >
> > > assert(p);
> > > if (!p)
> > > do_something_correct();
> > > else
> > > int x = *p;
> > >
> > > >
> > > > Should this assert also not be here in library code?  I mean it's
> obvious that the program is about to crash if p is invalid.  Asserts should
> mean "you're about to invoke undefined behavior", and a crash is *better*
> than undefined behavior.  It surfaces the problem so that you can't let it
> slip under the radar, and it also alerts you to the point that the UB is
> invoked, rather than later.
> > > >
> > > > What about this assert?
> > > >
> > > > assert(ptr);
> > > > int x = strlen(ptr);
> > > >
> > > > Surely that assert is ok right?  Do we need to check whether ptr is
> valid EVERY SINGLE TIME we invoke strlen, or any other function for that
> matter?  The code would be a disastrous mess.
> > >
> > > Again, check before you call if this is in a shared library! What is
> so hard about that? It is called software that doesn't crash.
> > >
> > > assert(ptr)
> > > int x = ptr ? strlen(ptr) : 0;
> > >
> > > I find it hard to believe that you are arguing that you cannot EVER
> know ANYTHING about the state of your program.  :-/
> > >
> > > This is like arguing that you should run a full heap integrity check
> every time you perform a memory write, just to be sure you aren't about to
> crash.
> > >
> > > If you make a std::vector<>, do we need to verify that its internal
> pointer is not null before we write to it?   Probably not, right?  Why
> not?  Because it has a specification of how it works, and it is documented
> that you can construct one, you can use it.
> > >
> > > It's ok to document how functions work, and it is ok to assume that
> functions work the way they claim to work.
> >
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Mehdi Amini via lldb-dev

> On Sep 20, 2016, at 2:31 PM, Greg Clayton  wrote:
> 
> We should avoid crashing if there is a reasonable work around when the input 
> is bad. StringRef with NULL is easy, just put NULL and zero length and don't 
> crash. Just because it is documented, doesn't mean it needs to stay that way, 
> but I am not going to fight that battle.
> 
> We should make every effort to not crash if we can. If it is way too 
> difficult, document the issue and make sure clients know that this is the way 
> things are. StringRef does this and we accept it. Doesn't mean it won't crash 
> us. I just hate seeing the crash logs where we have:
> 
> StringRef s(die.GetName());
> 
> It shouldn't crash IMHO, but we know it does and we now code around it. Yes, 
> we put in code like:
> 
> StringRef s;
> const char *cstr = die.GetName();
> if (cstr)
>s = cstr;
> 
> Is this nice code?

Well no, it is indeed terrible: changing die.GetName() to return StringRef make 
the code a lot safer.

When StringRef is used everywhere, this problem disappear by itself. 
StringRef(const char *) is a very unsafe API, that is not intended to be used 
*inside* the library (or very sporadically). So if you build a StringRef out of 
a const char *, you’re supposed to be in a place that deals with “uncontrolled” 
input and you need to sanitize it. 

(Also StringRef(const char *) is “costly": it calls strlen)

— 
Mehdi



> I am glad it makes it simple for the LLVM side, but I would rather write:
> 
> StringRef s(die.GetName());
> 
> Maybe I will subclass llvm::StringRef as lldb::StringRef and override the 
> constructor.
> 
> 
>> On Sep 20, 2016, at 2:24 PM, Zachary Turner  wrote:
>> 
>> Well, but StringRef for example is well documented.  So it seems to me like 
>> there's an example of a perfectly used assert.  It's documented that you 
>> can't use null, and if you do it asserts.  Just like strlen.
>> 
>> The issue I have with "you can't ever assert" is that it brings it into an 
>> absolute when it really shouldn't be.  We already agreed (I think) that 
>> certain things that are well documented can assert.  But when we talk in 
>> absolutes, it tends to sway people that they should always do that thing, 
>> even when it's not the most appropriate solution.  And I think some of that 
>> shows in the LLDB codebase where you've got hugely complicated logic that is 
>> very hard to follow, reason about, or test, because no assumptions are ever 
>> made about any of the inputs.  Even when they are internal inputs that are 
>> entirely controlled by us.
>> 
>> On Tue, Sep 20, 2016 at 2:19 PM Greg Clayton  wrote:
>> Again, strlen is a stupid example as it is well documented. All of llvm and 
>> clang are not.
>>> On Sep 20, 2016, at 1:59 PM, Zachary Turner  wrote:
>>> 
>>> 
>>> 
>>> On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:
>>> 
 On Sep 20, 2016, at 1:45 PM, Zachary Turner  wrote:
 
 I do agree that asserts are sometimes used improperly.  But who's to say 
 that the bug was the assert, and not the surrounding code?  For example, 
 consider this code:
 
 assert(p);
 int x = *p;
>>> 
>>> Should be written as:
>>> 
>>> assert(p);
>>> if (!p)
>>>do_something_correct();
>>> else
>>>int x = *p;
>>> 
 
 Should this assert also not be here in library code?  I mean it's obvious 
 that the program is about to crash if p is invalid.  Asserts should mean 
 "you're about to invoke undefined behavior", and a crash is *better* than 
 undefined behavior.  It surfaces the problem so that you can't let it slip 
 under the radar, and it also alerts you to the point that the UB is 
 invoked, rather than later.
 
 What about this assert?
 
 assert(ptr);
 int x = strlen(ptr);
 
 Surely that assert is ok right?  Do we need to check whether ptr is valid 
 EVERY SINGLE TIME we invoke strlen, or any other function for that matter? 
  The code would be a disastrous mess.
>>> 
>>> Again, check before you call if this is in a shared library! What is so 
>>> hard about that? It is called software that doesn't crash.
>>> 
>>> assert(ptr)
>>> int x = ptr ? strlen(ptr) : 0;
>>> 
>>> I find it hard to believe that you are arguing that you cannot EVER know 
>>> ANYTHING about the state of your program.  :-/
>>> 
>>> This is like arguing that you should run a full heap integrity check every 
>>> time you perform a memory write, just to be sure you aren't about to crash.
>>> 
>>> If you make a std::vector<>, do we need to verify that its internal pointer 
>>> is not null before we write to it?   Probably not, right?  Why not?  
>>> Because it has a specification of how it works, and it is documented that 
>>> you can construct one, you can use it.
>>> 
>>> It's ok to document how functions work, and it is ok to assume that 
>>> functions work the way they claim to work.
>> 
> 

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
htt

Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Zachary Turner via lldb-dev
Occasionally (and in my experience *very* occasionally), you need to treat
"" as different from null.  But the frequency with which that is really
necessary is much lower than people realize.  It just seems high because of
the prevalence of raw pointers.

For every other case, you can use withNullAsEmpty()  (mostly used as a
helper for porting and when dealing with native APIs, which is to say quite
a bit right now, but in the long run, almost never (evidenced by the fact
that it only got added to LLVM a few days ago and nobody has ever needed
it).

On Tue, Sep 20, 2016 at 2:41 PM Mehdi Amini  wrote:

>
> > On Sep 20, 2016, at 2:31 PM, Greg Clayton  wrote:
> >
> > We should avoid crashing if there is a reasonable work around when the
> input is bad. StringRef with NULL is easy, just put NULL and zero length
> and don't crash. Just because it is documented, doesn't mean it needs to
> stay that way, but I am not going to fight that battle.
> >
> > We should make every effort to not crash if we can. If it is way too
> difficult, document the issue and make sure clients know that this is the
> way things are. StringRef does this and we accept it. Doesn't mean it won't
> crash us. I just hate seeing the crash logs where we have:
> >
> > StringRef s(die.GetName());
> >
> > It shouldn't crash IMHO, but we know it does and we now code around it.
> Yes, we put in code like:
> >
> > StringRef s;
> > const char *cstr = die.GetName();
> > if (cstr)
> >s = cstr;
> >
> > Is this nice code?
>
> Well no, it is indeed terrible: changing die.GetName() to return StringRef
> make the code a lot safer.
>
> When StringRef is used everywhere, this problem disappear by itself.
> StringRef(const char *) is a very unsafe API, that is not intended to be
> used *inside* the library (or very sporadically). So if you build a
> StringRef out of a const char *, you’re supposed to be in a place that
> deals with “uncontrolled” input and you need to sanitize it.
>
> (Also StringRef(const char *) is “costly": it calls strlen)
>
> —
> Mehdi
>
>
>
> > I am glad it makes it simple for the LLVM side, but I would rather write:
> >
> > StringRef s(die.GetName());
> >
> > Maybe I will subclass llvm::StringRef as lldb::StringRef and override
> the constructor.
> >
> >
> >> On Sep 20, 2016, at 2:24 PM, Zachary Turner  wrote:
> >>
> >> Well, but StringRef for example is well documented.  So it seems to me
> like there's an example of a perfectly used assert.  It's documented that
> you can't use null, and if you do it asserts.  Just like strlen.
> >>
> >> The issue I have with "you can't ever assert" is that it brings it into
> an absolute when it really shouldn't be.  We already agreed (I think) that
> certain things that are well documented can assert.  But when we talk in
> absolutes, it tends to sway people that they should always do that thing,
> even when it's not the most appropriate solution.  And I think some of that
> shows in the LLDB codebase where you've got hugely complicated logic that
> is very hard to follow, reason about, or test, because no assumptions are
> ever made about any of the inputs.  Even when they are internal inputs that
> are entirely controlled by us.
> >>
> >> On Tue, Sep 20, 2016 at 2:19 PM Greg Clayton 
> wrote:
> >> Again, strlen is a stupid example as it is well documented. All of llvm
> and clang are not.
> >>> On Sep 20, 2016, at 1:59 PM, Zachary Turner 
> wrote:
> >>>
> >>>
> >>>
> >>> On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton 
> wrote:
> >>>
>  On Sep 20, 2016, at 1:45 PM, Zachary Turner 
> wrote:
> 
>  I do agree that asserts are sometimes used improperly.  But who's to
> say that the bug was the assert, and not the surrounding code?  For
> example, consider this code:
> 
>  assert(p);
>  int x = *p;
> >>>
> >>> Should be written as:
> >>>
> >>> assert(p);
> >>> if (!p)
> >>>do_something_correct();
> >>> else
> >>>int x = *p;
> >>>
> 
>  Should this assert also not be here in library code?  I mean it's
> obvious that the program is about to crash if p is invalid.  Asserts should
> mean "you're about to invoke undefined behavior", and a crash is *better*
> than undefined behavior.  It surfaces the problem so that you can't let it
> slip under the radar, and it also alerts you to the point that the UB is
> invoked, rather than later.
> 
>  What about this assert?
> 
>  assert(ptr);
>  int x = strlen(ptr);
> 
>  Surely that assert is ok right?  Do we need to check whether ptr is
> valid EVERY SINGLE TIME we invoke strlen, or any other function for that
> matter?  The code would be a disastrous mess.
> >>>
> >>> Again, check before you call if this is in a shared library! What is
> so hard about that? It is called software that doesn't crash.
> >>>
> >>> assert(ptr)
> >>> int x = ptr ? strlen(ptr) : 0;
> >>>
> >>> I find it hard to believe that you are arguing that you cannot EVER
> know ANYTHING about the state of your program.  :-/
> >>

Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Mehdi Amini via lldb-dev

> On Sep 20, 2016, at 2:46 PM, Zachary Turner  wrote:
> 
> Occasionally (and in my experience *very* occasionally), you need to treat "" 
> as different from null.

return an Optional?

— 
Mehdi



>   But the frequency with which that is really necessary is much lower than 
> people realize.  It just seems high because of the prevalence of raw pointers.
> 
> For every other case, you can use withNullAsEmpty()  (mostly used as a helper 
> for porting and when dealing with native APIs, which is to say quite a bit 
> right now, but in the long run, almost never (evidenced by the fact that it 
> only got added to LLVM a few days ago and nobody has ever needed it).
> 
> On Tue, Sep 20, 2016 at 2:41 PM Mehdi Amini  > wrote:
> 
> > On Sep 20, 2016, at 2:31 PM, Greg Clayton  > > wrote:
> >
> > We should avoid crashing if there is a reasonable work around when the 
> > input is bad. StringRef with NULL is easy, just put NULL and zero length 
> > and don't crash. Just because it is documented, doesn't mean it needs to 
> > stay that way, but I am not going to fight that battle.
> >
> > We should make every effort to not crash if we can. If it is way too 
> > difficult, document the issue and make sure clients know that this is the 
> > way things are. StringRef does this and we accept it. Doesn't mean it won't 
> > crash us. I just hate seeing the crash logs where we have:
> >
> > StringRef s(die.GetName());
> >
> > It shouldn't crash IMHO, but we know it does and we now code around it. 
> > Yes, we put in code like:
> >
> > StringRef s;
> > const char *cstr = die.GetName();
> > if (cstr)
> >s = cstr;
> >
> > Is this nice code?
> 
> Well no, it is indeed terrible: changing die.GetName() to return StringRef 
> make the code a lot safer.
> 
> When StringRef is used everywhere, this problem disappear by itself. 
> StringRef(const char *) is a very unsafe API, that is not intended to be used 
> *inside* the library (or very sporadically). So if you build a StringRef out 
> of a const char *, you’re supposed to be in a place that deals with 
> “uncontrolled” input and you need to sanitize it.
> 
> (Also StringRef(const char *) is “costly": it calls strlen)
> 
> —
> Mehdi
> 
> 
> 
> > I am glad it makes it simple for the LLVM side, but I would rather write:
> >
> > StringRef s(die.GetName());
> >
> > Maybe I will subclass llvm::StringRef as lldb::StringRef and override the 
> > constructor.
> >
> >
> >> On Sep 20, 2016, at 2:24 PM, Zachary Turner  >> > wrote:
> >>
> >> Well, but StringRef for example is well documented.  So it seems to me 
> >> like there's an example of a perfectly used assert.  It's documented that 
> >> you can't use null, and if you do it asserts.  Just like strlen.
> >>
> >> The issue I have with "you can't ever assert" is that it brings it into an 
> >> absolute when it really shouldn't be.  We already agreed (I think) that 
> >> certain things that are well documented can assert.  But when we talk in 
> >> absolutes, it tends to sway people that they should always do that thing, 
> >> even when it's not the most appropriate solution.  And I think some of 
> >> that shows in the LLDB codebase where you've got hugely complicated logic 
> >> that is very hard to follow, reason about, or test, because no assumptions 
> >> are ever made about any of the inputs.  Even when they are internal inputs 
> >> that are entirely controlled by us.
> >>
> >> On Tue, Sep 20, 2016 at 2:19 PM Greg Clayton  >> > wrote:
> >> Again, strlen is a stupid example as it is well documented. All of llvm 
> >> and clang are not.
> >>> On Sep 20, 2016, at 1:59 PM, Zachary Turner  >>> > wrote:
> >>>
> >>>
> >>>
> >>> On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  >>> > wrote:
> >>>
>  On Sep 20, 2016, at 1:45 PM, Zachary Turner   > wrote:
> 
>  I do agree that asserts are sometimes used improperly.  But who's to say 
>  that the bug was the assert, and not the surrounding code?  For example, 
>  consider this code:
> 
>  assert(p);
>  int x = *p;
> >>>
> >>> Should be written as:
> >>>
> >>> assert(p);
> >>> if (!p)
> >>>do_something_correct();
> >>> else
> >>>int x = *p;
> >>>
> 
>  Should this assert also not be here in library code?  I mean it's 
>  obvious that the program is about to crash if p is invalid.  Asserts 
>  should mean "you're about to invoke undefined behavior", and a crash is 
>  *better* than undefined behavior.  It surfaces the problem so that you 
>  can't let it slip under the radar, and it also alerts you to the point 
>  that the UB is invoked, rather than later.
> 
>  What about this assert?
> 
>  assert(ptr);
>  int x = strlen(ptr);
> 
>  Surely that assert is ok right?  Do we need to check whether ptr is 
> >>>

Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Greg Clayton via lldb-dev

> On Sep 20, 2016, at 2:49 PM, Mehdi Amini  wrote:
> 
> 
>> On Sep 20, 2016, at 2:46 PM, Zachary Turner  wrote:
>> 
>> Occasionally (and in my experience *very* occasionally), you need to treat 
>> "" as different from null.

doesn't StringRef store an actual pointer to ""? This would mean 
StringRef::data() would return non null, but StringRef::size() would return 0. 
So I believe that isn't a problem with StringRef

> 
> return an Optional?
> 
> — 
> Mehdi
> 
> 
> 
>>   But the frequency with which that is really necessary is much lower than 
>> people realize.  It just seems high because of the prevalence of raw 
>> pointers.
>> 
>> For every other case, you can use withNullAsEmpty()  (mostly used as a 
>> helper for porting and when dealing with native APIs, which is to say quite 
>> a bit right now, but in the long run, almost never (evidenced by the fact 
>> that it only got added to LLVM a few days ago and nobody has ever needed it).
>> 
>> On Tue, Sep 20, 2016 at 2:41 PM Mehdi Amini  wrote:
>> 
>> > On Sep 20, 2016, at 2:31 PM, Greg Clayton  wrote:
>> >
>> > We should avoid crashing if there is a reasonable work around when the 
>> > input is bad. StringRef with NULL is easy, just put NULL and zero length 
>> > and don't crash. Just because it is documented, doesn't mean it needs to 
>> > stay that way, but I am not going to fight that battle.
>> >
>> > We should make every effort to not crash if we can. If it is way too 
>> > difficult, document the issue and make sure clients know that this is the 
>> > way things are. StringRef does this and we accept it. Doesn't mean it 
>> > won't crash us. I just hate seeing the crash logs where we have:
>> >
>> > StringRef s(die.GetName());
>> >
>> > It shouldn't crash IMHO, but we know it does and we now code around it. 
>> > Yes, we put in code like:
>> >
>> > StringRef s;
>> > const char *cstr = die.GetName();
>> > if (cstr)
>> >s = cstr;
>> >
>> > Is this nice code?
>> 
>> Well no, it is indeed terrible: changing die.GetName() to return StringRef 
>> make the code a lot safer.
>> 
>> When StringRef is used everywhere, this problem disappear by itself. 
>> StringRef(const char *) is a very unsafe API, that is not intended to be 
>> used *inside* the library (or very sporadically). So if you build a 
>> StringRef out of a const char *, you’re supposed to be in a place that deals 
>> with “uncontrolled” input and you need to sanitize it.
>> 
>> (Also StringRef(const char *) is “costly": it calls strlen)
>> 
>> —
>> Mehdi
>> 
>> 
>> 
>> > I am glad it makes it simple for the LLVM side, but I would rather write:
>> >
>> > StringRef s(die.GetName());
>> >
>> > Maybe I will subclass llvm::StringRef as lldb::StringRef and override the 
>> > constructor.
>> >
>> >
>> >> On Sep 20, 2016, at 2:24 PM, Zachary Turner  wrote:
>> >>
>> >> Well, but StringRef for example is well documented.  So it seems to me 
>> >> like there's an example of a perfectly used assert.  It's documented that 
>> >> you can't use null, and if you do it asserts.  Just like strlen.
>> >>
>> >> The issue I have with "you can't ever assert" is that it brings it into 
>> >> an absolute when it really shouldn't be.  We already agreed (I think) 
>> >> that certain things that are well documented can assert.  But when we 
>> >> talk in absolutes, it tends to sway people that they should always do 
>> >> that thing, even when it's not the most appropriate solution.  And I 
>> >> think some of that shows in the LLDB codebase where you've got hugely 
>> >> complicated logic that is very hard to follow, reason about, or test, 
>> >> because no assumptions are ever made about any of the inputs.  Even when 
>> >> they are internal inputs that are entirely controlled by us.
>> >>
>> >> On Tue, Sep 20, 2016 at 2:19 PM Greg Clayton  wrote:
>> >> Again, strlen is a stupid example as it is well documented. All of llvm 
>> >> and clang are not.
>> >>> On Sep 20, 2016, at 1:59 PM, Zachary Turner  wrote:
>> >>>
>> >>>
>> >>>
>> >>> On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:
>> >>>
>>  On Sep 20, 2016, at 1:45 PM, Zachary Turner  wrote:
>> 
>>  I do agree that asserts are sometimes used improperly.  But who's to 
>>  say that the bug was the assert, and not the surrounding code?  For 
>>  example, consider this code:
>> 
>>  assert(p);
>>  int x = *p;
>> >>>
>> >>> Should be written as:
>> >>>
>> >>> assert(p);
>> >>> if (!p)
>> >>>do_something_correct();
>> >>> else
>> >>>int x = *p;
>> >>>
>> 
>>  Should this assert also not be here in library code?  I mean it's 
>>  obvious that the program is about to crash if p is invalid.  Asserts 
>>  should mean "you're about to invoke undefined behavior", and a crash is 
>>  *better* than undefined behavior.  It surfaces the problem so that you 
>>  can't let it slip under the radar, and it also alerts you to the point 
>>  that the UB is invoked, rather than later.
>> 
>>  What ab

Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Greg Clayton via lldb-dev

> On Sep 20, 2016, at 2:41 PM, Mehdi Amini  wrote:
> 
>> 
>> On Sep 20, 2016, at 2:31 PM, Greg Clayton  wrote:
>> 
>> We should avoid crashing if there is a reasonable work around when the input 
>> is bad. StringRef with NULL is easy, just put NULL and zero length and don't 
>> crash. Just because it is documented, doesn't mean it needs to stay that 
>> way, but I am not going to fight that battle.
>> 
>> We should make every effort to not crash if we can. If it is way too 
>> difficult, document the issue and make sure clients know that this is the 
>> way things are. StringRef does this and we accept it. Doesn't mean it won't 
>> crash us. I just hate seeing the crash logs where we have:
>> 
>> StringRef s(die.GetName());
>> 
>> It shouldn't crash IMHO, but we know it does and we now code around it. Yes, 
>> we put in code like:
>> 
>> StringRef s;
>> const char *cstr = die.GetName();
>> if (cstr)
>>   s = cstr;
>> 
>> Is this nice code?
> 
> Well no, it is indeed terrible: changing die.GetName() to return StringRef 
> make the code a lot safer.
> 
> When StringRef is used everywhere, this problem disappear by itself. 
> StringRef(const char *) is a very unsafe API, that is not intended to be used 
> *inside* the library (or very sporadically). So if you build a StringRef out 
> of a const char *, you’re supposed to be in a place that deals with 
> “uncontrolled” input and you need to sanitize it. 
> 
> (Also StringRef(const char *) is “costly": it calls strlen)

That is fine, but someone is still making the initial StringRef with the "const 
char *" so strlen is still being called. Strings from DWARF come from a string 
table so we will always call strlen. We might reduce the number of times strlen 
is called though and that is good.


> — 
> Mehdi
> 
> 
> 
>> I am glad it makes it simple for the LLVM side, but I would rather write:
>> 
>> StringRef s(die.GetName());
>> 
>> Maybe I will subclass llvm::StringRef as lldb::StringRef and override the 
>> constructor.
>> 
>> 
>>> On Sep 20, 2016, at 2:24 PM, Zachary Turner  wrote:
>>> 
>>> Well, but StringRef for example is well documented.  So it seems to me like 
>>> there's an example of a perfectly used assert.  It's documented that you 
>>> can't use null, and if you do it asserts.  Just like strlen.
>>> 
>>> The issue I have with "you can't ever assert" is that it brings it into an 
>>> absolute when it really shouldn't be.  We already agreed (I think) that 
>>> certain things that are well documented can assert.  But when we talk in 
>>> absolutes, it tends to sway people that they should always do that thing, 
>>> even when it's not the most appropriate solution.  And I think some of that 
>>> shows in the LLDB codebase where you've got hugely complicated logic that 
>>> is very hard to follow, reason about, or test, because no assumptions are 
>>> ever made about any of the inputs.  Even when they are internal inputs that 
>>> are entirely controlled by us.
>>> 
>>> On Tue, Sep 20, 2016 at 2:19 PM Greg Clayton  wrote:
>>> Again, strlen is a stupid example as it is well documented. All of llvm and 
>>> clang are not.
 On Sep 20, 2016, at 1:59 PM, Zachary Turner  wrote:
 
 
 
 On Tue, Sep 20, 2016 at 1:55 PM Greg Clayton  wrote:
 
> On Sep 20, 2016, at 1:45 PM, Zachary Turner  wrote:
> 
> I do agree that asserts are sometimes used improperly.  But who's to say 
> that the bug was the assert, and not the surrounding code?  For example, 
> consider this code:
> 
> assert(p);
> int x = *p;
 
 Should be written as:
 
 assert(p);
 if (!p)
   do_something_correct();
 else
   int x = *p;
 
> 
> Should this assert also not be here in library code?  I mean it's obvious 
> that the program is about to crash if p is invalid.  Asserts should mean 
> "you're about to invoke undefined behavior", and a crash is *better* than 
> undefined behavior.  It surfaces the problem so that you can't let it 
> slip under the radar, and it also alerts you to the point that the UB is 
> invoked, rather than later.
> 
> What about this assert?
> 
> assert(ptr);
> int x = strlen(ptr);
> 
> Surely that assert is ok right?  Do we need to check whether ptr is valid 
> EVERY SINGLE TIME we invoke strlen, or any other function for that 
> matter?  The code would be a disastrous mess.
 
 Again, check before you call if this is in a shared library! What is so 
 hard about that? It is called software that doesn't crash.
 
 assert(ptr)
 int x = ptr ? strlen(ptr) : 0;
 
 I find it hard to believe that you are arguing that you cannot EVER know 
 ANYTHING about the state of your program.  :-/
 
 This is like arguing that you should run a full heap integrity check every 
 time you perform a memory write, just to be sure you aren't about to crash.
 
 If you make a std::vector<>, do we

Re: [lldb-dev] LLDB Evolution - Final Form

2016-09-20 Thread Zachary Turner via lldb-dev
On Tue, Sep 20, 2016 at 2:51 PM Greg Clayton  wrote:

>
> > On Sep 20, 2016, at 2:49 PM, Mehdi Amini  wrote:
> >
> >
> >> On Sep 20, 2016, at 2:46 PM, Zachary Turner  wrote:
> >>
> >> Occasionally (and in my experience *very* occasionally), you need to
> treat "" as different from null.
>
> doesn't StringRef store an actual pointer to ""? This would mean
> StringRef::data() would return non null, but StringRef::size() would return
> 0. So I believe that isn't a problem with StringRef
>

Right, it's only a problem when you need to write a function that returns
or accepts a string, and null has to be treated differently than empty.  As
Mehdi said, one option is an Optional, although I admit that
there are times where pointers would still be more convenient.  They're
still used in some places in llvm and clang for example, just not nearly as
often as StringRef.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] running lldb-mi with LLDB_DISABLE_PYTHON

2016-09-20 Thread Chunseok Lee via lldb-dev
Do you have any idea about regression after applying workaround 1 ? (For
example, wrong output in some cases)


2016-09-20 21:37 GMT+09:00 Ilia K :

> Hi!
>
> Please see possible workarounds in https://llvm.org/bugs/show_
> bug.cgi?id=28253.
>
> On Tue, Sep 20, 2016 at 10:57 AM, Chunseok Lee via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
>
>> Dear lldb dev team,
>>
>> I am working on running lldb-mi on arm32 device(like rpi) for remote
>> debugging usage.
>> Is there any way to run lldb-mi with python disabled ?
>> When building lldb with LLDB_DISABLE_PYTHON, it seems that dataformatter
>> initialization is failed due to the following code in
>> MICmnLLDBDebugger.cpp:59
>>
>> //++
>> //--
>> --
>> // MI summary helper routines
>> static inline bool MI_add_summary(lldb::SBTypeCategory category,
>>   const char *typeName,
>>   lldb::SBTypeSummary::FormatCallback cb,
>>   uint32_t options, bool regex = false) {
>> #if defined(LLDB_DISABLE_PYTHON)
>>   return false;
>> #else
>>   lldb::SBTypeSummary summary =
>>   lldb::SBTypeSummary::CreateWithCallback(cb, options);
>>   return summary.IsValid()
>>  ? category.AddTypeSummary(
>>lldb::SBTypeNameSpecifier(typeName, regex), summary)
>>  : false;
>> #endif
>> }
>>
>> Thank you.
>>
>> BR,
>> Chunseok Lee
>>
>> --
>> Where Do We come from? What Are We? Where Are We Going?
>>
>> ___
>> lldb-dev mailing list
>> lldb-dev@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>>
>>
>
>
> --
> - Ilia
>



-- 
Where Do We come from? What Are We? Where Are We Going?
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev