[Lldb-commits] [PATCH] D51999: build: add libedit to include paths

tatyana-krasnukha updated this revision to Diff 166507. tatyana-krasnukha added a comment. LLDB's static libraries link each other as INTERFACE, that's why CMake doesn't propagate lldbHost's include directories to lldbCore and lldbInterpreter. For lldbHost itself, I had to use PUBLIC, because its

[Lldb-commits] [PATCH] D48623: Move architecture-specific address adjustment to architecture plugins.

tatyana-krasnukha updated this revision to Diff 166520. Repository: rLLDB LLDB https://reviews.llvm.org/D48623 Files: include/lldb/Core/Architecture.h source/Plugins/Architecture/Arm/ArchitectureArm.cpp source/Plugins/Architecture/Arm/ArchitectureArm.h source/Plugins/Architecture/CMake

[Lldb-commits] [PATCH] D48623: Move architecture-specific address adjustment to architecture plugins.

tatyana-krasnukha added a comment. Thanks for your patience;) Repository: rLLDB LLDB https://reviews.llvm.org/D48623 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52376: [API][Swig] Overloaded functions for SBTarget

tatyana-krasnukha created this revision. tatyana-krasnukha added reviewers: spyffe, clayborg, zturner. tatyana-krasnukha added a project: LLDB. Herald added a subscriber: lldb-commits. Slightly refactored typemaps to combine overloading with default arguments. Repository: rLLDB LLDB https://r

[Lldb-commits] [PATCH] D52376: [Swig] Merge typemaps with same bodies

tatyana-krasnukha updated this revision to Diff 166719. tatyana-krasnukha retitled this revision from "[API][Swig] Overloaded functions for SBTarget" to "[Swig] Merge typemaps with same bodies". tatyana-krasnukha edited the summary of this revision. tatyana-krasnukha removed a reviewer: spyffe. ta

[Lldb-commits] [PATCH] D52139: [lldb-mi] Fix hanging of target-select-so-path.test

tatyana-krasnukha accepted this revision. tatyana-krasnukha added a comment. This revision is now accepted and ready to land. A lot of tests are failing with Python 3, at least on CentOS. So, I agree the problem doesn't block this review. https://reviews.llvm.org/D52139 _

[Lldb-commits] [PATCH] D52498: [lldb-mi] Fix bugs in target-select-so-path.test

tatyana-krasnukha added a comment. `pass_fds` parameter was added in Python 3.7. With version 3.6 the test fails with a `TypeError`. https://reviews.llvm.org/D52498 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi

[Lldb-commits] [PATCH] D52498: [lldb-mi] Fix bugs in target-select-so-path.test

tatyana-krasnukha added a comment. It seems I ran into an unrelated issue like this . If the revision works for others, you may ignore this. https://reviews.llvm.org/D52498 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D52498: [lldb-mi] Fix bugs in target-select-so-path.test

