[Lldb-commits] [PATCH] D151366: [lldb] Disable variable watchpoints when going out of scope

mib added a comment. In D151366#4372731 , @aprantl wrote: >> @aprantl Do you know if we can detect the end of the scope ? I'm not sure >> it's possible currently ... But even if we could do it, that wouldn't cover >> the following case: > > When setting

[Lldb-commits] [PATCH] D151366: [lldb] Disable variable watchpoints when going out of scope

This revision was automatically updated to reflect the committed changes. Closed by commit rG7c847ac4bd1b: [lldb] Disable variable watchpoints when going out of scope (authored by mib). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151366/new/ https

[Lldb-commits] [PATCH] D151497: [lldb] Improve function caller error message

mib added a subscriber: kastiglione. mib added a comment. In D151497#4374080 , @bulbazord wrote: > I think it's good to improve the error messaging but I think we can probably > do better. "Function caller" is specific to the internals of LLDB and isn't

[Lldb-commits] [PATCH] D151399: [lldb] Introduce FileSpec::GetComponents

mib accepted this revision. mib added a comment. This revision is now accepted and ready to land. LGTM! Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1241 +// We want the components in reverse order. +std::reverse(path_parts.begin(), path_parts.end(

[Lldb-commits] [PATCH] D151392: Fix SBValue::FindValue for file static variables

mib accepted this revision. mib added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151392/new/ https://reviews.llvm.org/D151392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D151366: [lldb] Disable variable watchpoints when going out of scope

mib marked an inline comment as done. mib added inline comments. Comment at: lldb/source/Breakpoint/Watchpoint.cpp:127 + +bool Watchpoint::VariableWatchpointDisabler(void *baton, +StoppointCallbackContext *context, JDev

[Lldb-commits] [PATCH] D151366: [lldb] Disable variable watchpoints when going out of scope

mib marked an inline comment as done. mib added inline comments. Comment at: lldb/source/Commands/CommandObjectWatchpoint.cpp:959 ", variable expression='%s').\n", addr, (uint64_t)size, command.GetArgumentAtIndex(0)); if (error.AsCString(nullptr)) ---

[Lldb-commits] [PATCH] D151762: [lldb] Remove __FUNCTION__ from log messages in lldbHost (NFC)

mib accepted this revision as: mib. mib added a comment. LGTM! Are you considering doing it for the rest of the codebase as well ? That would be very nice :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151762/new/ https://reviews.llvm.org/D151762

[Lldb-commits] [PATCH] D151764: [lldb] Support file and function names in LLDB_LOGF macro

mib accepted this revision as: mib. mib added a comment. This revision is now accepted and ready to land. LGTM! Can we add a test for this ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151764/new/ https://reviews.llvm.org/D151764 ___ lldb-c

[Lldb-commits] [PATCH] D151597: [lldb][NFCI] Remove use of ConstString from IOHandler

mib added a comment. TBH, it feels like a lot of - risky - changes just so `IOHandlerGetControlSequence` can return a `llvm::StringRef` ? Is that really necessary ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151597/new/ https://reviews.llvm.or

[Lldb-commits] [PATCH] D151591: HostInfoMacOS: Add a utility function for finding an SDK-specific tool

mib added inline comments. Comment at: lldb/unittests/Host/HostInfoTest.cpp:90 + EXPECT_FALSE(find_tool("MacOSX.sdk", "clang").empty()); + EXPECT_TRUE(find_tool("MacOSX.sdk", "CeciNestPasUnOutil").empty()); +} JDevlieghere wrote: > LOL ^^ CHANGES SINCE LAST A

[Lldb-commits] [PATCH] D151844: [lldb/crashlog] Fix crash when loading non-symbolicated report

mib created this revision. mib added reviewers: bulbazord, JDevlieghere. mib added a project: LLDB. Herald added a project: All. mib requested review of this revision. Herald added a subscriber: lldb-commits. This patch should address the crashes when parsing a the crash report frame dictionary.

[Lldb-commits] [PATCH] D151849: [lldb/crashlog] Create interactive crashlog with no binary

