[Lldb-commits] [PATCH] D134906: Have MemoryCache cache addresses that were unreadable, so duplicate read attempts can be suppressed

2022-09-30 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a reviewer: DavidSpickett. DavidSpickett added a comment. Seems like a valid thing to do. Is it a problem that maybe you read from address N to N+100 and the problem address is actually at N+50, not N? I think you might be handling that already but I didn't read all the logi

[Lldb-commits] [PATCH] D134873: [LLDB] Add "frame select" as equivalent of GDB's "frame" command

2022-09-30 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Thanks for the suggestion @jingham , does this look good to you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134873/new/ https://reviews.llvm.org/D134873 ___ lldb-commits

[Lldb-commits] [PATCH] D134962: [LLDB] Change pointer to ref in EmulateInstruction::ReadRegister methods

2022-09-30 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added subscribers: atanasyan, jrtc27. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. ReadRegister and ReadRegisterAsUnsigned are always passed valid poi

[Lldb-commits] [PATCH] D134963: [LLDB] Switch to RegisterInfo& for EmulateInstruction::WriteRegister

2022-09-30 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added subscribers: atanasyan, jrtc27. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. WriteRegister and WriteRegisterUnsigned were never pased nullptr, a

lldb-commits@lists.llvm.org

2022-09-30 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added subscribers: atanasyan, jrtc27, kbarton, nemanjai, sdardis. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Most of the paths to this never passed

[Lldb-commits] [PATCH] D133778: Fix LLDB build on old Linux kernels (pre-4.1)

2022-10-03 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Did this ever land? I only see https://github.com/llvm/llvm-project/commit/d35702efe73010409c75b1f1b64f205cc3b2f6d3. We have a short time to get this into 15.0.2, so I'd like to land this if it is needed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D134906: Have MemoryCache cache addresses that were unreadable, so duplicate read attempts can be suppressed

2022-10-03 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. > If we wanted to track these decisions it would be more appropriate to log > them, but I'm not sure even that is necessary. Yeah this is a better idea, if we do it at all. Let me rephrase the question. If I had a memory read failing and I suspected that the cach

[Lldb-commits] [PATCH] D133778: Fix LLDB build on old Linux kernels (pre-4.1)

2022-10-03 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. I only have newer kernels but replacing the type with what it looks like on an older kernel and undefing PERF_ATTR_SIZE_VER5, lldb does build. So it looks like this is a good thing to land. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

2022-10-03 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. I like the direction, thanks for working on this. To no one's surprise, I was thinking about testing :) - Errors related to the filesystem e.g. can't open a path, is too complicated to setup on demand. The code looks to be passing on the error, which is good enou

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

2022-10-04 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. > I think that part should be easy to test and I was planning on adding a test > as part of that. Sounds good to me. Nothing needed in this change then. A command also gives you a chance to get the diagnostics out if running until the actual crash fails to dump t

[Lldb-commits] [PATCH] D135169: [LLDB] Fix printing a static bool struct member when using "image lookup -t"

2022-10-04 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added a reviewer: shafik. Herald added a project: All. DavidSpickett requested review of this revision. Herald added projects: clang, LLDB. Herald added subscribers: lldb-commits, cfe-commits. Fixes #58135 Somehow lldb was able to print the member on it

[Lldb-commits] [PATCH] D135170: [LLDB] Fix crash when printing a struct with a static signed char member

2022-10-04 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added a project: All. DavidSpickett requested review of this revision. Herald added projects: clang, LLDB. Herald added subscribers: lldb-commits, cfe-commits. As with static bool for whatever reason printing them on their own worked fine but wasn't hand

[Lldb-commits] [PATCH] D135169: [LLDB] Fix printing a static bool struct member when using "image lookup -t"

2022-10-04 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Herald added a subscriber: JDevlieghere. For https://github.com/llvm/llvm-project/issues/58135. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135169/new/ https://reviews.llvm.org/D135169 _

