eugene added inline comments.
Comment at: include/lldb/Host/common/NativeProcessProtocol.h:104
- virtual bool GetArchitecture(ArchSpec &arch) const = 0;
+ virtual const ArchSpec &GetArchitecture() const = 0;
Why return reference instead of a value?
https:
eugene added inline comments.
Comment at: include/lldb/Host/common/NativeProcessProtocol.h:104
- virtual bool GetArchitecture(ArchSpec &arch) const = 0;
+ virtual const ArchSpec &GetArchitecture() const = 0;
labath wrote:
> eugene wrote:
> > Why return refe
eugene updated this revision to Diff 129002.
eugene edited the summary of this revision.
eugene added a reviewer: tberghammer.
eugene added a project: LLDB.
eugene added a subscriber: lldb-commits.
eugene added a comment.
Fix tests.
https://reviews.llvm.org/D41533
Files:
packages/Python/lldb
eugene marked 5 inline comments as done.
eugene added inline comments.
Comment at:
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:365-368
+static const char *DebugStateCandidates[] = {
+"_dl_debug_state", "rtld_db_dlactivity", "__dl_rtld_db_dlacti
eugene updated this revision to Diff 129216.
eugene marked an inline comment as done.
eugene added a comment.
Addressing code review comments. Switching from manual symbol resolution to
the appropriate overload of CreateBreakpoint.
https://reviews.llvm.org/D41533
Files:
packages/Python/ll
This revision was automatically updated to reflect the committed changes.
Closed by commit rL322209: Advanced guessing of rendezvous breakpoint (authored
by eugene, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D41533?vs=129216&id=129
eugene added inline comments.
Comment at: source/Host/linux/Host.cpp:129
+ auto buffer_sp = DataBufferLLVM::CreateSliceFromPath(exe_path, 0x20, 0);
+ if (buffer_sp)
+return ArchSpec();
Shouldn't it be
```
if (!buffer_sp)
```
?
https://reviews.llvm.org/D
eugene added a comment.
Greg, this name is amazing. My c++filt refuses to demangle it. We can probably
give up on parsing C++ names if they're longer than 1000 characters or
something.
Repository:
rL LLVM
https://reviews.llvm.org/D31451
___
lld
eugene created this revision.
eugene added a reviewer: labath.
Now incorrect type argument that looks like T doesn't cause an assert,
but just a parsing error.
Bug: 36224
https://reviews.llvm.org/D42939
Files:
source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
unittests/Lan
eugene updated this revision to Diff 132916.
eugene added a comment.
fix formating
https://reviews.llvm.org/D42939
Files:
source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
Index: unittests/Language/CPlusPlus/CPlusPlusLanguageT
eugene added inline comments.
Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp:199
case tok::greatergreater:
template_counter -= 2;
can_open_template = false;
labath wrote:
> While looking at the bug, this part here struck
This revision was automatically updated to reflect the committed changes.
Closed by commit rL324380: More correct handling of error cases C++ name parser
(authored by eugene, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D42939?vs=132
eugene created this revision.
eugene added reviewers: labath, jingham.
Fixing crash after "breakpoint delete". Bug:
https://bugs.llvm.org/show_bug.cgi?id=36430
https://reviews.llvm.org/D45554
Files:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpoin
eugene updated this revision to Diff 142209.
eugene marked 2 inline comments as done.
eugene added a comment.
add comment to the test
https://reviews.llvm.org/D45554
Files:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
packages/Pytho
eugene added inline comments.
Comment at:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c:13
{
+printf("Observable side effect\n");
return 0; // Set break point at this line.
labath wrote:
> Why did you need to add t
eugene updated this revision to Diff 142319.
eugene added a comment.
Got rid of the printf in the test
https://reviews.llvm.org/D45554
Files:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
packages/Python/lldbsuite/test/functionalitie
eugene marked 2 inline comments as done.
eugene added a comment.
There is an ownership cycle between BreakpointSite::m_owners and
BreakpointLocation::m_bp_site_sp.
We should probably make m_owners a collection of weak references.
But currently most of the code just works it around by calling
Br
eugene added a comment.
Well, I agree that breakpoints, locations and sites could benefit from
ownership refactoring.
shared_ptr cycles are bad.
Let's discuss it at lldb-dev.
Meanwhile I think it's still ok to fix this bug right now, by doing what has
already been done in other places.
http
eugene added a comment.
If nobody minds, I'd appreciate if somebody would accept this patch.
https://reviews.llvm.org/D45554
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL330163: Make sure deleting all breakpoints clears their
sites first (authored by eugene, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D45554
eugene accepted this revision.
eugene added a comment.
This revision is now accepted and ready to land.
I like how much code you deleted.
https://reviews.llvm.org/D31129
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org
eugene updated this revision to Diff 93438.
eugene added reviewers: labath, zturner, jingham.
eugene changed the visibility from "eugene (Eugene Zemtsov)" to "Public (No
Login Required)".
eugene changed the edit policy from "eugene (Eugene Zemtsov)" to "All Users".
eugene added a subscriber: lldb-
eugene updated this revision to Diff 93572.
eugene added a comment.
Addressing code-review comments.
Most notable change: MethodName::Parse() tries simple version of name parser,
before invoking full power of CPlusPlusNameParser. It really helps with the
perf.
https://reviews.llvm.org/D31451
eugene marked 4 inline comments as done.
eugene added a comment.
> I think we should do some performance measurements to see whether this needs
> more optimising or it's fine as is.
>
> I propose the following benchmark:
>
> bin/lldb bin/clang
>
> make sure clang is statically linked
> br
eugene updated this revision to Diff 93693.
eugene marked 3 inline comments as done.
eugene added a comment.
Addressing review commnets
https://reviews.llvm.org/D31451
Files:
source/Plugins/Language/CPlusPlus/CMakeLists.txt
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Pl
eugene updated this revision to Diff 93694.
eugene marked an inline comment as done.
https://reviews.llvm.org/D31451
Files:
source/Plugins/Language/CPlusPlus/CMakeLists.txt
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
source
eugene added a subscriber: labath.
eugene marked 2 inline comments as done.
eugene added a comment.
In https://reviews.llvm.org/D31451#715649, @tberghammer wrote:
> Because of this I think some targeted micro benchmark will be much more
> useful to measure the performance of this code then an en
This revision was automatically updated to reflect the committed changes.
Closed by commit rL299374: New C++ function name parsing logic (authored by
eugene).
Changed prior to commit:
https://reviews.llvm.org/D31451?vs=93694&id=93910#toc
Repository:
rL LLVM
https://reviews.llvm.org/D31451
eugene accepted this revision.
eugene added a comment.
This revision is now accepted and ready to land.
Thanks for doing it. IMO we should always strive to use "absolute" path for all
headers.
https://reviews.llvm.org/D31877
___
lldb-commits mailin
eugene added a comment.
Is it really necessary to check in binary ELF files?
I understand that we don't always have a C compiler capable of producing ELF
files, but maybe it's ok to skip this test on those platforms.
https://reviews.llvm.org/D32434
_
eugene accepted this revision.
eugene added a comment.
This revision is now accepted and ready to land.
Neatly done!
https://reviews.llvm.org/D32434
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list
eugene added inline comments.
Comment at: source/Host/common/MainLoop.cpp:82
+ int queue_id;
+ std::vector events;
+ struct kevent event_list[4];
here and below struct seems to be redundant
https://reviews.llvm.org/D32600
_
eugene accepted this revision.
eugene added inline comments.
This revision is now accepted and ready to land.
Comment at: source/Host/common/MainLoop.cpp:66
class MainLoop::RunImpl {
public:
Could you please add here a comment describing when kqueue/pselect/
eugene added inline comments.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.h:148
- ::pid_t Attach(lldb::pid_t pid, Status &error);
+ static llvm::Expected> Attach(::pid_t pid);
labath wrote:
> zturner wrote:
> > Before it was only returning 1
eugene added inline comments.
Comment at: source/Host/common/Host.cpp:1010
+static constexpr char type[] = "WXS";
+OS << formatv("{0}{1:x-2}", type[WS.type], WS.status);
+return;
type[WS.type] seems to be a somewhat unnecessary hack. I would use a si
eugene accepted this revision.
eugene added a comment.
This revision is now accepted and ready to land.
Great change. The only comment I have is about location of TestUtilities.cpp.
Utility\Helpers kinda implies that these are helpers for Utility tests which is
not true.
I would move it up one l
eugene accepted this revision.
eugene added a comment.
This revision is now accepted and ready to land.
Great change! Thanks making it.
https://reviews.llvm.org/D34911
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/c
eugene added a comment.
In https://reviews.llvm.org/D34911#808059, @wengxt wrote:
> I don't have commit access. May any one help me to commit?
I'll do it for you.
https://reviews.llvm.org/D34911
___
lldb-commits mailing list
lldb-commits@lists.l
eugene added a comment.
Jim already did it. Thanks Jim.
https://reviews.llvm.org/D34911
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
eugene created this revision.
eugene added a project: LLDB.
Currently on Linux there is now way to force LLDB to use symbols from a certain
file unless there is a UUID match established by build-id or debuglink.
This change somewhat extends ''target symbols add', making it possible to
specify s
eugene added a comment.
In https://reviews.llvm.org/D35607#814560, @labath wrote:
> Btw, will this work correctly if we are cross-debugging from a mac? (because
> then we will use the fancy DownloadObjectAndSymbolFile implementation)
Yeah, you're right. I'll move this logic to the CommandObjec
eugene added a comment.
In https://reviews.llvm.org/D35607#814982, @clayborg wrote:
> So we should just make sure this works:
>
> (lldb) target symbols add --shlib a.out debug_binaries/a.out
>
I like this, and I'm going to implement it. Pavel is right and the way UUID is
set for modules wit
eugene updated this revision to Diff 107628.
eugene retitled this revision from "Extend 'target symbols add' to load symbols
from a given file by UUID." to "Extend 'target symbols add' to set symbol file
for a given module".
eugene edited the summary of this revision.
eugene added a comment.
Fou
eugene updated this revision to Diff 107725.
eugene added a comment.
Added error message when more than one symbol file is provided.
https://reviews.llvm.org/D35607
Files:
packages/Python/lldbsuite/test/linux/add-symbols/Makefile
packages/Python/lldbsuite/test/linux/add-symbols/TestTargetS
eugene added a comment.
Greg, are you with me checking this in?
https://reviews.llvm.org/D35607
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL308933: Extend 'target symbols add' to load symbols from a
given module (authored by eugene).
Changed prior to commit:
https://reviews.llvm.org/D35607?vs=107725&id=107983#toc
Repository:
rL LLVM
htt
eugene created this revision.
eugene added a project: LLDB.
https://reviews.llvm.org/D36126
Files:
source/Symbol/Symtab.cpp
Index: source/Symbol/Symtab.cpp
===
--- source/Symbol/Symtab.cpp
+++ source/Symbol/Symtab.cpp
@@ -616,8 +
eugene added a comment.
clang drew my attention to it:
Symtab.cpp:620:5: warning: ignoring return value of function declared with
'warn_unused_result' attribute [-Wunused-result]
std::unique(indexes.begin(), indexes.end());
^~~ ~~
https://reviews.llvm.o
eugene closed this revision.
eugene added a comment.
Checked in as r309648
https://reviews.llvm.org/D36126
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
eugene accepted this revision.
eugene added a comment.
LGTM. Test run on Linux is clear. I also see a bit of perf bump.
Before checking in please remove outdated comment from
ProcessGDBRemote::LaunchAndConnectToDebugserver.
// Use a socketpair on Apple for now until other platforms can verify
This revision was automatically updated to reflect the committed changes.
Closed by commit rL312151: Now a ppc64le binary is correctly detected:
(authored by eugene).
Repository:
rL LLVM
https://reviews.llvm.org/D36804
Files:
lldb/trunk/include/lldb/Core/ArchSpec.h
lldb/trunk/source/Core/
eugene accepted this revision.
eugene added a comment.
I did mark it Accepted.
https://reviews.llvm.org/D33213
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL314127: Use socketpair on all Unix platforms (authored by
eugene).
Repository:
rL LLVM
https://reviews.llvm.org/D33213
Files:
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb
This revision was automatically updated to reflect the committed changes.
Closed by commit rL315008: Enable breakpoints and read/write GPRs for ppc64le
(authored by eugene).
Repository:
rL LLVM
https://reviews.llvm.org/D38323
Files:
lldb/trunk/source/Plugins/Process/Linux/CMakeLists.txt
l
eugene added a comment.
DisableUnlocked methods makes me uneasy.
Maybe we can take a log lock before forking and then properly release it in
both parent and child processes?
https://reviews.llvm.org/D38938
___
lldb-commits mailing list
lldb-commits
eugene accepted this revision.
eugene added inline comments.
This revision is now accepted and ready to land.
Comment at: source/Utility/Log.cpp:333
+
+void Log::DisableLoggingChild() {
+ for (auto &c: *g_channel_map)
A little comment here describing nature of a
eugene created this revision.
eugene added a project: LLDB.
main.cpp is complete mess of tabs and spaces. This change brings it to
compliance with LLVM coding style.
https://reviews.llvm.org/D30234
Files:
packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
Index: packages/Python/lldbs
eugene added a comment.
In https://reviews.llvm.org/D30234#682990, @jmajors wrote:
> Do we have any plans to put something like cpplint into effect?
First step would be to have warning free build.
Then we can turn "warnings as errors" on.
https://reviews.llvm.org/D30234
___
eugene updated this revision to Diff 89311.
https://reviews.llvm.org/D30234
Files:
packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
Index: packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
===
--- packages/Python/
eugene updated this revision to Diff 89312.
https://reviews.llvm.org/D30234
Files:
packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
Index: packages/Python/lldbsuite/test/tools/lldb-server/main.cpp
===
--- packages/Python/
eugene added a comment.
In https://reviews.llvm.org/D30234#683018, @jingham wrote:
> Anyway, it might be better just to do that to this file using the top level
> .clang-format. Note that you are making several choices which were not the
> choices made by clang-format using the .clang-format f
eugene marked an inline comment as done.
eugene added inline comments.
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/main.cpp:32-42
+static const char *const RETVAL_PREFIX = "retval:";
+static const char *const SLEEP_PREFIX = "sleep:";
+static const char *const STD
eugene updated this revision to Diff 89404.
eugene added a comment.
I have added local .clang-format file. But it didn't change the way main.cpp
was formated.
https://reviews.llvm.org/D30234
Files:
packages/Python/lldbsuite/test/tools/lldb-server/.clang-format
packages/Python/lldbsuite/tes
eugene created this revision.
eugene added a project: LLDB.
Herald added a subscriber: emaste.
https://reviews.llvm.org/D30286
Files:
include/lldb/Host/common/NativeProcessProtocol.h
packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
packages/Python/lldbsuite/test/tools/
eugene updated this revision to Diff 89575.
eugene edited the summary of this revision.
eugene added a comment.
Herald added subscribers: srhines, danalbert.
Addressing code review comments.
https://reviews.llvm.org/D30286
Files:
include/lldb/Host/common/NativeProcessProtocol.h
packages/Pyt
eugene added inline comments.
Comment at: source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp:31
+// Number of register sets provided by this context.
+enum { k_num_register_sets = 1 };
+
Here and below why not use
constexpr size_t k_num_register_s
eugene created this revision.
If QPassSignals packaet is supported by lldb-server, lldb-client will utilize
it and ask the server to ignore signals that don't require stops or
notifications.
Such signals will be immediately re-injected into inferior to continue normal
execution.
https://revie
eugene updated this revision to Diff 90407.
eugene added a comment.
Addressing code review commends, and moving a signal filtering method to the
base Process class.
https://reviews.llvm.org/D30520
Files:
include/lldb/Target/Process.h
include/lldb/Target/UnixSignals.h
source/Plugins/Proce
eugene updated this revision to Diff 90424.
eugene marked an inline comment as done.
eugene added a comment.
Added comment to UnixSignals::m_version.
Still thinking about the best way to test it all.
https://reviews.llvm.org/D30520
Files:
include/lldb/Target/Process.h
include/lldb/Target/U
eugene added inline comments.
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3766-3776
+ llvm::SmallVector signals_to_ignore;
+ int32_t signo = m_unix_signals_sp->GetFirstSignalNumber();
+ while (signo != LLDB_INVALID_SIGNAL_NUMBER) {
+bool should_ignor
eugene updated this revision to Diff 90563.
eugene added a comment.
Herald added a subscriber: mgorny.
Moved signal filtering to UnixSignals and added unit tests for UnixSignals
class.
https://reviews.llvm.org/D30520
Files:
include/lldb/Target/Process.h
include/lldb/Target/UnixSignals.h
eugene updated this revision to Diff 90755.
eugene added a comment.
Addressing review comments on SignalTests and getting rid of dependency on
DidLaunch and WillResume
https://reviews.llvm.org/D30520
Files:
include/lldb/Target/Process.h
include/lldb/Target/UnixSignals.h
source/Plugins/Pr
eugene marked 3 inline comments as done.
eugene added a comment.
Addressing review comments.
Comment at: unittests/Signals/UnixSignalsTest.cpp:43
+
+#define ASSERT_EQ_ARRAYS(expected, observed)
\
+ AssertEqArrays((expected), (observed), __FIL
eugene added inline comments.
Comment at: include/lldb/Target/Process.h:3148
+ virtual Error UpdateAutomaticSignalFiltering();
+
clayborg wrote:
> Can we remove this and only have it in ProcessGDBRemote? Then we just call it
> when we need to in ProcessGDBRem
eugene added a comment.
UnixSignals is a nice self contained class that already does 99% of the work
(see UnixSignals::GetFilteredSignals). I don't think we should have it call
anybody.
Only process knows when it is the right time to send actual QPassSignals
packet, there is not need to someho
eugene updated this revision to Diff 90921.
eugene added a comment.
Rename ASSERT_EQ_ARRAYS to EXPECT_EQ_ARRAYS
https://reviews.llvm.org/D30520
Files:
include/lldb/Target/Process.h
include/lldb/Target/UnixSignals.h
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
sourc
eugene added inline comments.
Comment at: include/lldb/Utility/Log.h:152
- Flags &GetOptions();
+ const Flags GetOptions() const;
Seems like const on return value is not really needed.
Comment at: include/lldb/Utility/Log.h:163
+ std::at
This revision was automatically updated to reflect the committed changes.
Closed by commit rL297231: Make LLDB skip server-client roundtrip for signals
that don't require any… (authored by eugene).
Changed prior to commit:
https://reviews.llvm.org/D30520?vs=90921&id=90930#toc
Repository:
rL
eugene accepted this revision.
eugene added inline comments.
Comment at: source/Host/linux/Host.cpp:167
+ (llvm::Twine("/proc/") + llvm::Twine(pid) + "/exe").toVector(ProcExe);
+ llvm::SmallString<0> ExePath;
+ ExePath.assign(PATH_MAX, '\0');
std::string? Do w
eugene accepted this revision.
eugene added a comment.
This revision is now accepted and ready to land.
Makes sense to me.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62168/new/
https://reviews.llvm.org/D62168
_
80 matches
Mail list logo
