[Lldb-commits] [PATCH] D153043: [lldb] Fix handling of cfi_restore in the unwinder

jarin added a comment. In D153043#4427660 , @Michael137 wrote: > Looks like this is failing on the Darwin x86_64 buildbots: > https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/56510/execution/node/74/log/ > > Exit Code: 1 > > Command

[Lldb-commits] [PATCH] D153043: [lldb] Fix handling of cfi_restore in the unwinder

This revision was automatically updated to reflect the committed changes. Closed by commit rG07b9e6ed0db2: [lldb] Fix handling of cfi_restore in the unwinder (authored by jarin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153043/new/

[Lldb-commits] [PATCH] D153043: [lldb] Fix handling of cfi_restore in the unwinder

jarin updated this revision to Diff 531987. jarin added a comment. Added a comment explaining that we are restoring the caller's value of the register. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153043/new/ https://reviews.llvm.org/D153043 Files:

[Lldb-commits] [PATCH] D153043: [lldb] Fix handling of cfi_restore in the unwinder

jarin created this revision. jarin added a reviewer: labath. jarin added a project: LLDB. Herald added a subscriber: JDevlieghere. Herald added a project: All. jarin requested review of this revision. Herald added a subscriber: lldb-commits. Currently, lldb's unwinder ignores cfi_restore opcodes

[Lldb-commits] [PATCH] D115104: [lldb] Fix guessing of windows path style

This revision was automatically updated to reflect the committed changes. Closed by commit rGf72ae5cba1d6: [lldb] Fix windows path guessing for root paths (authored by jarin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115104/new/

[Lldb-commits] [PATCH] D115104: [lldb] Fix guessing of windows path style

jarin created this revision. jarin added a reviewer: labath. jarin requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Currently, only paths with length>3 are guessed as Windows paths. This prevents root paths (such as C:\) to be recognized

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

This revision was automatically updated to reflect the committed changes. Closed by commit rG5a3556aa5563: [lldb] Add omitted abstract formal parameters in DWARF symbol files (authored by Jaroslav Sevcik ja...@chromium.org, committed by jarin). Changed prior to commit:

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

jarin added a comment. Pavel, could you take another look? Interestingly the null-deref problem was already fixed in the follow-up patch , maybe I just did not land quickly enough :-)). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110570/new/

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