[Lldb-commits] [PATCH] D135169: [LLDB] Fix printing a static bool struct member when using "image lookup -t"

2022-10-05 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments. Comment at: clang/include/clang/AST/ExprCXX.h:733 + static CXXBoolLiteralExpr *Create(const ASTContext &C, bool Val, QualType Ty, +SourceLocation Loc) { shafik wrote: > I think this makes

[Lldb-commits] [PATCH] D133778: Fix LLDB build on old Linux kernels (pre-4.1)

2022-10-05 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa9ffb4734535: Fix LLDB build on old Linux kernels (pre-4.1) (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133778/new/ https://r

[Lldb-commits] [PATCH] D133778: Fix LLDB build on old Linux kernels (pre-4.1)

2022-10-05 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. I've landed this so it has some time to go through bots before a backport to 15. Apologies that your name isn't in the commit, I did add an author line locally but Arc decided to remove it before landing. If you do more fixes (and please do) get commit access your

[Lldb-commits] [PATCH] D135169: [LLDB] Fix printing a static bool struct member when using "image lookup -t"

2022-10-05 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett updated this revision to Diff 465341. DavidSpickett added a comment. Rebase, Create moved to parent change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135169/new/ https://reviews.llvm.org/D135169 Files: lldb/include/lldb/Symbol/

[Lldb-commits] [PATCH] D135170: [LLDB] Fix crash when printing a struct with a static signed char member

2022-10-05 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett updated this revision to Diff 465342. DavidSpickett added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135170/new/ https://reviews.llvm.org/D135170 Files: clang/lib/AST/StmtPrinter.cpp lldb/test/API/lang/cpp/c

[Lldb-commits] [PATCH] D135170: [LLDB] Fix crash when printing a struct with a static signed char member

2022-10-06 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments. Comment at: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp:39 const static auto char_min = std::numeric_limits::min(); const static auto uchar_min = std::numeric_limits::min(); shafik wrote: > We use `sig

[Lldb-commits] [PATCH] D135352: [LLDB] Complete set of char tests for static integral members

2022-10-06 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Previously we had a bit of a mix of "signed char" "unsigned char" and "char". This adds seperate min and max ch

[Lldb-commits] [PATCH] D134962: [LLDB] Change pointer to ref in EmulateInstruction::ReadRegister methods

2022-10-06 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134962/new/ https://reviews.llvm.org/D134962 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llv

[Lldb-commits] [PATCH] D134963: [LLDB] Switch to RegisterInfo& for EmulateInstruction::WriteRegister

2022-10-06 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134963/new/ https://reviews.llvm.org/D134963 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llv

lldb-commits@lists.llvm.org

2022-10-06 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134965/new/ https://reviews.llvm.org/D134965 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llv

[Lldb-commits] [PATCH] D134041: [LLDB] Enable non-trivial types in EmulateInstruction::Context

2022-10-06 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett abandoned this revision. DavidSpickett added a comment. https://reviews.llvm.org/D135015 landed using std::variant and no one has complained so far, so this whole patch is now irrelevant. If it comes back, I'll just use variant. I agree that there are reasons to keep things POD so

[Lldb-commits] [PATCH] D135169: [LLDB] Fix printing a static bool struct member when using "image lookup -t"

2022-10-07 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG02c1c939486f: [LLDB] Fix printing a static bool struct member when using "image lookup -t" (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[Lldb-commits] [PATCH] D135170: [LLDB] Fix crash when printing a struct with a static signed char member

2022-10-07 Thread David Spickett via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5a9e21305803: [LLDB] Fix crash when printing a struct with a static signed char member (authored by DavidSpickett). Repository: rG LLVM Github Mon

[Lldb-commits] [PATCH] D134873: [LLDB] Add "frame select" as equivalent of GDB's "frame" command

2022-10-07 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG68ab7accc72b: [LLDB] Add "frame select" as equivalent of GDB's "frame" command (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134

[Lldb-commits] [PATCH] D135461: [LLDB] Fix crash when printing a struct with a static wchar_t member

2022-10-10 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. You need to update the python file for the test as well. What does the output look like, do we actually print the character itself or it's value? (just curious, which one we do is probably out of scope for this change) Comment at: clang/lib/AST

[Lldb-commits] [PATCH] D135352: [LLDB] Complete set of char tests for static integral members

2022-10-10 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135352/new/ https://reviews.llvm.org/D135352 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llv

[Lldb-commits] [PATCH] D135352: [LLDB] Complete set of char tests for static integral members

2022-10-10 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb3d4d9ced17c: [LLDB] Complete set of char tests for static integral members (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135352

[Lldb-commits] [PATCH] D135461: [LLDB] Fix crash when printing a struct with a static wchar_t member

2022-10-11 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. > although those changes don't actually test the code path changed here Yeah I'm just cargo culting on that one so it doesn't look strange that a few are missing. If we're going

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

2022-10-11 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments. Comment at: lldb/source/Utility/Diagnostics.cpp:43 + std::lock_guard guard(m_callbacks_mutex); + m_callbacks.push_back(callback); +} DavidSpickett wrote: > Is it worth adding an assert that the callback is not already in the

[Lldb-commits] [PATCH] D134962: [LLDB] Change pointer to ref in EmulateInstruction::ReadRegister methods

2022-10-11 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. No problem, thanks for the reviews! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134962/new/ https://reviews.llvm.org/D134962 ___ lldb-commits mailing list lldb-commits@li

[Lldb-commits] [PATCH] D134962: [LLDB] Change pointer to ref in EmulateInstruction::ReadRegister methods

2022-10-11 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG2a627e2ad1d8: [LLDB] Change pointer to ref in EmulateInstruction::ReadRegister methods (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[Lldb-commits] [PATCH] D135622: [lldb] Add a "diagnostics dump" command

2022-10-11 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments. Comment at: lldb/source/Commands/CommandObjectDiagnostics.cpp:85 + result.AppendError(llvm::toString(directory.takeError())); + result.SetStatus(eReturnStatusFailed); + return result.Succeeded(); AppendError set

[Lldb-commits] [PATCH] D135631: [lldb] Copy log files into diagnostic directory (RFC)

2022-10-11 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. So either way, if someone was running with no logs enabled we would have to ask them to enable them then re-run and get a new set of diagnostics (which is fine as is , not a criticism). Option 1 they would need to add `-o "log enable gdb-remote packets -f "`. Opti

[Lldb-commits] [PATCH] D135631: [lldb] Copy log files into diagnostic directory (RFC)

2022-10-11 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Also is the buffer option the same as "circular" here? -h ( --log-handler ) Specify a log handler which determines where log messages are written. Values: default | stream | circular | os So we could have diagnostics also save anything kept in "ci

[Lldb-commits] [PATCH] D134963: [LLDB] Switch to RegisterInfo& for EmulateInstruction::WriteRegister

2022-10-11 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG81832afc04e1: [LLDB] Switch to RegisterInfo& for EmulateInstruction::WriteRegister (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

lldb-commits@lists.llvm.org

2022-10-11 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc7ddbd62d811: [LLDB] Change RegisterValue::GetAsMemoryData to const RegisterInfo& (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

lldb-commits@lists.llvm.org

2022-10-11 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. All callers were either assuming their pointer was not null before calling this, or checking beforehand. Repos

lldb-commits@lists.llvm.org

2022-10-11 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a reviewer: clayborg. DavidSpickett added a comment. Herald added a subscriber: JDevlieghere. This set of 4 patches will be it for a while on this front, honest :) Thanks for the reviews so far! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[Lldb-commits] [PATCH] D135670: [LLDB] Pass const RegisterInfo& to RegisterValue::SetValueFromData

2022-10-11 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Familiar story, callers are either checking upfront that the pointer wasn't null or not checking at all. SetValu

lldb-commits@lists.llvm.org

2022-10-11 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. No one was pasing nullptr here. Depends on D135670 Repository: rG LLVM G

[Lldb-commits] [PATCH] D135672: [LLDB] Change EmulateInstriction::ReadRegister to return Optional

2022-10-11 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added subscribers: atanasyan, jrtc27. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Making it easier to understand and harder to misuse. This only appl

[Lldb-commits] [PATCH] D135577: Summary:

2022-10-12 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. > Note: The reason the summary is so terse is that the ARC tool opens two > windows and I was very confused about where to write what. Certainly would confuse me, maybe I've learned to autopilot through this though. Can you describe them? We can certainly add a no

[Lldb-commits] [PATCH] D135577: Summary:

2022-10-12 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Oh and if you would like someone to land this on your behalf please provide a name and email address for the commit (of course waiting for access is fine too). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135577/new

lldb-commits@lists.llvm.org

2022-10-12 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett updated this revision to Diff 467041. DavidSpickett added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135668/new/ https://reviews.llvm.org/D135668 Files: lldb/include/lldb/Utility/RegisterValue.h lldb/so

lldb-commits@lists.llvm.org

2022-10-12 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG812ad2167bd2: [LLDB] Change RegisterValue::SetFromMemoryData to const RegisterInfo& (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[Lldb-commits] [PATCH] D135670: [LLDB] Pass const RegisterInfo& to RegisterValue::SetValueFromData

2022-10-12 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6faa345da9d7: [LLDB] Pass const RegisterInfo& to RegisterValue::SetValueFromData (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

lldb-commits@lists.llvm.org

2022-10-12 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG28e65a6a63ab: [LLDB] Change RegisterValue::SetType param to const RegisterInfo& (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13

[Lldb-commits] [PATCH] D135672: [LLDB] Change EmulateInstriction::ReadRegister to return Optional

2022-10-12 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett updated this revision to Diff 467057. DavidSpickett added a comment. Fix some undeclared vars, somehow I forgot to actualy build this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135672/new/ https://reviews.llvm.org/D135672 Files:

[Lldb-commits] [PATCH] D135672: [LLDB] Change EmulateInstriction::ReadRegister to return Optional

2022-10-12 Thread David Spickett via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0577a9f8d06b: [LLDB] Change EmulateInstriction::ReadRegister to return Optional (authored by DavidSpickett). Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D135631: [lldb] Copy log files into diagnostic directory (RFC)

2022-10-12 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. > Technically yes, but then we have two ad-hoc solutions that support half the > log handlers. I think if we care about that we might as well go with the > T-based log handler behind the scenes. Sure. More wondering whether the choice between circular buffer and f

[Lldb-commits] [PATCH] D135825: [LLDB] Only run lldb-server Shell tests if it gets built

2022-10-13 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Would a `lit.local.cfg` work in this folder (`lldb/test/Shell/lldb-server/` that is)? That would automatically apply it to all existing and future tests. `llvm/test/MC/AArch64/lit.local.cfg` is one example. Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[Lldb-commits] [PATCH] D135827: [lldb] Print newline between found types

2022-10-14 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a reviewer: DavidSpickett. DavidSpickett added a comment. This LGTM thanks for the fix just one comment on the test. Comment at: lldb/test/API/lang/cpp/type_lookup_duplicate/TestCppTypeLookupDuplicate.py:16 + +self.expect("image lookup -A -t Foo", D

[Lldb-commits] [PATCH] D135826: [lldb] Start from end of previous substr when checking ordered substrs

2022-10-14 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a reviewer: DavidSpickett. DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. This LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135826/new/ https://review

[Lldb-commits] [PATCH] D135825: [LLDB] Only run lldb-server Shell tests if it gets built

2022-10-14 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. LGTM, thanks for the fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135825/new/ https://reviews.llvm.org/D135825 ___ lld

[Lldb-commits] [PATCH] D135827: [lldb] Print newline between found types

2022-10-17 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135827/new/ https://reviews.llvm.org/D135827

[Lldb-commits] [PATCH] D136011: [lldb] Don't check environment default char signedness when creating clang type for "char"

2022-10-17 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. > With -f(un)signed-char, the die corresponding to "char" may be the wrong > DW_ATE_(un)signed_char. As the producer of the DWARF (so, clang for example) is this correct by the existing rules? As I understand it so far, if the compiler is using "char" (no sign ch

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-24 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. I don't have the expertise to review, but have one question. Does this account for the situation where you spawn many gdbserver from the platform and therefore need more ports? Does it even need to? (just guessing that that could have been the reason for selecting

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-24 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Ok, so this change means that the randomisation still happens, unless you override it with these environment variables? This seems like a good way to do it. Where do these env vars need to be set? On the local machine, on the lldb-server machine, on the device?

[Lldb-commits] [PATCH] D136584: [LLDB] Check that RegisterInfo and ContextInfo are trivial

2022-10-24 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. RegisterInfo is often initialised with a memcpy, and ContextInfo does not run destructors for anything within it

[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

2022-10-24 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Always good to see another architecture in lldb. A few points up front. Changes like this are fine and we can review them, but can only be landed once we see that they'll be built on. So please stack changes so that we can see what is upcoming. https://llvm.org/d

[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

2022-10-24 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. > What's more, AFAIK, @seehearfeel (yangtie...@loongson.cn) is the LoongArch > port maintainer of gdb. > Regarding the public buildbot, I'm trying to set it up and maybe we can see > it in one or two weeks. Great! CHANGES SINCE LAST ACTION https://reviews.llvm

[Lldb-commits] [PATCH] D136584: [LLDB] Check that RegisterInfo and ContextInfo are trivial

2022-10-24 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8d06ef565818: [LLDB] Check that RegisterInfo and ContextInfo are trivial (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136584/ne

[Lldb-commits] [PATCH] D136462: [LLDB] Add color to output text when searching for symbols

2022-10-24 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. I agree that this change names - print - restore names is going to cause problems. Pavel is right that this should happen in some dump function somewhere. But, assuming what you've got works to an extent, you should add some testing first. That will allow you to

[Lldb-commits] [PATCH] D136462: [LLDB] Add color to output text when searching for symbols

2022-10-24 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. And if it helps to refactor in parent patches to make this change easier, go for it. Maybe you add generic colouring support to something, then the last patch hooks it up to the symbol lookup, something along those lines. Repository: rG LLVM Github Monorepo CH

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-25 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Yes, somehow, and yes just update again. Happens to us all, I have learned to not try to be clever with arc and just keep one commit per review. Tip: if you look in the "Revision Contents" box there is a "history" and you can diff between versions of the patch, i

[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

2022-10-25 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. This LGTM from the lldb side. Feel free to upload a series of patches in future if you want. You don't have to do one at a time, just update the whole stack as needed. Plus it gi

[Lldb-commits] [PATCH] D135622: [lldb] Add a "diagnostics dump" command

2022-10-25 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments. Comment at: lldb/source/Utility/Diagnostics.cpp:70 - Error error = Create(FileSpec(diagnostics_dir.str())); + Error error = Create(*diagnostics_dir); if (error) { JDevlieghere wrote: > DavidSpickett wrote: > > Silly que

[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

2022-10-25 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. > I am a newcomer, here are some of my thoughts: > (1) Add the minimal changes to fix the build errors. > (2) Submit other more patches step by step to make > the basic command "run", "breakpoint", "next" ... > can be used to debug, single patch or patch series. > (3

[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

2022-10-25 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa3be778ed09b: [LLDB] [LoongArch] Add minimal LoongArch support (authored by seehearfeel, committed by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[Lldb-commits] [PATCH] D136578: [LLDB] [LoongArch] Add minimal LoongArch support

2022-10-25 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Landed as https://github.com/llvm/llvm-project/commit/a3be778ed09b7badcda20c5c8738ba19531dad48. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136578/new/ https://reviews.llvm.org/D136578

[Lldb-commits] [PATCH] D135622: [lldb] Add a "diagnostics dump" command

2022-10-25 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135622/new/ https://reviews.llvm.org/D135622 ___ lldb-commits mailing lis

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-26 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. > Great! I'd just like to note that I do not have commit access, per the > guide's instructions. What name/email address do you want on the commit? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136465/new/ https://r

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-26 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1e210abf9925: [LLDB] Make remote-android local ports configurable (authored by mark2185, committed by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-26 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Landed as https://github.com/llvm/llvm-project/commit/1e210abf9925ad08fb7c79894b4ec5ef8f0ef173. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136465/new/ https://reviews.llvm.org/D136465

[Lldb-commits] [PATCH] D136465: Make remote-android local ports configurable

2022-10-26 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Thanks for your efforts here, very much appreciated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136465/new/ https://reviews.llvm.org/D136465 ___ lldb-commits mailing lis

[Lldb-commits] [PATCH] D136362: [LLDB][RISCV] Add RV64C instruction support for EmulateInstructionRISCV

2022-10-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Can you give us: - the full cmake command you used to configure the project - the gcc/g++ version used - the distro used - the architecture (unlikely to matter but I'm assuming Loongson?) - the version of clang++ that did work Could be tripping over a bug in g++. I

[Lldb-commits] [PATCH] D136928: [LLDB] Fix help text for "platform settings"

2022-10-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This claims to take a platform name argument but doesn't. That was probably the intent in fbb7634934d40548b6505

[Lldb-commits] [PATCH] D136928: [LLDB] Fix help text for "platform settings"

2022-10-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a reviewer: clayborg. DavidSpickett added a comment. Herald added a subscriber: JDevlieghere. Alternatively, I could look at adding that option. Not sure off the top of my head if configuring an inactive platform would even work. Repository: rG LLVM Github Monorepo CHANGE

[Lldb-commits] [PATCH] D136938: [LLDB] Fix code breakpoints on tagged addresses

2022-10-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett created this revision. Herald added subscribers: atanasyan, arichardson, sdardis. Herald added a project: All. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This allows you to break on the value of a function p

[Lldb-commits] [PATCH] D136938: [LLDB] Fix code breakpoints on tagged addresses

2022-10-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett updated this revision to Diff 471481. DavidSpickett added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Add a release note. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136938/new/ https://reviews.

[Lldb-commits] [PATCH] D136938: [LLDB] Fix code breakpoints on tagged addresses

2022-10-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Tests will run only for armv8.3-a hardware so I've tested them with `qemu-system`. Existing tests pass on Armv8 Linux as before. Some of these PAC tests could run on Apple Silicon now that I think about it. That's for another time, needs extra work. Repository:

[Lldb-commits] [PATCH] D136938: [LLDB] Fix code breakpoints on tagged addresses

2022-10-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. If you want to understand the motivation beyond "this should work why doesn't it", here it is. If your ABI mandated signing all function pointers you might store a callback in a struct somewhere. Not knowing what that callback points to you would like to be able

[Lldb-commits] [PATCH] D135664: [wasm] Always treat DWARF expression addresses as load addresses

2022-10-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments. Comment at: lldb/unittests/Expression/DWARFExpressionTest.cpp:435 + ASSERT_EQ(result.GetValueType(), Value::ValueType::LoadAddress); +} DavidSpickett wrote: > There are two `target && target->GetArchitecture().GetCore() == >

[Lldb-commits] [PATCH] D135664: [wasm] Always treat DWARF expression addresses as load addresses

2022-10-28 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135664/new/ https://reviews.llvm.org/D135664

[Lldb-commits] [PATCH] D137045: [lldb] Don't crash when printing static enum members with bool as underlying type

2022-10-31 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:7612 + ->getIntegerType() + ->isSpecificBuiltinType(BuiltinType::Bool)) && "only boolean supported"); Can you call `TypeS

[Lldb-commits] [PATCH] D137057: [LLDB][LoongArch] Add LoongArch ArchSpec and subtype detection

2022-10-31 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. Looks good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137057/new/ https://reviews.llvm.org/D137057

[Lldb-commits] [PATCH] D137045: [lldb] Don't crash when printing static enum members with bool as underlying type

2022-11-01 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. LGTM Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:7612 + ->getIntegerType() + ->isSpecificBuiltinType(BuiltinT

[Lldb-commits] [PATCH] D137247: [lldb] Allow plugins to extend DWARF expression parsing for vendor extensions

2022-11-02 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. I'm missing some context here I think. I assume the use case is that, for example, we don't want to be adding a bunch of WASM specific DWARF operations to the generic operations. If those can live in a WASM (the "vendor") plugin then we know where to look and they

[Lldb-commits] [PATCH] D137247: [lldb] Allow plugins to extend DWARF expression parsing for vendor extensions

2022-11-02 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments. Comment at: lldb/unittests/Expression/DWARFExpressionTest.cpp:719 + Type:ET_EXEC + Machine: EM_386 +Sections: I know it's just a test but is there an `EM_WASM` you could use? Maybe lldb doesn't understan

[Lldb-commits] [PATCH] D137247: [lldb] Allow plugins to extend DWARF expression parsing for vendor extensions

2022-11-02 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments. Comment at: lldb/source/Expression/DWARFExpressionList.cpp:93 const DWARFExpression &expr = m_exprs.GetEntryRef(0).data; - return expr.ContainsThreadLocalStorage(); + return expr.ContainsThreadLocalStorage(*m_dwarf_cu); } ---

[Lldb-commits] [PATCH] D137312: [LLDB] [LoongArch] Add loongarch64 case in ComputeHostArchitectureSupport()

2022-11-03 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett accepted this revision. DavidSpickett added a comment. LGTM. Follows the same logic AArch64 does. (meaning: we don't support debugging 32 bit from a 64 bit lldb very well, if it all, but it's still noted here as possible) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D137272: [lldb][Test] Make TestFrameFormatNameWithArgs.test more compatible across platforms

2022-11-03 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Test passes on AArch64 Ubuntu 20.04.4, where it failed prior to this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137272/new/ https://reviews.llvm.org/D137272 ___

[Lldb-commits] [PATCH] D138181: [test] Allow skipTestIfFn to apply to entire classes for skipIfNoSBHeaders

2022-11-17 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. LGTM. Do what makes sense to you with the comment. Comment at: lldb/packages/Python/lldbsuite/test/decorators.py:146 +# reason is used for both args

[Lldb-commits] [PATCH] D137247: [lldb] Allow plugins to extend DWARF expression parsing for vendor extensions

2022-11-17 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. What was the decision on `m_dwarf_cu` possibly being nullptr? We at least want asserts in paths where we assume it'll not be null. Beyond that this seems fine but I'm not really into DWARF, @labath any comments? Seems like quite a cheap fallback to add, and my ass

[Lldb-commits] [PATCH] D138197: [lldb] Fix bitfield incorrectly printing when field crosses a storage unit

2022-11-21 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment. Thanks for working on this. Due to llvm dev and vacation it took me a while to get to this. Comment at: lldb/test/API/commands/expression/expr-bitfield-on-boundary/Makefile:3 + +CXXFLAGS_EXTRAS := -gdwarf-2 + Add a comment here

[Lldb-commits] [PATCH] D138407: [LLDB] Add LoongArch register definitions and operations

2022-11-22 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments. Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp:194 + + uint8_t *dst = const_cast(data_sp->GetBytes()); + ::memcpy(dst, GetGPRBuffer(), GetGPRSize()); I'm not sure const cast is need

[Lldb-commits] [PATCH] D138539: Use std::nullopt_t instead of NoneType (NFC)

2022-11-23 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a reviewer: DavidSpickett. DavidSpickett added a comment. lldb parts LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138539/new/ https://reviews.llvm.org/D138539 ___ lldb-commits

<    6   7   8   9   10   11   12   13   14   15   >