tatyana-krasnukha added a comment. Forget that the test calls default python, not which was passed to cmake. So, it is enough to replace (3,0) pair with (3,2) to fix. Comment at: lit/tools/lldb-mi/target/inputs/target-select-so-path.py:11 +kwargs = {} +if sys.version_info >= (

[Lldb-commits] [PATCH] D52498: [lldb-mi] Fix bugs in target-select-so-path.test

tatyana-krasnukha added a comment. Now works with Python 3.1 too. LGTM. https://reviews.llvm.org/D52498 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49739: Add new API to SBTarget class

tatyana-krasnukha added inline comments. Comment at: lit/tools/lldb-mi/target/target-select-so-path.test:4 +# RUN: %cc -o %t %p/inputs/main.c -g +# RUN: python %p/inputs/target-select-so-path.py "%debugserver" "%lldbmi %t" %s + Could you run the same python that

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

tatyana-krasnukha created this revision. tatyana-krasnukha added reviewers: clayborg, zturner, jingham. tatyana-krasnukha added a project: LLDB. Herald added subscribers: lldb-commits, teemperor, JDevlieghere. As a follow-up to https://reviews.llvm.org/D49017. Repository: rLLDB LLDB https://r

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

tatyana-krasnukha added a comment. Hmm... There is no such logic left, isn't it? If you mean iterating OptionValueProperties::m_properties vector, I can replace cycles with range based for loop, but this doesn't seem to be related to these changes. Repository: rLLDB LLDB https://reviews.llv

[Lldb-commits] [PATCH] D52572: Replace pointer to C-array of PropertyDefinition with llvm::ArrayRef

tatyana-krasnukha added a comment. It seems you are speaking about OptionDefinition, not PropertyDefinition. I was going to create a separate revision for it. Should I add OptionDefinition related changes to here? Repository: rLLDB LLDB https://reviews.llvm.org/D52572 ___

[Lldb-commits] [PATCH] D52604: Clean-up usage of OptionDefinition arrays

tatyana-krasnukha created this revision. tatyana-krasnukha added reviewers: clayborg, zturner. tatyana-krasnukha added a project: LLDB. Herald added a subscriber: lldb-commits. Removed unused static arrays from headers, removed sentinel entry from OptionDefinition array in Driver.cpp. Repositor

[Lldb-commits] [PATCH] D52604: Clean-up usage of OptionDefinition arrays

tatyana-krasnukha added inline comments. Comment at: tools/driver/Driver.cpp:71 typedef struct { uint32_t usage_mask; // Used to mark options that can be used together. If (1 This type is duplicated here to not break the encapsulation of lldb's private t

[Lldb-commits] [PATCH] D52604: Clean-up usage of OptionDefinition arrays

tatyana-krasnukha updated this revision to Diff 167475. tatyana-krasnukha added a comment. Modifies Driver::ParseArgs checks Repository: rLLDB LLDB https://reviews.llvm.org/D52604 Files: include/lldb/Target/Platform.h source/Commands/CommandObjectDisassemble.h source/Commands/CommandOb

[Lldb-commits] [PATCH] D52604: Clean-up usage of OptionDefinition arrays

tatyana-krasnukha added inline comments. Comment at: tools/driver/Driver.cpp:581-585 + if (g_num_options == 0) { if (argc > 1) error.SetErrorStringWithFormat("invalid number of options"); return error; } clayborg wrote: > Do we even need this

[Lldb-commits] [PATCH] D54084: [LLDB] Fix FreeBSD build

tatyana-krasnukha added inline comments. Comment at: source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp:378 namespace fs = llvm::sys::fs; - if (working_dir && (!working_dir.ResolvePath() || - !fs::is_directory(working_dir.GetPath( { + if (working_dir

[Lldb-commits] [PATCH] D54084: [LLDB] Fix FreeBSD build

tatyana-krasnukha added a comment. Could you please make the same fix for NativeProcessDarwin (`NativeProcessProtocol::Launch`) as well? Repository: rLLDB LLDB https://reviews.llvm.org/D54084 ___ lldb-commits mailing list lldb-commits@lists.llvm

[Lldb-commits] [PATCH] D54084: [LLDB] Fix FreeBSD/Darwin build

tatyana-krasnukha accepted this revision. tatyana-krasnukha added a comment. This revision is now accepted and ready to land. Thank you! LGTM, though I'm not able to test it under any of these systems. https://reviews.llvm.org/D54084 ___ lldb-commit

[Lldb-commits] [PATCH] D55472: Speadup memory regions handling and cleanup related code

tatyana-krasnukha created this revision. tatyana-krasnukha added a reviewer: clayborg. tatyana-krasnukha added a project: LLDB. Herald added subscribers: lldb-commits, abidh. - replace unnecessary shared pointers with unique pointers - reserve space before filling a vector with 'push_back' in a lo

[Lldb-commits] [PATCH] D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available.

tatyana-krasnukha added a comment. Without overriding Process::GetMemoryRegions still does a lot of unnecessary job (FindMemoryRegion for each element in the vector). With these changes, it may just return m_regions (if not empty), doesn't it? Comment at: source/Plugins/Proces

[Lldb-commits] [PATCH] D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available.

tatyana-krasnukha added a comment. Don't you mind to take overridden GetMemoryRegions in this patch so that I can remove all minidump-related changes from D55472 ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55522/new/ https://reviews.llvm.org/D55522

[Lldb-commits] [PATCH] D55472: Speadup memory regions handling and cleanup related code

tatyana-krasnukha added a comment. Thanks to all for comments! As I wrote in inline comments, 'non-const' -> 'const' changes don't seem to break anything, I'd like to leave it here. Correct me, if I'm wrong. The second question is whether it is allowed to change private member functions of SBX

[Lldb-commits] [PATCH] D55653: Check pointer results on nullptr before using them

tatyana-krasnukha created this revision. tatyana-krasnukha added reviewers: ki.stfu, abidh. Herald added a subscriber: lldb-commits. Prevent crashing like llvm.org/pr37054 Repository: rLLDB LLDB https://reviews.llvm.org/D55653 Files: MICmdCmdData.cpp MICmdCmdGdbInfo.cpp MICmdCmdMiscell

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

tatyana-krasnukha created this revision. tatyana-krasnukha added reviewers: clayborg, jasonmolenda. tatyana-krasnukha added a project: LLDB. Herald added subscribers: lldb-commits, mgorny. Add ARC architecture (bare-metal) that can be debugged through an RSP-server. Repository: rLLDB LLDB htt

[Lldb-commits] [PATCH] D55724: [ARC] Add SystemV ABI

tatyana-krasnukha created this revision. tatyana-krasnukha added a reviewer: clayborg. tatyana-krasnukha added a project: LLDB. Herald added subscribers: lldb-commits, teemperor, mgorny. Repository: rLLDB LLDB https://reviews.llvm.org/D55724 Files: source/API/SystemInitializerFull.cpp sour

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

tatyana-krasnukha marked 2 inline comments as done. tatyana-krasnukha added a comment. My intent was to move ARC-specific code to the architecture plugin as much as possible, but it requires to add undesired dependencies to Architecture interface. So, you are right, it seems to be better to keep

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

tatyana-krasnukha updated this revision to Diff 178643. tatyana-krasnukha added a comment. Addressed review comments Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55718/new/ https://reviews.llvm.org/D55718 Files: include/lldb/Utility/ArchSpec.h source/Plug

[Lldb-commits] [PATCH] D55827: Update current working directory to avoid test crashes

tatyana-krasnukha created this revision. tatyana-krasnukha added reviewers: JDevlieghere, zturner. tatyana-krasnukha added a project: LLDB. Herald added subscribers: lldb-commits, abidh. Running `dotest.py` with a path to tests directory results in: terminate called after throwing an instance o

[Lldb-commits] [PATCH] D55472: Speadup memory regions handling and cleanup related code

tatyana-krasnukha updated this revision to Diff 178690. tatyana-krasnukha added a comment. Replace vector of unique pointers with vector of values, revert some API changes, remove MinidumpParser changes. I will put overridden GetMemoryRegions in a separate patch. Repository: rLLDB LLDB CHANG

[Lldb-commits] [PATCH] D55472: Speadup memory regions handling and cleanup related code

tatyana-krasnukha updated this revision to Diff 178732. tatyana-krasnukha marked an inline comment as done. tatyana-krasnukha added a comment. Addressed more comments Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55472/new/ https://reviews.llvm.org/D55472 File

[Lldb-commits] [PATCH] D55841: GetMemoryRegions for the ProcessMinidump

tatyana-krasnukha created this revision. tatyana-krasnukha added a reviewer: clayborg. tatyana-krasnukha added a project: LLDB. Herald added a subscriber: lldb-commits. Moved from D55472 Repository: rLLDB LLDB https://reviews.llvm.org/D55841 Files: C:/Rep

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

tatyana-krasnukha marked an inline comment as not done. tatyana-krasnukha added a comment. `ARCflags` are used by ABISysV_arc (related patch D55724 ). I would be glad to move it to architecture plugin, but I ought to add SetFlags/GetFlags to Architecture interfac

[Lldb-commits] [PATCH] D55472: Speadup memory regions handling and cleanup related code

tatyana-krasnukha updated this revision to Diff 178883. tatyana-krasnukha added a comment. Minor change: replace `using` with `typedef` since old swig versions don't understand c++11's aliases. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55472/new/ https://r

[Lldb-commits] [PATCH] D55841: GetMemoryRegions for the ProcessMinidump

tatyana-krasnukha updated this revision to Diff 178916. tatyana-krasnukha added a comment. Add a test Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55841/new/ https://reviews.llvm.org/D55841 Files: packages/Python/lldbsuite/test/functionalities/postmortem/m

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

tatyana-krasnukha added inline comments. Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4707 + +arc::AdjustRegisterInfo(m_register_info, arch_to_use); } clayborg wrote: > ok, makes sense. Clearing the register info would be a go

[Lldb-commits] [PATCH] D55841: GetMemoryRegions for the ProcessMinidump

tatyana-krasnukha updated this revision to Diff 178931. tatyana-krasnukha added a comment. Removed dump file from the patch Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55841/new/ https://reviews.llvm.org/D55841 Files: packages/Python/lldbsuite/test/functi

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

tatyana-krasnukha updated this revision to Diff 178933. tatyana-krasnukha added a comment. Clear `m_register_info` if it is incorrect. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55718/new/ https://reviews.llvm.org/D55718 Files: include/lldb/Utility/ArchSp

[Lldb-commits] [PATCH] D55472: Speadup memory regions handling and cleanup related code

tatyana-krasnukha updated this revision to Diff 178941. tatyana-krasnukha added a comment. Rid off of including std::vector at the cost of adding a wrapper class that can be forward declared. Without this, it is not possible to do https://reviews.llvm.org/D55472#inline-491159 Repository: rLL

[Lldb-commits] [PATCH] D51569: Hold GIL while allocating memory for PythonString.

tatyana-krasnukha created this revision. tatyana-krasnukha added reviewers: clayborg, granata.enrico. Herald added a subscriber: lldb-commits. Swig wraps C++ code into `SWIG_PYTHON_THREAD_BEGIN_ALLOW; ... SWIG_PYTHON_THREAD_END_ALLOW;` Thus, lldb crashs with "Fatal Python error: Python memory a

[Lldb-commits] [PATCH] D51569: Hold GIL while allocating memory for PythonString.

tatyana-krasnukha updated this revision to Diff 163646. tatyana-krasnukha added a comment. %feature("nothreadallow") looks more appropriate in this case. GetDescription method is quite cheap, no need to handle the GIL. Repository: rLLDB LLDB https://reviews.llvm.org/D51569 Files: scripts/

[Lldb-commits] [PATCH] D51594: [ARC] Make char unsigned by default

tatyana-krasnukha accepted this revision. tatyana-krasnukha added a subscriber: clayborg. tatyana-krasnukha added a comment. This revision is now accepted and ready to land. Good to me, if no one is confused about absent general support of ARC target. Please, add @clayborg to reviewers. Reposit

[Lldb-commits] [PATCH] D51569: Hold GIL while allocating memory for PythonString.

tatyana-krasnukha added a comment. Yes, and `%feature("nothreadallow")` doesn't allow to release GIL before calling these functions - it prevents Swig from wrapping them into SWIG_PYTHON_THREAD_BEGIN_ALLOW .. SWIG_PYTHON_THREAD_END_ALLOW block. Repository: rLLDB LLDB https://reviews.llvm.o

[Lldb-commits] [PATCH] D51569: Hold GIL while allocating memory for PythonString.

tatyana-krasnukha added a comment. I have a third party module which implements a custom command for LLDB. The command calls `str(target)`, `str(process)`, etc. for logging. Anyway, every call to these functions will cause this crash to happen, because PythonString is allocated after releasing

[Lldb-commits] [PATCH] D51569: Hold GIL while allocating memory for PythonString.

tatyana-krasnukha updated this revision to Diff 164008. tatyana-krasnukha added a comment. Use more readable aliases to set end clear the feature. Repository: rLLDB LLDB https://reviews.llvm.org/D51569 Files: scripts/Python/python-extensions.swig Index: scripts/Python/python-extensions.sw

[Lldb-commits] [PATCH] D51999: build: add libedit to include paths

tatyana-krasnukha created this revision. tatyana-krasnukha added a reviewer: compnerd. Herald added subscribers: lldb-commits, mgorny. Without that build of Host, Core and Interpreter with custom libedit fails. Repository: rLLDB LLDB https://reviews.llvm.org/D51999 Files: CMakeLists.txt

[Lldb-commits] [PATCH] D51999: build: add libedit to include paths

tatyana-krasnukha added a comment. Everything that includes Host/Editline.h (Host/Editline.cpp, Core/IOHandler.cpp, ...). Build fails with "fatal error: histedit.h: No such file or directory" Repository: rLLDB LLDB https://reviews.llvm.org/D51999 _

[Lldb-commits] [PATCH] D51999: build: add libedit to include paths

tatyana-krasnukha added a comment. TBC, I pass libedit_INCLUDE_DIRS and libedit_LIBRARIES manually to CMake. Repository: rLLDB LLDB https://reviews.llvm.org/D51999 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cg

[Lldb-commits] [PATCH] D48802: [lldb-mi] Re-implement symbol-list-lines command.

tatyana-krasnukha added a comment. symbol-list-lines.test fails on Linux too, input is compiled with gcc. Comment at: lldb/trunk/lit/tools/lldb-mi/symbol/inputs/symbol-list-lines.c:6 + return 0; +} The check doesn't count this line Comment a

[Lldb-commits] [PATCH] D49739: Add new API to SBTarget class

tatyana-krasnukha added a comment. On CentOS this test hangs forever. If I manually kill lldb-mi and lldb-server subprocesses, the output is -- Exit Code: 1 Command Output (stderr): -- /lldb/lit/tools/lldb-mi/target/target-select-so-path.test:16:10: error: CHECK: expected string no

[Lldb-commits] [PATCH] D51999: build: add libedit to include paths

tatyana-krasnukha updated this revision to Diff 166258. tatyana-krasnukha added a comment. Unfortunately, doing this only on `lldbHost` is not enough, because CMake doesn't propagate include directories to dependent projects. I added these lines to every project that includes "Editline.h". Rep

[Lldb-commits] [PATCH] D52139: [lldb-mi] Fix hanging of target-select-so-path.test

tatyana-krasnukha accepted this revision. tatyana-krasnukha added a comment. This revision is now accepted and ready to land. These changes fixed the issue for me. https://reviews.llvm.org/D52139 ___ lldb-commits mailing list lldb-commits@lists.llvm

[Lldb-commits] [PATCH] D52139: [lldb-mi] Fix hanging of target-select-so-path.test

tatyana-krasnukha requested changes to this revision. tatyana-krasnukha added a comment. This revision now requires changes to proceed. Alexander, please add a timeout to be sure that the test is not hanging even if something goes wrong. A possible solution for Python 2: F7266803: target-select-s

[Lldb-commits] [PATCH] D48623: Move architecture-specific address adjustment to architecture plugins.

tatyana-krasnukha updated this revision to Diff 166342. tatyana-krasnukha retitled this revision from "WIP: Move architecture-specific address adjustment to architecture plugins." to "Move architecture-specific address adjustment to architecture plugins.". tatyana-krasnukha added a comment. Heral

[Lldb-commits] [PATCH] D48623: Move architecture-specific address adjustment to architecture plugins.

tatyana-krasnukha updated this revision to Diff 166475. tatyana-krasnukha added a comment. Make overriding functions public. Repository: rLLDB LLDB https://reviews.llvm.org/D48623 Files: include/lldb/Core/Architecture.h include/lldb/Target/Target.h source/Core/Address.cpp source/Plug

[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function

tatyana-krasnukha created this revision. Comments in this function didn't match code. Moved it in right place and grouped cases, that need only WriteMemoryPrivate call, at the top of the function. Applied clang-format. Repository: rL LLVM https://reviews.llvm.org/D39967 Files: source/Tar

[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function

tatyana-krasnukha added inline comments. Comment at: source/Target/Process.cpp:2462 - // We weren't able to write all of the requested bytes, we - // are done looping and will return the number of bytes that - // we have written so far.

[Lldb-commits] [PATCH] D39969: Set error status in ObjectFile::LoadInMemory if it is not set

tatyana-krasnukha created this revision. Herald added a subscriber: ki.stfu. When breakpoints intersect a section, process->WriteMemory returns 0 without setting the error, LoadInMemory breaks execution and returns this error, but client code treat it as success. Just added setting of error sta

[Lldb-commits] [PATCH] D40022: Corrected two typos and removed unused variable

tatyana-krasnukha created this revision. Herald added a subscriber: ki.stfu. + clang-format Repository: rL LLVM https://reviews.llvm.org/D40022 Files: include/lldb/Core/Architecture.h source/Commands/CommandObjectTarget.cpp source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp

[Lldb-commits] [PATCH] D40022: Corrected two typos and removed unused variable

tatyana-krasnukha added inline comments. Comment at: source/Commands/CommandObjectTarget.cpp:2589 "Write file contents to the memory.", false, true), -m_pc_option(LLDB_OPT_SET_1, false, "--set-pc-to-entry", 'p', "Set PC to the e

[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function

tatyana-krasnukha updated this revision to Diff 122966. tatyana-krasnukha added a comment. Sorry for wrong formatting, I've removed it. What I've actually done: - grouped cases returning WriteMemoryPrivate in the top of function; - moved last comment to code it relates; - change other comment ac

[Lldb-commits] [PATCH] D39969: Set error status in ObjectFile::LoadInMemory if it is not set

tatyana-krasnukha updated this revision to Diff 122967. tatyana-krasnukha added a comment. Removed clang-format changes. Repository: rL LLVM https://reviews.llvm.org/D39969 Files: source/Symbol/ObjectFile.cpp Index: source/Symbol/ObjectFile.cpp ===

[Lldb-commits] [PATCH] D40022: Corrected two typos and removed unused variable

tatyana-krasnukha added a comment. I have not commit access, that is why I've decided to group all typos in one patch... Repository: rL LLVM https://reviews.llvm.org/D40022 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.l

[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function

tatyana-krasnukha updated this revision to Diff 122972. tatyana-krasnukha added a comment. Same for comment in the header file. Repository: rL LLVM https://reviews.llvm.org/D39967 Files: include/lldb/Target/Process.h source/Target/Process.cpp Index: source/Target/Process.cpp ===

[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function

tatyana-krasnukha added a comment. Still have a feeling that it is not enough to clarify for function users that "if (bytes_written != bytes_to_write)" is not always criteria of error, as well as "if (error.Success())" doesn't mean that whole area was written to memory... Repository: rL LLVM

[Lldb-commits] [PATCH] D39969: Set error status in ObjectFile::LoadInMemory if it is not set

tatyana-krasnukha added inline comments. Comment at: source/Symbol/ObjectFile.cpp:695 + "One or more breakpoints intersect section '%s'", + section_sp->GetName().AsCString()); return error; abidh wrote: > If WriteMemory is not s

[Lldb-commits] [PATCH] D40022: Remove extra minuses from command option

tatyana-krasnukha updated this revision to Diff 123942. tatyana-krasnukha retitled this revision from "Corrected two typos and removed unused variable" to "Remove extra minuses from command option". tatyana-krasnukha edited the summary of this revision. tatyana-krasnukha added a comment. Things t

[Lldb-commits] [PATCH] D40022: Remove extra minuses from command option

tatyana-krasnukha updated this revision to Diff 123953. tatyana-krasnukha added a comment. Made diff with full paths (studying to use arcanist)... Repository: rL LLVM https://reviews.llvm.org/D40022 Files: source/Commands/CommandObjectTarget.cpp Index: source/Commands/CommandObjectTarget

[Lldb-commits] [PATCH] D41584: Check existence of each required component during construction of LLVMCDisassembler.

tatyana-krasnukha created this revision. tatyana-krasnukha added reviewers: zturner, clayborg, jingham, jasonmolenda. Actually, fix two issues: 1. remove repeat creation of reg_info, use m_reg_info_ap for createMCAsmInfo instead; 2. remove possibility to dereference nullptr during createMCAsmIn

[Lldb-commits] [PATCH] D41584: Check existence of each required component during construction of LLVMCDisassembler.

tatyana-krasnukha updated this revision to Diff 128177. https://reviews.llvm.org/D41584 Files: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp Index: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp === --- source/Plu

[Lldb-commits] [PATCH] D41584: Check existence of each required component during construction of LLVMCDisassembler.

tatyana-krasnukha added a comment. Thank you, Pavel. Would you mind if I move LLVMCDisassembler declaration in .cpp also? It looks like perfect candidate for pimpl. And... doesn't DisassemblerLLVMC::LLVMCDisassembler confuse anyone but me?) https://reviews.llvm.org/D41584 __

[Lldb-commits] [PATCH] D41584: Check existence of each required component during construction of LLVMCDisassembler.

tatyana-krasnukha updated this revision to Diff 129127. tatyana-krasnukha added a comment. Added function Create that creates an instance of LLVMCDisassembler only if pass all constraints. Moved LLVMCDisassembler declaration to .cpp file, renamed to MCDisasmToolset (is this name ok?). Added cons

[Lldb-commits] [PATCH] D41584: Check existence of each required component during construction of LLVMCDisassembler.

tatyana-krasnukha added inline comments. Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp:1002 + assert(m_instr_info && m_reg_info && m_subtarget_info && m_asm_info && + m_context && disasm && instr_printer); +} labath wrote: > this sho

[Lldb-commits] [PATCH] D41584: Check existence of each required component during construction of LLVMCDisassembler.

tatyana-krasnukha updated this revision to Diff 129268. tatyana-krasnukha added a comment. Added "_up" suffix to each unique_ptr, renamed MCDisasmToolset to MCDisasmInstance. https://reviews.llvm.org/D41584 Files: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp source/Plugins/Disass

[Lldb-commits] [PATCH] D41584: Check existence of each required component during construction of LLVMCDisassembler.

tatyana-krasnukha added a comment. There is the function GetDisasmToUse in InstructionLLVMC class that can return nullptr. But this case is not handled in any usage. I suppose that caller functions cannot be invoked if !DisassemblerLLVMC::IsValid(). But it still looks dangerous for me. May be G

[Lldb-commits] [PATCH] D41584: Check existence of each required component during construction of LLVMCDisassembler.

tatyana-krasnukha updated this revision to Diff 129425. tatyana-krasnukha added a comment. Fixed comment spacing. Changed the comment slightly (words “I added this class to…” sound now like I did this, so, replaced it with passive voice). https://reviews.llvm.org/D41584 Files: source/Plugins

[Lldb-commits] [PATCH] D39969: Set error status in ObjectFile::LoadInMemory if it is not set

tatyana-krasnukha added a comment. Herald added a subscriber: llvm-commits. ping Greg, Abid, if you disagree with the changes, let me know and I'll close the revision. Repository: rL LLVM https://reviews.llvm.org/D39969 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function

tatyana-krasnukha added a comment. Herald added a subscriber: llvm-commits. What about having 2 versions: 1. WriteMemory(..., bool force = false), that fails if there are breakpoints in the area. Let user decide what to do. If force == true just call DoWriteMemory without checking on breakpoint

[Lldb-commits] [PATCH] D39969: Set error status in ObjectFile::LoadInMemory if it is not set

tatyana-krasnukha added a comment. Yes, I see... There is some inconsistency between status and returned value. That is why I wandered about this https://reviews.llvm.org/D39967#984065 https://reviews.llvm.org/D39969 ___ lldb-commits mailing list l

[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function

tatyana-krasnukha added a comment. > Does this functionality really belong in the client? In case of memory reads, > it's the server who patches out the breakpoint opcodes > (NativeProcessProtocol::ReadMemoryWithoutTrap). I think it would make sense > to do this in the same place. Will not wor

[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function

tatyana-krasnukha added a comment. I completely agree with your point, but why isn't enough just to return an error about breakpoints in the area user wants to write? Or to disable breakpoints forcibly? https://reviews.llvm.org/D39967 ___ lldb-com

[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function

tatyana-krasnukha added a comment. Cannot promise that I'll do it (with all tests) quickly, but I'll do. One more question: what are the cases when intersection can happen, beside user forgot to disable it manually? (Will not it be annoying for user to get breakpoints in unpredictable locations

[Lldb-commits] [PATCH] D39967: Refactoring of MemoryWrite function

tatyana-krasnukha updated this revision to Diff 135370. tatyana-krasnukha added a comment. Herald added a subscriber: mgorny. Disable breakpoints before writing memory and re-enable after. I found that bp_site_list filled by BreakpointSiteList::FindInRange has its own mutex and doesn’t hold mute

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha updated this revision to Diff 135387. tatyana-krasnukha added a comment. pass callbacks by const-reference https://reviews.llvm.org/D39967 Files: include/lldb/Breakpoint/BreakpointSiteList.h source/Breakpoint/BreakpointSiteList.cpp source/Target/Process.cpp unittests/P

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha updated this revision to Diff 135397. tatyana-krasnukha added a comment. diff with more context https://reviews.llvm.org/D39967 Files: include/lldb/Breakpoint/BreakpointSiteList.h source/Breakpoint/BreakpointSiteList.cpp source/Target/Process.cpp unittests/Process/CMak

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha updated this revision to Diff 135518. tatyana-krasnukha added a comment. Use Disable|EnableBreakpointSite instead of Disable|EnableSoftwareBreakpoint. Make BreakpointSiteList::ForEach callbacks return bool. https://reviews.llvm.org/D39967 Files: include/lldb/Breakpoint/Break

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha added a comment. In https://reviews.llvm.org/D39967#1015860, @clayborg wrote: > So one issue with this is some things, like GDB servers, might not need you > to disable breakpoints in a range because they actually set the breakpoint > for you and you don't know what opcode the

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha added inline comments. Comment at: include/lldb/Breakpoint/BreakpointSiteList.h:159-175 + class Guard final { +std::recursive_mutex *m_mutex; - typedef void (*BreakpointSiteSPMapFunc)(lldb::BreakpointSiteSP &bp, -

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha added inline comments. Comment at: source/Breakpoint/BreakpointSiteList.cpp:191 if (lower != m_bp_site_list.begin()) { -collection::const_iterator prev_pos = lower; -prev_pos--; +auto prev_pos = std::prev(lower); const BreakpointSiteSP &prev_

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha updated this revision to Diff 135527. tatyana-krasnukha added a comment. Update according suggestions above https://reviews.llvm.org/D39967 Files: include/lldb/Breakpoint/BreakpointSiteList.h source/Breakpoint/BreakpointSiteList.cpp source/Target/Process.cpp unittests/

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha added a comment. In https://reviews.llvm.org/D39967#1016533, @zturner wrote: > It might be true that allowing the use of manual `lock` and `unlock` is > unsafe, but adding additional code also has some cost. Just look: only 3 additional lines of code;) https://reviews.llvm.

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha added inline comments. Comment at: source/Target/Process.cpp:2437 + for (auto &bp_site : enabled_bp_sites_in_range) +update_status(DisableSoftwareBreakpoint(bp_site.get())); xiaobai wrote: > If disabling any breakpoint fails, the status's

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha updated this revision to Diff 135533. tatyana-krasnukha added a comment. Remove Guard class completely https://reviews.llvm.org/D39967 Files: include/lldb/Breakpoint/BreakpointSiteList.h source/Breakpoint/BreakpointSiteList.cpp source/Target/Process.cpp unittests/Proce

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha updated this revision to Diff 135537. tatyana-krasnukha added a comment. Fix naming https://reviews.llvm.org/D39967 Files: include/lldb/Breakpoint/BreakpointSiteList.h source/Breakpoint/BreakpointSiteList.cpp source/Target/Process.cpp unittests/Process/CMakeLists.txt

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha updated this revision to Diff 135612. tatyana-krasnukha added a comment. Remove Optional from return type of FindInRange https://reviews.llvm.org/D39967 Files: include/lldb/Breakpoint/BreakpointSiteList.h source/Breakpoint/BreakpointSiteList.cpp source/Target/Process.cpp

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha added inline comments. Comment at: include/lldb/Breakpoint/BreakpointSiteList.h:130 + using ModifyingCallback = std::function; + bool ForEach(const ModifyingCallback &callback); clayborg wrote: > I was a bit vague with my explanations. Only

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha updated this revision to Diff 135674. tatyana-krasnukha added a comment. Change ForEach return type (bool -> void) https://reviews.llvm.org/D39967 Files: include/lldb/Breakpoint/BreakpointSiteList.h source/Breakpoint/BreakpointSiteList.cpp source/Target/Process.cpp uni

[Lldb-commits] [PATCH] D39967: Disable breakpoints before writing memory and re-enable after.

tatyana-krasnukha added a comment. May be I misunderstand your question, but I checked, that all targets of both lldb-server and debugserver read opcode from memory and save it, when enable software breakpoint. https://reviews.llvm.org/D39967 ___

  1   2   3   >