[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

This revision was not accepted when it landed; it landed in state "Needs Revision". This revision was automatically updated to reflect the committed changes. Closed by commit rG6700f4b9fe63: [LLDB] Add checks for ValueObjectSP in Cocoa summary providers (authored by shafik). Herald added a projec

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

teemperor accepted this revision. teemperor added a comment. I'm still not sure how this can happen or how this could be tested, but this patch seems reasonable. Also it seems we usually do nullptr checks on GetSyntheticChildAtOffset results, so at least it would be consistent if we do it here

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

shafik added a comment. I updated to include the radar number. So we have a crash in `NSStringSummaryProvider(...)` when calling `valobj.GetProcessSP()` and we have what looks like a `nullptr` e.g. `0x0068` In this specific case we are coming from `NSBundleSummaryProvider(...)` and

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

teemperor added a comment. I think Davide's point is that it's not clear what motivated this change and why this doesn't have a regression test. I'm also a bit confused by the description ("We saw a crash recently that looks related to we had good ValueObjectSP for some Cocoa summary providers.

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

shafik added a comment. In D84272#2168172 , @davide wrote: > why? Do you have a testcase? There are already tests but we have a crash report w/o a reproducer, these are obviously correct checks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. why? Do you have a testcase? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84272/new/ https://reviews.llvm.org/D84272 ___ lldb

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

shafik updated this revision to Diff 279965. shafik added a comment. Removing `text->GetValueAsUnsigned(0) == 0` check. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84272/new/ https://reviews.llvm.org/D84272 Files: lldb/source/Plugins/Language/ObjC/Cocoa.cpp Index: lldb/source/Plu

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

shafik added a comment. I was testing out how `NSStringSummaryProvider` deals w/ `NULL` using `NSString *foo = nullptr` and we filter out `NULL` values in `ValueObjectPrinter::GetValueSummaryError(...)` : if (IsNil()) summary.assign("nil"); That should mean that the `text->GetValueAs

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

jingham added a comment. The `!text` test is correct, since you intend to pass `*text` in as a `ValueObject &`. But I wouldn't add the GetValueAsUnsigned check, that seems confusing. The NSStringSummaryProvider is returning a bool to tell you whether it succeeded or not, so it seems odd to pr

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

shafik created this revision. shafik added a reviewer: jingham. We saw a crash recently that looks related to we had good `ValueObjectSP` for some Cocoa summary providers. This adds checks before we use them when calling `NSStringSummaryProvider`. https://reviews.llvm.org/D84272 Files: lldb