jarin updated this revision to Diff 377474. jarin marked 2 inline comments as done. jarin added a comment. Avoid nullptr deref/ref. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110570/new/ https://reviews.llvm.org/D110570 Files:

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3545 + +DIEVector MergeBlockAbstractParameters(const DWARFDIE _die, + DIEVector &_dies) { labath wrote: > In llvm, we prefer

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin updated this revision to Diff 377020. jarin marked 4 inline comments as done. jarin added a comment. Addressed Pavel's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110571/new/ https://reviews.llvm.org/D110571 Files:

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin added a comment. Thank you for the great comments, Pavel. I took a stab at merging the parameters without interleaving them with the locals. Let me know what you think; I can certainly put this back to the original state if you think this is a change for the worse. (I am sorry for the

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin updated this revision to Diff 376501. jarin marked 5 inline comments as done. jarin added a comment. Addressed reviewer comments, separated merging of the abstract parameters into a function, prevented interleaving of parameters with locals. CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin added a comment. In D110571#3033481 , @labath wrote: > In D110571#3033282 , @jarin wrote: > >> In D110571#3033078 , @labath wrote: >> >>> Here's one more question.

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin updated this revision to Diff 376235. jarin marked an inline comment as done. jarin added a comment. Cache only global variables. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110571/new/ https://reviews.llvm.org/D110571 Files:

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin added a comment. In D110571#3033078 , @labath wrote: > Here's one more question. AIUI, lldb relies on the order of formal parameter > declarations in dwarf to establish the the function signature (dwarf doesn't > leave us much choice. This then

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin updated this revision to Diff 376124. jarin marked an inline comment as done. jarin added a comment. Improved the comment, as Greg suggested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110571/new/ https://reviews.llvm.org/D110571 Files:

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin added a comment. In D110571#3031846 , @clayborg wrote: > In D110571#3031140 , @jarin wrote: > >> For illustration: >> >> 0x100 DW_TAG_subprogram >>DW_AT_name "inlined_function" >>

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin added a comment. First of all, thank you, Greg and Pavel, for all the great feedback and discussion. I have followed all your suggestions (use plain method, use `image lookup`, added the additional tests). I have also packaged the C test, but as Greg notes I am not convinced it will keep

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin updated this revision to Diff 375901. jarin added a comment. Changed the test to avoid running the process and use `image lookup` instead of `frame variable`. I think I would still slightly prefer `frame variable`, mostly because there seem to be differences between what `image lookup`

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin added a comment. In D110571#3030222 , @labath wrote: > In D110571#3030192 , @jarin wrote: > >> This still uses `frame variable` rather than `image lookup`, mostly because >> `frame variable` tests better

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin updated this revision to Diff 375886. jarin added a comment. Added a C test (I have also verified that the C test fails without the SymbolFileDWARF patch). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110571/new/

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin updated this revision to Diff 375865. jarin added a comment. Rewrote the recursive parser to use a plain method. Pruned and annotated the test. Added other test cases: - all parameters unused, - inlining from two different functions, - stack trace. This still uses `frame variable`

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin added a comment. Just a few replies below; I am hoping to put the relevant code changes together tomorrow. In D110571#3027173 , @labath wrote: > I haven't looked at the actual code yet, so I could be off, but here are some > thoughts. > > In

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

jarin added a comment. Thank you for the review! Does the separate method look more reasonable? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110570/new/ https://reviews.llvm.org/D110570 ___

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

jarin updated this revision to Diff 375538. jarin added a comment. Added the `/*can_create=*/` comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110570/new/ https://reviews.llvm.org/D110570 Files:

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

jarin updated this revision to Diff 375535. jarin added a comment. Separated ParseVariablesInFunctionContext's lambda into a method. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110570/new/ https://reviews.llvm.org/D110570 Files:

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

jarin added a comment. Thank you for taking a look, shafik@. I hope you don't mind if I address your comments later, once a full review arrives from Pavel (or Johannes). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110570/new/

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin added a comment. Hi, could you take a look at this change? Some discussion points: - In the ParseVariablesInFunctionContext method, we are using a lambda for the recursive parser. We could also use a function-local class or inner class of SymbolFileDWARF. Would any of these be

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

jarin created this revision. jarin added a reviewer: labath. jarin added a project: LLDB. Herald added a subscriber: JDevlieghere. jarin requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: lldb-commits, sstefan1. This change fixes a problem introduced

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

jarin added a comment. Hi, could you possibly take a look at this change? The main motivation for this patch is to move the setup of the variable list for parsing children to the DIE node that corresponds to the block containing the children. This will be particularly important for

[Lldb-commits] [PATCH] D110570: [lldb] Refactor variable parsing in DWARF symbol file

jarin created this revision. jarin added a reviewer: labath. jarin added a project: LLDB. Herald added a subscriber: JDevlieghere. jarin requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: lldb-commits, sstefan1. This is in preparation for a fix of

[Lldb-commits] [PATCH] D73191: Only match mangled name in full-name function lookup (with accelerators)

jarin added a comment. Thanks for getting back to this, Raphael! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73191/new/ https://reviews.llvm.org/D73191 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D90318: Return actual type from SBType::GetArrayElementType

jarin accepted this revision. jarin added a comment. This revision is now accepted and ready to land. The code change looks good to me and it is in line with the change that Raphael and Greg wanted in https://reviews.llvm.org/D72133. As far as I remember, https://reviews.llvm.org/D72133 did not

[Lldb-commits] [PATCH] D85719: Initialize static const fields in the AST for expression evaluation

jarin created this revision. jarin added a reviewer: teemperor. jarin added a project: LLDB. Herald added subscribers: lldb-commits, JDevlieghere. Herald added a reviewer: shafik. jarin requested review of this revision. This patch is for discussion. Currently, the evaluator does not know about

[Lldb-commits] [PATCH] D83858: [lldb] Desugar template specializations

This revision was automatically updated to reflect the committed changes. Closed by commit rG93ec6cd68426: [lldb] Desugar template specializations (authored by jarin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83858/new/

[Lldb-commits] [PATCH] D83858: [lldb] Desugar template specializations

jarin updated this revision to Diff 278200. jarin added a comment. Undo the infinite loop detection. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83858/new/ https://reviews.llvm.org/D83858 Files:

[Lldb-commits] [PATCH] D83858: [lldb] Desugar template specializations

jarin added a comment. In D83858#2152772 , @teemperor wrote: > This could cause that `RemoveWrappingTypes` goes into an infinite loop under > some situations. Usually this function is reserved for types that are always > 'sugar', but

[Lldb-commits] [PATCH] D83858: [lldb] Desugar template specializations

jarin updated this revision to Diff 278173. jarin marked 3 inline comments as done. jarin added a comment. Addressed review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83858/new/ https://reviews.llvm.org/D83858 Files:

[Lldb-commits] [PATCH] D83858: [lldb] Desugar template specializations

jarin created this revision. jarin added a reviewer: teemperor. jarin added a project: LLDB. Herald added a subscriber: lldb-commits. Template specializations are not handled in many of the TypeSystemClang methods. For example, GetNumChildren does not handle the TemplateSpecialization type class,

[Lldb-commits] [PATCH] D81465: [lldb] Fix and enable Windows minidump tests

This revision was automatically updated to reflect the committed changes. Closed by commit rGfac5d05eb75f: [lldb] Fix and enable Windows minidump tests (authored by jarin). Changed prior to commit: https://reviews.llvm.org/D81465?vs=269609=269649#toc Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D81465: [lldb] Fix and enable Windows minidump tests

jarin updated this revision to Diff 269609. jarin added a comment. Added a comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81465/new/ https://reviews.llvm.org/D81465 Files: lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py Index:

[Lldb-commits] [PATCH] D81465: [lldb] Fix and enable Windows minidump tests

jarin created this revision. jarin added a reviewer: labath. jarin added a project: LLDB. Herald added a subscriber: lldb-commits. jarin edited the summary of this revision. SBFileSpec.fullpath always uses the forward slash to join the directory with the base name. This causes mismatches when

[Lldb-commits] [PATCH] D81363: Disable remove-add module test on Windows

This revision was automatically updated to reflect the committed changes. Closed by commit rG6143874f734b: [lldb] Disable remove-add module test on Windows (authored by jarin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81363/new/

[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef

This revision was automatically updated to reflect the committed changes. Closed by commit rG1beffc18886a: Support build-ids of other sizes than 16 in UUID::SetFromStringRef (authored by jarin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef

jarin updated this revision to Diff 268497. jarin added a comment. Exclude UUID strings ending with "-". CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80755/new/ https://reviews.llvm.org/D80755 Files: lldb/include/lldb/Utility/UUID.h lldb/source/Interpreter/OptionValueUUID.cpp

[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef

jarin updated this revision to Diff 267754. jarin added a comment. Added a test for missing nibble in UUID. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80755/new/ https://reviews.llvm.org/D80755 Files: lldb/include/lldb/Utility/UUID.h lldb/source/Interpreter/OptionValueUUID.cpp

[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef

jarin updated this revision to Diff 267739. jarin edited the summary of this revision. jarin added a comment. Removed size restrictions on UUIDs. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80755/new/ https://reviews.llvm.org/D80755 Files: lldb/include/lldb/Utility/UUID.h

[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef

jarin marked an inline comment as done. jarin added inline comments. Comment at: lldb/source/Utility/UUID.cpp:109 *this = fromData(bytes); -return str.size() - rest.size(); +return str.size(); } jankratochvil wrote: > Now the return type could be

[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef

jarin updated this revision to Diff 267490. jarin marked an inline comment as done. jarin added a comment. Change SetFromStringRef to return bool. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80755/new/ https://reviews.llvm.org/D80755 Files: lldb/include/lldb/Utility/UUID.h

[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in UUID::SetFromStringRef

jarin updated this revision to Diff 267241. jarin retitled this revision from "Support build-ids of other sizes than 16 in SBTarget::AddModule" to "Support build-ids of other sizes than 16 in UUID::SetFromStringRef". jarin edited the summary of this revision. Herald added subscribers: MaskRay,

[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in SBTarget::AddModule

jarin marked an inline comment as done. jarin added inline comments. Comment at: lldb/source/API/SBTarget.cpp:1593 + llvm::StringRef rest = + UUID::DecodeUUIDBytesFromString(uuid_str, bytes, 20); + // If the UUID string was consumed and it is not empty, let us

[Lldb-commits] [PATCH] D80755: Support build-ids of other sizes than 16 in SBTarget::AddModule

jarin created this revision. jarin added a reviewer: labath. jarin added a project: LLDB. Herald added a subscriber: lldb-commits. SBTarget::AddModule currently handles the UUID parameter in a very weird way: UUIDs with more than 16 bytes are trimmed to 16 bytes. On the other hand,

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

jarin added a comment. Raphael, could you land this for me? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80254/new/ https://reviews.llvm.org/D80254 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

jarin added a comment. In D80254#2047982 , @clayborg wrote: > This looks good, thanks for subscribing me. We need to have GetNumChildren > and GetChildAtIndex agreeing on things and we definitely shouldn't be walking > more than on pointer recursively.

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

jarin updated this revision to Diff 265237. jarin marked 3 inline comments as done. jarin added a comment. Merged the ObjC pointer case with the reference case, simplified the test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80254/new/ https://reviews.llvm.org/D80254 Files:

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

jarin added a comment. In D80254#2046275 , @labath wrote: > Looks fine to me too. The way this test is phrased, it would probably make > more sense under `test/API/python_api/value`, than here. (I mean, it's not > technically wrong because everything is

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

jarin marked an inline comment as not done. jarin added inline comments. Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:5215 +uint32_t num_pointee_children = 0; +if (pointee_clang_type.IsAggregateType()) + num_pointee_children =

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

jarin updated this revision to Diff 265141. jarin marked 2 inline comments as done. jarin added a comment. Added more assertions, reformatted the test code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80254/new/ https://reviews.llvm.org/D80254 Files:

[Lldb-commits] [PATCH] D80254: Prevent GetNumChildren from transitively walking pointer chains

jarin created this revision. jarin added reviewers: teemperor, jingham. jarin added a project: LLDB. Herald added a subscriber: lldb-commits. This is an attempt to fix https://bugs.llvm.org/show_bug.cgi?id=45988, where SBValue::GetNumChildren returns 2, but SBValue::GetChildAtIndex(1) returns an

[Lldb-commits] [PATCH] D79308: [lldb-server] Reset stop reason of all threads when resuming

jarin added a comment. Pavel, could you possibly land this for us? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79308/new/ https://reviews.llvm.org/D79308 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D79308: [lldb-server] Reset stop reason of all threads when resuming

jarin added a comment. Jim, do you think this is good to go? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79308/new/ https://reviews.llvm.org/D79308 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D73191: Only match mangled name in full-name function lookup (with accelerators)

jarin reopened this revision. jarin added a comment. This revision is now accepted and ready to land. Reopening for further investigation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73191/new/ https://reviews.llvm.org/D73191

[Lldb-commits] [PATCH] D79308: [lldb-server] Reset stop reason of all threads when resuming

jarin added inline comments. Comment at: lldb/test/API/functionalities/thread/break_step_other/TestThreadBreakStepOther.py:21 +class ThreadBreakStepOtherTestCase(TestBase): +mydir = TestBase.compute_mydir(__file__) + labath wrote: > You can add

[Lldb-commits] [PATCH] D79308: [lldb-server] Reset stop reason of all threads when resuming

jarin updated this revision to Diff 262087. jarin marked 2 inline comments as done. jarin added a comment. ... now also fixed the 'volatile'. It took only three patches to copy four lines of code by hand. Not bad, huh? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79308/new/

[Lldb-commits] [PATCH] D79308: [lldb-server] Reset stop reason of all threads when resuming

jarin updated this revision to Diff 262082. jarin added a comment. ... and remove the extra braces. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79308/new/ https://reviews.llvm.org/D79308 Files: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp

[Lldb-commits] [PATCH] D79308: [lldb-server] Reset stop reason of all threads when resuming

jarin updated this revision to Diff 262081. jarin marked an inline comment as done. jarin added a comment. Addressed reviewer comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79308/new/ https://reviews.llvm.org/D79308 Files:

[Lldb-commits] [PATCH] D79404: Fix error handling after [] in 'frame variable'

jarin added a comment. Do you think Raphael would want to review this as well? If you think it is not necessary, could you land the patch for me? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79404/new/ https://reviews.llvm.org/D79404

[Lldb-commits] [PATCH] D79404: Fix error handling after [] in 'frame variable'

jarin created this revision. jarin added reviewers: teemperor, labath. Herald added subscribers: lldb-commits, arphaman. Herald added a project: LLDB. jarin retitled this revision from "Fix handling of [] in 'frame variable'" to "Fix error handling after [] in 'frame variable'". This fixes a bug

[Lldb-commits] [PATCH] D79308: [lldb-server] Reset stop reason of all threads when resuming

jarin updated this revision to Diff 261805. jarin added a comment. Simplify the test based on the suggestion from labath@. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79308/new/ https://reviews.llvm.org/D79308 Files:

[Lldb-commits] [PATCH] D79308: [lldb-server] Reset stop reason of all threads when resuming

jarin added a comment. In D79308#2017348 , @labath wrote: > The test setup here seems unnecessarily complex. Wouldn't an inferior like > this work better? > > void thread1() { > pseudo_barrier_wait(g_barrier); // See other tests how this works. >

[Lldb-commits] [PATCH] D79308: [lldb-server] Reset stop reason of all threads when resuming

jarin created this revision. jarin added a reviewer: jingham. jarin added a project: LLDB. Herald added a subscriber: lldb-commits. This patch makes the stop reason reset logic similar to MacOS' debugserver, where exceptions are reset for all threads when resuming process for stepping or

[Lldb-commits] [PATCH] D77790: [NFC] Add a test for the inferior memory cache (mainly L1)

jarin added a comment. In D77790#1975324 , @clayborg wrote: > In D77790#1974047 , @jarin wrote: > > > It appears it is really hard to reach agreement about this, so another > > alternative is I submit a bug report

[Lldb-commits] [PATCH] D77765: Fix incorrect L1 inferior memory cache flushing

jarin added a comment. Abandoning the patch since we cannot reach agreement on how this should be tested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77765/new/ https://reviews.llvm.org/D77765 ___

[Lldb-commits] [PATCH] D77790: [NFC] Add a test for the inferior memory cache (mainly L1)

jarin added a comment. Regarding the callback idea, I have bad experience with callbacks because they break if the code is not crafted for re-entrancy and they are much harder to understand. That change feels out of scope for just adding a test and fixing an unrelated bug. Adding the

[Lldb-commits] [PATCH] D77765: Fix incorrect L1 inferior memory cache flushing

jarin added a comment. As labath@ suggested, I have teased apart the test and the testability refactoring into a separate patch. The patch lives at https://reviews.llvm.org/D77790, could you please take a look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D77790: [NFC] Add a test for the inferior memory cache (mainly L1)

jarin added inline comments. Comment at: lldb/source/Target/Memory.cpp:63 if (pos != m_L1_cache.begin()) { - --pos; + pos--; } labath wrote: > I guess this is a leftover from splitting the patches? > > Speaking of post-increment the [[ >

[Lldb-commits] [PATCH] D77790: [NFC] Add a test for the inferior memory cache (mainly L1)

jarin updated this revision to Diff 256257. jarin marked 2 inline comments as done. jarin added a comment. Addressed reviewer comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77790/new/ https://reviews.llvm.org/D77790 Files:

[Lldb-commits] [PATCH] D77790: [NFC] Add a test for the inferior memory cache (mainly L1)

jarin created this revision. jarin added reviewers: labath, clayborg. jarin added a project: LLDB. Herald added subscribers: lldb-commits, mgorny. This patch adds a test for L1 of the inferior's memory cache and makes the cache testable. This is mostly in

[Lldb-commits] [PATCH] D77765: Fix incorrect L1 inferior memory cache flushing

jarin added a comment. I rewrote parts of the test, hopefully making it a bit clearer. Please let me know if this made it better. Comment at: lldb/source/Target/Memory.cpp:23-24 // MemoryCache constructor -MemoryCache::MemoryCache(Process )

[Lldb-commits] [PATCH] D77765: Fix incorrect L1 inferior memory cache flushing

jarin updated this revision to Diff 256207. jarin marked 10 inline comments as done. jarin added a comment. Addressed some of the reviewer comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77765/new/ https://reviews.llvm.org/D77765 Files:

[Lldb-commits] [PATCH] D77765: Fix incorrect L1 inferior memory cache flushing

jarin created this revision. jarin added reviewers: labath, clayborg. jarin added a project: LLDB. Herald added subscribers: lldb-commits, mgorny. jarin retitled this revision from "Fix incorrect L1 cache flushing" to "Fix incorrect L1 inferior memory cache flushing". As discussed in

[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

jarin added a comment. Looking at the code for flushing L1 cache , it appears broken. I am guessing that is the reason for the failure of my patch on the bots. Here is the

[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

jarin added a comment. Pavel, could you land this for me? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74398/new/ https://reviews.llvm.org/D74398 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

jarin added a comment. In D74398#1939372 , @jarin wrote: > In D74398#1935438 , @jasonmolenda > wrote: > > > (and if you're still seeing mystery reads, put a breakpoint on > > ProcessGDBRemote::DoReadMemory and

[Lldb-commits] [PATCH] D76650: Data formatters: fix detection of C strings

jarin updated this revision to Diff 252276. jarin marked 2 inline comments as done. jarin added a comment. Addressed reviewer comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76650/new/ https://reviews.llvm.org/D76650 Files:

[Lldb-commits] [PATCH] D76650: Data formatters: fix detection of C strings

jarin added a comment. Thanks for the review! Could you possibly land this for me? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76650/new/ https://reviews.llvm.org/D76650 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D76650: Data formatters: fix detection of C strings

jarin created this revision. jarin added a reviewer: teemperor. jarin added a project: LLDB. Herald added a subscriber: lldb-commits. Detection of C strings does not work well for pointers. If the value object holding a (char*) pointer does not have an address (e.g., if it is a temp), the value

[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

jarin added a comment. Regarding the packet savings - there are still things that worry me. First of all, when lldb CLI stops on a breakpoint, it will first unwind top of the stack of each thread as part of ThreadList::ShouldStop. This sends lots of "x" packets to lldb-server and only then

[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

jarin updated this revision to Diff 251714. jarin marked 3 inline comments as done. jarin added a comment. Addressed comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74398/new/ https://reviews.llvm.org/D74398 Files:

[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

jarin updated this revision to Diff 251512. jarin added a comment. Adding a tighter x64 test as suggested by labath@. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74398/new/ https://reviews.llvm.org/D74398 Files:

[Lldb-commits] [PATCH] D76216: Improve step over performance

jarin added a comment. Pavel or Jim, could you possibly land this for me? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76216/new/ https://reviews.llvm.org/D76216 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D76216: Improve step over performance

jarin added a comment. Thanks Greg, I will wait for Jim's comment. I also see that build-bot is not happy about my patch. Clang-tidy somewhat mysteriously fails on missing lldb/Target/ThreadPlanStepOverRange.h, which I did not touch at all (neither the fail nor the #include). Any idea what

[Lldb-commits] [PATCH] D76216: Improve step over performance

jarin updated this revision to Diff 251208. jarin added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76216/new/ https://reviews.llvm.org/D76216 Files: lldb/source/Target/ThreadPlanStepOverRange.cpp Index:

[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

jarin added a comment. In D74398#1929019 , @labath wrote: > Thanks. Could you also add the other kind of test (the one inline asm) I > mentioned. In an ideal world we'd have a test case for every boundary > condition, but we're pretty far from that

[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

jarin updated this revision to Diff 251181. jarin marked 5 inline comments as done. jarin added a comment. Addressed reviewer comments in the code, but still have no clue how to write the test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D74398: [lldb-server] jThreadsInfo returns stack memory

jarin updated this revision to Diff 250951. jarin added a comment. - Added a test that checks consistency of thread info's memory chunks with the actual memory. - Using DataExtractor to extract pointers with the right endian-ness and the right size. Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D76216: Improve step over performance

jarin added a comment. Thanks for all the clarifications, this is very useful. I have always wanted to learn about thread plans, and this was a nice opportunity to do that. The extra background from you guys is a nice bonus. Regarding the patch itself, is there anything preventing an LGTM?

[Lldb-commits] [PATCH] D76216: Improve step over performance

jarin marked an inline comment as done. jarin added inline comments. Comment at: lldb/source/Target/ThreadPlanStepOverRange.cpp:176 +// rely on that breakpoint to trigger once we return to the range. +if (m_next_branch_bp_sp) + return false;

[Lldb-commits] [PATCH] D76216: Improve step over performance

jarin created this revision. jarin added reviewers: clayborg, labath. jarin added a project: LLDB. Herald added a subscriber: lldb-commits. This patch improves step over performance for the case when we are stepping over a call with a next-branch-breakpoint (see

  1   2   >