mib created this revision. mib added reviewers: bulbazord, JDevlieghere. mib added a project: LLDB. Herald added a project: All. mib requested review of this revision. Herald added a subscriber: lldb-commits. This patch changes the way we load a crash report into a scripted process by creating a e

[Lldb-commits] [PATCH] D151844: [lldb/crashlog] Fix crash when loading non-symbolicated report

mib added inline comments. Comment at: lldb/examples/python/crashlog.py:599-602 +if 'symbol' in json_frame: +symbol = json_frame['symbol'] +location = 0 +if "symbolLocation" in json_frame and json_frame["symbolLocation"

[Lldb-commits] [PATCH] D151849: [lldb/crashlog] Create interactive crashlog with no binary

mib marked an inline comment as done. mib added inline comments. Comment at: lldb/examples/python/crashlog.py:493 self.images = list() +self.crashlog = CrashLog(debugger, self.path, self.verbose) bulbazord wrote: > Any reason you moved this? It

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set name to targets

mib created this revision. mib added reviewers: JDevlieghere, bulbazord, jingham. mib added a project: LLDB. Herald added a project: All. mib requested review of this revision. Herald added a subscriber: lldb-commits. This patch add the ability for the user to set a name for a target. This can be

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set name to targets

mib added a comment. I still need to add some tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151859/new/ https://reviews.llvm.org/D151859 ___ lldb-commits mailing list lldb-commits@lists.llvm.org h

[Lldb-commits] [PATCH] D151843: Add EXC_SYSCALL to the allowable ignored exceptions for Darwin

mib added a comment. LGTM! Sorry I didn't look at this earlier... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151843/new/ https://reviews.llvm.org/D151843 ___ lldb-commits mailing list lldb-commits@lis

[Lldb-commits] [PATCH] D151849: [lldb/crashlog] Create interactive crashlog with no binary

mib updated this revision to Diff 527575. mib marked an inline comment as done. mib added a subscriber: jingham. mib added a comment. Address @bulbazord & @jingham's comments: - Rename name -> label - Add test - Add sanity check to avoid having integer only labels - Add target index when label al

[Lldb-commits] [PATCH] D151849: [lldb/crashlog] Create interactive crashlog with no binary

mib updated this revision to Diff 527576. mib added a comment. Reload the previous version of this diff CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151849/new/ https://reviews.llvm.org/D151849 Files: lldb/examples/python/crashlog.py lldb/source/Target/Process.cpp Index: lldb/sou

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set name to targets

mib marked 3 inline comments as done. mib added inline comments. Comment at: lldb/source/Commands/CommandObjectTarget.cpp:546-551 +if (!name.empty()) { + if (name == target_idx_arg) { +target_idx = i; +break; +

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set a label to targets

mib updated this revision to Diff 527580. mib marked an inline comment as done. mib retitled this revision from "[lldb/Target] Add ability to set name to targets" to "[lldb/Target] Add ability to set a label to targets". mib edited the summary of this revision. mib added a comment. Address @bulba

[Lldb-commits] [PATCH] D151940: Fix regex & startsWith name lookup in SBTarget::FindGlobalVariables

mib accepted this revision. mib added a comment. LGTM! Comment at: lldb/test/API/lang/cpp/class_static/TestStaticVariables.py:182 self.DebugSBValue(val) -self.assertEqual(val.GetName(), "A::g_points") +value_check_A.check_value(self, val, "FiindValue al

[Lldb-commits] [PATCH] D151849: [lldb/crashlog] Create interactive crashlog with no binary

mib updated this revision to Diff 527672. mib marked 2 inline comments as done. mib added a comment. Address @bulbazord last comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151849/new/ https://reviews.llvm.org/D151849 Files: lldb/examples/python/crashlog.py lldb/source/Targe

[Lldb-commits] [PATCH] D151844: [lldb/crashlog] Fix crash when loading non-symbolicated report

This revision was automatically updated to reflect the committed changes. Closed by commit rGa5a6c03c448b: [lldb/crashlog] Fix crash when loading non-symbolicated report (authored by mib). Changed prior to commit: https://reviews.llvm.org/D151844?vs=527212&id=527673#toc Repository: rG LLVM G

[Lldb-commits] [PATCH] D151849: [lldb/crashlog] Create interactive crashlog with no binary

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG032d91cb2fb5: [lldb/crashlog] Create interactive crashlog with no binary (authored by mib). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set a label to targets

mib updated this revision to Diff 527776. mib added a comment. Address @bulbazord comments: - Make `Target::SetLabel` return an `llvm::Error` to propagate error messages to both the `CommandObject` and the `SBAPI` callers. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151859/new/ http

[Lldb-commits] [PATCH] D152011: [lldb/Commands] Add support to auto-completion for user commands

mib created this revision. mib added reviewers: bulbazord, jingham, JDevlieghere. mib added a project: LLDB. Herald added a project: All. mib requested review of this revision. Herald added a subscriber: lldb-commits. This patch should allow the user to set specific auto-completion type for their

[Lldb-commits] [PATCH] D152012: [lldb/crashlog] Expand crash report file path before parsing

mib created this revision. mib added reviewers: bulbazord, kastiglione. mib added a project: LLDB. Herald added a subscriber: JDevlieghere. Herald added a project: All. mib requested review of this revision. Herald added a subscriber: lldb-commits. This patch should fix a crash in the opening a cr

[Lldb-commits] [PATCH] D152013: [lldb/Commands] Fix disk completion from root directory

mib created this revision. mib added reviewers: JDevlieghere, bulbazord, kastiglione. mib added a project: LLDB. Herald added a project: All. mib requested review of this revision. Herald added a subscriber: lldb-commits. This patch should fix path completion starting from the root directory. To

[Lldb-commits] [PATCH] D152012: [lldb/crashlog] Expand crash report file path before parsing

mib added a comment. In D152012#4391336 , @bulbazord wrote: > The key here is that you're expanding the path and checking for the validity > before calling `CrashLogParser.create()` right? It looks like for the > `load_crashlog_in_scripted_process` case

[Lldb-commits] [PATCH] D152013: [lldb/Commands] Fix disk completion from root directory

mib updated this revision to Diff 528058. mib added a comment. Add test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152013/new/ https://reviews.llvm.org/D152013 Files: lldb/source/Commands/CommandCompletions.cpp lldb/test/API/functionalities/completion/TestCompletion.py Index:

[Lldb-commits] [PATCH] D152011: [lldb/Commands] Add support to auto-completion for user commands

mib updated this revision to Diff 528069. mib added a comment. Address @bulbazord comments: - Add test & usage descriptions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152011/new/ https://reviews.llvm.org/D152011 Files: lldb/docs/python_api_enums.rst lldb/examples/python/crashlo

[Lldb-commits] [PATCH] D152013: [lldb/Commands] Fix disk completion from root directory

mib marked an inline comment as done. mib added inline comments. Comment at: lldb/test/API/functionalities/completion/TestCompletion.py:439-450 +def test_completion_target_create_from_root_dir(self): +"""Tests source file completion by completing .""" +root_di

[Lldb-commits] [PATCH] D152013: [lldb/Commands] Fix disk completion from root directory

mib updated this revision to Diff 528560. mib marked an inline comment as done. mib added a comment. Address @bulbazord comments! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152013/new/ https://reviews.llvm.org/D152013 Files: lldb/source/Commands/CommandCompletions.cpp lldb/test/

[Lldb-commits] [PATCH] D152011: [lldb/Commands] Add support to auto-completion for user commands

mib marked an inline comment as done. mib added inline comments. Comment at: lldb/include/lldb/lldb-enumerations.h:1279 + // if you & in this bit the base code will not process the option. + eCustomCompletion = (1u << 24) +}; JDevlieghere wrote: > Should this b

[Lldb-commits] [PATCH] D152011: [lldb/Commands] Add support to auto-completion for user commands

mib updated this revision to Diff 528928. mib marked an inline comment as done. mib added a comment. Address @JDevlieghere comment CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152011/new/ https://reviews.llvm.org/D152011 Files: lldb/docs/python_api_enums.rst lldb/examples/python/c

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set a label to targets

This revision was automatically updated to reflect the committed changes. mib marked 2 inline comments as done. Closed by commit rG1e82b20118e3: [lldb/Target] Add ability to set a label to targets (authored by mib). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[Lldb-commits] [PATCH] D152011: [lldb/Commands] Add support to auto-completion for user commands

This revision was automatically updated to reflect the committed changes. Closed by commit rG6a9c3e611505: [lldb/Commands] Add support to auto-completion for user commands (authored by mib). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152011/new/

[Lldb-commits] [PATCH] D152012: [lldb/crashlog] Expand crash report file path before parsing

This revision was automatically updated to reflect the committed changes. Closed by commit rG3bc0baf9d439: [lldb/crashlog] Expand crash report file path before parsing (authored by mib). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152012/new/ http

[Lldb-commits] [PATCH] D152013: [lldb/Commands] Fix disk completion from root directory

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe8966125e281: [lldb/Commands] Fix disk completion from root directory (authored by mib). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[Lldb-commits] [PATCH] D151859: [lldb/Target] Add ability to set a label to targets

mib added a comment. > Could you take a look? If it takes some time to fix, could you revert this > change please? @haowei bcfd85a25857294cb4a5cfa3f616f57a6911cda6 should fix the test failure. Let me know if there is another

[Lldb-commits] [PATCH] D152319: [lldb] Run crashlog inside lldb when invoked in interactive mode from the command line

mib accepted this revision. mib added a comment. LGTM! Thanks for taking care of this :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152319/new/ https://reviews.llvm.org/D152319 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http

[Lldb-commits] [PATCH] D152582: [lldb] Change return type of UnixSignals::GetShortName

mib added a comment. Don't you need to update the callers as well ? Otherwise LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152582/new/ https://reviews.llvm.org/D152582 ___ lldb-commits mailing lis

[Lldb-commits] [PATCH] D152569: [lldb] Introduce a tool to quickly generate projects with an arbitrary number of sources

mib added inline comments. Comment at: lldb/scripts/generate-project.py:23 +with open(header_path, "w") as f: +file_contents = ( +f"#ifndef _OBJ{index}_H\n" nit: Is it necessary to put it in a variable or could you have a multiline string

[Lldb-commits] [PATCH] D152594: [lldb] Introduce DynamicRegisterInfo::CreateFromDict

mib added a comment. Are we replacing all the constructors that can fail by static methods ? Wouldn't it be easier to pass an `Status&` as an extra parameter ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152594/new/ https://reviews.llvm.org/D152

[Lldb-commits] [PATCH] D152597: [lldb][NFCI] Remove StructuredData::Dictionary::GetValueForKeyAsString overloads involving ConstString

mib accepted this revision. mib added a comment. This revision is now accepted and ready to land. I thought `StructuredData::String` used a `ConstString` for storage but it actually uses a `std::string` instead. Makes sense to remove this then. LGTM! Repository: rG LLVM Github Monorepo CHANG

[Lldb-commits] [PATCH] D152842: [lldb] Improve corefile saving ergonomics

mib created this revision. mib added reviewers: bulbazord, jasonmolenda. mib added a project: LLDB. Herald added a subscriber: JDevlieghere. Herald added a project: All. mib requested review of this revision. Herald added a subscriber: lldb-commits. This patch improves the way the user can save th

[Lldb-commits] [PATCH] D152848: [lldb] Fix failure in TestStackCoreScriptedProcess on x86_64

mib created this revision. mib added reviewers: bulbazord, jingham, JDevlieghere. mib added a project: LLDB. Herald added a subscriber: pengfei. Herald added a project: All. mib requested review of this revision. Herald added a subscriber: lldb-commits. This patch should address the failure of Tes

[Lldb-commits] [PATCH] D152848: [lldb] Fix failure in TestStackCoreScriptedProcess on x86_64

mib updated this revision to Diff 531017. mib added a comment. Re-enable test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152848/new/ https://reviews.llvm.org/D152848 Files: lldb/include/lldb/Utility/StructuredData.h lldb/source/Plugins/Process/scripted/ScriptedThread.cpp lldb/

[Lldb-commits] [PATCH] D152846: [lldb][NFCI] Remove custom matcher classes in Listener in favor of lambdas

mib added inline comments. Comment at: lldb/source/Utility/Listener.cpp:214 } else { -pos = std::find_if(m_events.begin(), m_events.end(), - EventMatcher(broadcaster, event_type_mask)); +pos = std::find_if(m_events.begin(), m_events.end(), event_m

[Lldb-commits] [PATCH] D152842: [lldb] Improve corefile saving ergonomics

This revision was automatically updated to reflect the committed changes. Closed by commit rGec05cf50daec: [lldb] Improve corefile saving ergonomics (authored by mib). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152842/new/ https://reviews.llvm.or

[Lldb-commits] [PATCH] D152886: [lldb] Make it easier to spot if sources were resolved in crashlog output

mib added inline comments. Comment at: lldb/examples/python/crashlog.py:340-343 +for key in path_remapping: +source_path = os.path.expanduser( +path_remapping[key] +

[Lldb-commits] [PATCH] D152886: [lldb] Make it easier to spot if sources were resolved in crashlog output

mib added inline comments. Comment at: lldb/examples/python/crashlog.py:344 +) +if os.path.exists(source_path): +self.resolved_source = True JDevlieghe

[Lldb-commits] [PATCH] D152870: [lldb][NFCI] Remove StructuredData::Array::GetItemAtIndexAsString overloads with ConstString

mib added inline comments. Comment at: lldb/source/Target/DynamicRegisterInfo.cpp:204 +m_sets.push_back( +{ConstString(set_name).AsCString(), nullptr, 0, nullptr}); } else { I guess `m_sets` is a vector of `char*` ... Should we change i

[Lldb-commits] [PATCH] D152886: [lldb] Make it easier to spot if sources were resolved in crashlog output

mib accepted this revision. mib added a comment. This revision is now accepted and ready to land. LGTM! Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152886/new/ https://reviews.llvm.org/D152886 ___ lldb-commits mailing list lldb-comm

[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

mib added inline comments. Comment at: lldb/include/lldb/Host/common/ZipFileResolver.h:30 + + static bool ResolveBionicPath(const FileSpec &file_spec, FileKind &file_kind, +std::string &file_path, This function name sounds like th

[Lldb-commits] [PATCH] D152759: [lldb][Android] Support zip .so file

mib added inline comments. Comment at: lldb/include/lldb/Host/common/ZipFileResolver.h:30 + + static bool ResolveBionicPath(const FileSpec &file_spec, FileKind &file_kind, +std::string &file_path, mib wrote: > This function name s

[Lldb-commits] [PATCH] D152848: [lldb] Fix failure in TestStackCoreScriptedProcess on x86_64

This revision was automatically updated to reflect the committed changes. Closed by commit rG0c5b6320716b: [lldb] Fix failure in TestStackCoreScriptedProcess on x86_64 (authored by mib). Changed prior to commit: https://reviews.llvm.org/D152848?vs=531017&id=57#toc Repository: rG LLVM Git

[Lldb-commits] [PATCH] D153673: [lldb][NFCI] Remove unneeded ConstString constructions for OptionValueProperties::AppendProperty

mib accepted this revision. mib added a comment. LGTM! May be removing the implicit conversion method to StringRef would make your unnecessary ConstString removal quest easier 😅 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153673/new/ https://rev

[Lldb-commits] [PATCH] D153918: [lldb][NFCI] Deprecate SBValue::GetOpaqueType

mib added inline comments. Comment at: lldb/include/lldb/API/SBValue.h:286 + LLDB_DEPRECATED("SBValue::GetOpaqueType() is deprecated. Do not use.", "") void *GetOpaqueType(); JDevlieghere wrote: > Similar comment as in D153900: The "Do not use." part is red

[Lldb-commits] [PATCH] D153928: [lldb] Introduce a macro to mark methods as unsupported with no replacement

mib added inline comments. Comment at: lldb/include/lldb/lldb-defines.h:134-140 +#if defined(__clang__) +#define LLDB_UNSUPPORTED(MSG) \ + __attribute__((deprecated("This method is no longer supported: " MSG, ""))) +#else +#defin

[Lldb-commits] [PATCH] D154169: add a target dump section-load-list command for inspecting the Target's SectionLoadList

mib accepted this revision. mib added a comment. This revision is now accepted and ready to land. That's indeed very helpful! LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154169/new/ https://reviews.llvm.org/D154169

[Lldb-commits] [PATCH] D154169: add a target dump section-load-list command for inspecting the Target's SectionLoadList

mib added inline comments. Comment at: lldb/source/Target/SectionLoadList.cpp:265-266 ++pos) { -s.Printf("addr = 0x%16.16" PRIx64 ", section = %p: ", pos->first, - static_cast(pos->second.get())); pos->second->Dump(s.AsRawOstream(), s.GetIndentLevel()

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

mib created this revision. mib added reviewers: JDevlieghere, bulbazord, jingham. mib added a project: LLDB. Herald added a project: All. mib requested review of this revision. Herald added a subscriber: lldb-commits. This patch should fix some data races when a python script (i.e. a Scripted Proc

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

mib updated this revision to Diff 536514. mib edited the summary of this revision. mib added a comment. Address @JDevlieghere comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154271/new/ https://reviews.llvm.org/D154271 Files: lldb/source/Plugins/ScriptInterpreter/Python/Script

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

mib added a comment. In D154271#4466339 , @JDevlieghere wrote: > In D154271#4466170 , @bulbazord > wrote: > >> Making `m_lock_count`'s type into `std::atomic` makes sense to me, >> but I'm a little confused abo

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

mib updated this revision to Diff 536540. mib edited the summary of this revision. mib added a comment. Use `std::atomic::fetch_{add, sub}` to address @JDevlieghere feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154271/new/ https://reviews.llvm.org/D154271 Files: lldb/source

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

mib updated this revision to Diff 536541. mib added a comment. Fix typo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154271/new/ https://reviews.llvm.org/D154271 Files: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h lldb/source/Target/Process.cpp Inde

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

mib updated this revision to Diff 536542. mib added a subscriber: jasonmolenda. mib added a comment. Fix major typo, thanks @jasonmolenda ;) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154271/new/ https://reviews.llvm.org/D154271 Files: lldb/source/Plugins/ScriptInterpreter/Python/

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

mib added inline comments. Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h:386 +// to add 1 to its return value. +return m_lock_count.fetch_add(1, std::memory_order_relaxed) + 1; + } JDevlieghere wrote: > Sorry if

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

mib updated this revision to Diff 536573. mib added a comment. Make `DecrementLockCount` actually thread-safe and revert `IncrementLockCount` to its original implementation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154271/new/ https://reviews.llvm.org/D154271 Files: lldb/source

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

mib updated this revision to Diff 536850. mib added a comment. After chatting with Jonas, it turns out that my previous implementation didn't actually address the underflow issue using atomics, so we have to revert to using a `std::mutex` when incrementing/decrementing the python lock counter.

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

mib updated this revision to Diff 536859. mib added a comment. Address @JDevlieghere last comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154271/new/ https://reviews.llvm.org/D154271 Files: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h lldb/sourc

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9987646057d9: [lldb] Fix data race when interacting with python scripts (authored by mib). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D153900: [lldb] Deprecate SBHostOS threading functionality

mib added a comment. LGTM! @bulbazord when you land this, make sure to close @teemperor's diff (D104231 ). Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153900/new/ https://reviews.llvm.org/D153900 __

[Lldb-commits] [PATCH] D154534: [lldb][NFCI] Minor cleanups to StructuredData::GetObjectForDotSeparatedPath

mib accepted this revision. mib added a comment. LGTM with nits. Comment at: lldb/source/Utility/StructuredData.cpp:125 +if (match.second.empty()) return this->shared_from_this(); + Comment at: lldb/source/Utility/StructuredData.c

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

mib added a comment. This looks very cool! I left "a few" comments on how I think we can simplify this patch, and also had some remarks regarding the `in_process_target` sanity checks. Comment at: lldb/include/lldb/Core/Debugger.h:435-436 + ReportInterruption(Interruptio

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

mib added a comment. I had an offline chat with Jim and I misunderstood originally, what he was trying to achieve with the `InterruptionReport:: m_function_name` so my suggestion of using `LLVM_PRETTY_FUNCTION` doesn't actually work here. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

mib added a comment. Following @JDevlieghere question in https://reviews.llvm.org/D107585#2979988, here are some explanations: In order to read memory from the ScriptedProcess, the process plugin calls the python `read_memory_at_address` method passing the address at which we should start read

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

mib added a comment. ping ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108953/new/ https://reviews.llvm.org/D108953 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

mib added a comment. ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107585/new/ https://reviews.llvm.org/D107585 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

mib marked 7 inline comments as done. mib added inline comments. Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:105 +const char *ScriptedThread::GetName() { + if (m_name.empty()) { +CheckInterpreterAndScriptObject(); JDevlieghere wrote:

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

mib updated this revision to Diff 376502. mib added a comment. Address @JDevlieghere comments: - Remove "caching" - Refactor `error_with_message` lambda and `StructuredData` objects sanity checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1075

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

mib updated this revision to Diff 376504. mib added a comment. Reformat code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107585/new/ https://reviews.llvm.org/D107585 Files: lldb/bindings/python/python-wrapper.swig lldb/examples/python/scrip

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

mib added a comment. ping @JDevlieghere Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108953/new/ https://reviews.llvm.org/D108953 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

mib updated this revision to Diff 376507. mib added a comment. Pass `__PRETTY_FUNCTION__` to `CheckStructuredDataObject` to log the caller function name. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107585/new/ https://reviews.llvm.org/D107585 F

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

mib updated this revision to Diff 376508. mib added a comment. Use `ScriptedInterface::ErrorWithMessage` helper function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108953/new/ https://reviews.llvm.org/D108953 Files: lldb/bindings/interface/S

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

mib updated this revision to Diff 376509. mib added a comment. Update error message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108953/new/ https://reviews.llvm.org/D108953 Files: lldb/bindings/interface/SBMemoryRegionInfo.i lldb/bindings/i

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

mib marked 2 inline comments as done. mib added inline comments. Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:186 +void ScriptedThread::RefreshStateAfterStop() { + // TODO: Implement + if (m_reg_context_sp) JDevlieghere wrote: > Still rel

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

mib updated this revision to Diff 377324. mib marked 2 inline comments as done. mib added a comment. Address @JDevlieghere last comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107585/new/ https://reviews.llvm.org/D107585 Files: lldb/bind

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

mib updated this revision to Diff 377326. mib edited the summary of this revision. mib added a comment. Reverted from `shared_ptr` to `unique_ptr` for SBMemoryRegionInfo opaque_ptr. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108953/new/ https://

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

mib added a comment. ping @JDevlieghere Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108953/new/ https://reviews.llvm.org/D108953 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

mib added inline comments. Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:278 +if (error.Fail()) + return error; + JDevlieghere wrote: > If this is the only way out of this loop, does that mean we always return an > error here? In

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

mib updated this revision to Diff 377724. mib added a comment. Return an Optional to stop fetching memory regions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108953/new/ https://reviews.llvm.org/D108953 Files: lldb/bindings/interface/SBMemory

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG59d8dd79e1f9: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess (authored by mib). Changed prior to commit: https://reviews.llvm.or

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa758c9f7204c: [lldb/Plugins] Add memory region support in ScriptedProcess (authored by mib). Changed prior to commit: https://reviews.llvm.org/D10

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

mib added a comment. In D111409#3051140 , @d-millar wrote: > Am obviously brand new to your process and a bit of an old dog when it comes > to learning new tricks. Would you prefer I make a new submission with the > -U99 diff? Also, am more than

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

mib added a comment. In D107585#3051607 , @stella.stamenova wrote: > This broke the windows lldb bot: > > https://lab.llvm.org/buildbot/#/builders/83/builds/10836 C:\buildbot\lldb-x64-windows-ninja\llvm-project\lldb\source\Plugins\Process\scripted\

[Lldb-commits] [PATCH] D111409: proposed support for Java interface to Scripting Bridge

mib added a comment. In D111409#3051556 , @d-millar wrote: > Hmmm, well, have tried running "git clang-format" with a few different > invocations, but seem to always get "No modified files to format." > Suggestions? I was confused by the `patch` file,

<    4   5   6   7   8   9   10   11   12   13   >