labath reopened this revision.
labath added a comment.
This revision is now accepted and ready to land.
Reopening for a re-review of a fix.
Repository:
rL LLVM
https://reviews.llvm.org/D33283
___
lldb-commits mailing list
lldb-commits@lists.llvm.
labath updated this revision to Diff 100099.
labath added a comment.
Fixed version.
The original patch caused a regression in TestLoadUnload, which has only showed
up when running the remote test suite. The problem there was that we interrupted
the target just as it has hit the rendezvous breakpo
labath requested review of this revision.
labath added a comment.
Let me know what you think of the fix, and please confirm whether the ignoring
of the breakpoint condition is a bug.
thanks.
https://reviews.llvm.org/D33283
___
lldb-commits mailing
labath added inline comments.
Comment at:
packages/Python/lldbsuite/test/python_api/sbstructureddata/TestStructuredDataAPI.py:43
+# Now launch the process, and do not stop at entry point.
+process = target.LaunchSimple(
+None, None, self.get_process_w
labath added a comment.
Thanks for the explanation. I'll continue the discussion about the conditional
breakpoint thingy on bug #33164, which I've just filed.
https://reviews.llvm.org/D33283
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
labath added a comment.
Getting really close now. However, the debug trap issue is still not resolved.
And we still have to figure out how to make sure the tests don't blow up on
platforms that don't support debugging via lldb-server (i.e., anything except
linux, android, netbsd). One option wo
labath added a comment.
Affirmative. Tests should go in together with the feature they are testing.
Repository:
rL LLVM
https://reviews.llvm.org/D32149
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailma
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303848: Recommit "RunThreadPlan: Fix halting logic in
IgnoreBreakpoints = false" (authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D33283?vs=100099&id=100221#toc
Repository:
rL
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
Ok, I think this makes a reasonable starting point for further work. We just
need to tighten the condition on when to run these tests.
Comment at: unittests/tools/CMakeLists
labath added a comment.
thanks for the effort. I found the logic of the test quite difficult to follow,
with multiple breakpoints and notify_calls(). Instead of trying to point out
each problem, I figured it will be easier to write my take on how the test
could look like: https://paste.ubuntu.c
labath added inline comments.
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1304
+ // Allocate the response buffer.
+ uint8_t *buffer = new (std::nothrow) uint8_t[byte_count];
+ if (buffer == nullptr)
Hey, ravi. You're lea
labath added a comment.
First batch of comments from me, I think I will have some more once I gain more
insight into this. The main one is about the constructor/initialize,
destructor/destroy duality, which we should abolish. The rest is mostly
stylistic.
Comment at: source/
labath added a comment.
In https://reviews.llvm.org/D33035#767029, @abhishek.aggarwal wrote:
> In https://reviews.llvm.org/D33035#754640, @labath wrote:
>
> > I don't really like that we are adding a public shared library for every
> > tiny intel feature. Could we at least merge this "plugin" wi
labath added a comment.
In https://reviews.llvm.org/D33426#766525, @bgianfo wrote:
> Address Pavel and Greg's feedback on Diff 100365.
>
> Pavel: I took your suggestions to make the test case more readable,
> I really appreciate the guidance. I did have to tweak some of the
> functionality to m
labath added a comment.
Thanks for the support, @beanz.
Comment at: unittests/tools/CMakeLists.txt:1
+if(UNIX AND NOT APPLE)
+ add_subdirectory(lldb-server)
beanz wrote:
> labath wrote:
> > This is not what I meant. The only targets (at least until we have
>
labath created this revision.
Herald added a subscriber: mgorny.
https://reviews.llvm.org/D33771
Files:
cmake/modules/LLDBConfig.cmake
cmake/modules/LLDBGenerateConfig.cmake
include/lldb/Host/Config.h.cmake
include/lldb/Host/linux/Uio.h
source/Host/linux/LibcGlue.cpp
Index: source/Host
labath created this revision.
Herald added subscribers: mgorny, srhines.
This replaces the static functions used for creating
NativeProcessProtocol instances with a factory pattern, and modernizes
the interface of the new class in the process -- I use llvm::Expected
instead of the Status+value com
labath updated this revision to Diff 101023.
labath added a comment.
forgot to reformat the code
https://reviews.llvm.org/D33778
Files:
include/lldb/Host/common/NativeProcessProtocol.h
source/Host/common/NativeProcessProtocol.cpp
source/Plugins/Process/Linux/NativeProcessLinux.cpp
sourc
labath 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);
zturner wrote:
> Before it was only returning 1, now it's returni
This revision was automatically updated to reflect the committed changes.
Closed by commit rL304544: cmake: Put PROCESS_VM_READV detection results into
Config.h (authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D33771?vs=101012&id=101192#toc
Repository:
rL LLVM
https:/
labath planned changes to this revision.
labath added a comment.
I am going to come back to this later, I'm going to create one or two more
cleanup diffs which this will depend on.
https://reviews.llvm.org/D33778
___
lldb-commits mailing list
lldb-
labath created this revision.
Herald added subscribers: mgorny, emaste.
It is intended to wrap functions which can fail with EINTR (which we
have a surprising number of). It is inspired by the TEMP_FAILURE_RETRY
macro in glibc, but I've c++-ified it and made it more generic (by
specifying an expli
labath updated this revision to Diff 101212.
labath added a comment.
- fix freebsd typo
- use ::waitpid consistently
https://reviews.llvm.org/D33831
Files:
include/lldb/Host/Host.h
source/Host/common/File.cpp
source/Host/macosx/Host.mm
source/Host/posix/ConnectionFileDescriptorPosix.cpp
labath added inline comments.
Comment at: include/lldb/Host/Host.h:252
+result = f(args...);
+ while (result == fail_value && errno == EINTR);
+ return result;
krytarowski wrote:
> Will this build on Windows for their native API? If so, will it be useful
>
labath added a comment.
In https://reviews.llvm.org/D33426#772174, @bgianfo wrote:
> @jingham @labath do you have any more feedback?
As Jim pointed out, the Resume command does not do what I thought it does, so
having it in the test makes no sense. One option would be to just leave out the
ca
labath added a comment.
I am going to commit this, so we can move forward with the remote testing
aspect.
I'd like to reiterate that we are not planning to remove any existing tests
until both remote and debugserver testing work. At that point, we'd like to
start removing the existing tests, b
This revision was automatically updated to reflect the committed changes.
Closed by commit rL304793: New framework for lldb client-server communication
tests. (authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D32930?vs=100288&id=101553#toc
Repository:
rL LLVM
https://r
labath updated this revision to Diff 101555.
labath added a comment.
Herald added a subscriber: srhines.
Add documentation.
https://reviews.llvm.org/D32022
Files:
include/lldb/Core/Address.h
include/lldb/Core/Section.h
include/lldb/Target/SectionLoadList.h
packages/Python/lldbsuite/tes
labath added a comment.
I'd like to commit this this week. If you have any objections to how I
implemented this, let me know, so I can adjust the approach.
https://reviews.llvm.org/D32022
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
ht
labath added a comment.
Cool. Sorry about all the back-and-forth and thanks for the patience.
https://reviews.llvm.org/D33426
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
labath added inline comments.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.h:156
+ // -
+ static size_t ReadCyclicBuffer(void *buf, size_t buf_size, void *cyc_buf,
+
labath created this revision.
Herald added a subscriber: mgorny.
A number of places were trying to decode the result of wait(). Add a simple
utility function that does that and a struct that encapsulates the
decoded result. Then also provide a pretty-printer for that class.
https://reviews.llvm.
labath updated this revision to Diff 101759.
labath added a comment.
Fix typo
https://reviews.llvm.org/D33998
Files:
include/lldb/Host/Host.h
include/lldb/Host/common/NativeProcessProtocol.h
include/lldb/lldb-private-enumerations.h
source/Host/common/Host.cpp
source/Host/common/Native
labath added inline comments.
Comment at: source/Host/common/Host.cpp:1001
+return {Stop, uint8_t(WSTOPSIG(wstatus))};
+ llvm_unreachable("Unknown wait status");
+}
krytarowski wrote:
> `WIFCONTINUED()`?
I'm deliberately ignoring that, as we don't have a use
labath added a comment.
Thanks for the review. I'm not entirely proud of how I implemented this, but I
hope it's not too ugly either. The reason I opted for this approach is that I
needed to implement the "if the module is null then decrement the address and
recompute" logic in two places. Regi
This revision was automatically updated to reflect the committed changes.
Closed by commit rL304976: Fix backtrace of noreturn functions situated at the
end of a module (authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D32022?vs=101555&id=101900#toc
Repository:
rL LLVM
This revision was automatically updated to reflect the committed changes.
Closed by commit rL305197: Introduce new command: thread backtrace unique
(authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D33426?vs=101664&id=102192#toc
Repository:
rL LLVM
https://reviews.llvm
labath added a comment.
Committed as r305197. I needed to tweak the test logic a bit, as not even
thread.StepOut() did exactly what we needed there, because it was resuming also
all other threads even though it was not necessary. Take a look at the
resulting commit if you want to see the differ
labath marked 2 inline comments as done.
labath 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;
eugene wrote:
> type[WS.type]
labath updated this revision to Diff 102309.
labath marked an inline comment as done.
labath added a comment.
Use a switch instead of indexing the array with an enum value
https://reviews.llvm.org/D33998
Files:
include/lldb/Host/Host.h
include/lldb/Host/common/NativeProcessProtocol.h
incl
labath created this revision.
Herald added subscribers: kristof.beyls, javed.absar, aemerson.
The motivation for this is to make sure the first row of the plan
compares equal to the first row of a generic debug_frame unwind plan.
Right now, the code in FuncUnwinders::GetUnwindPlanAtNonCallSite
con
labath added a comment.
Second round of comments. I still see a lot of places with redundant checks for
what appear to be operational invariants. I'd like to get those cleared up to
make the code flow better.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:611
labath created this revision.
Herald added subscribers: mgorny, srhines.
ProcessLauncherPosix was using posix_spawn for launching the process,
but this function is not available on all platforms we support, and even
where it was avaialable, it did not support the full range of options we
require f
labath added a comment.
Unfortunately, posix_spawn does not satisfy all our needs for launching
processes on non-darwin platforms (according to its design rationale, that was
never the intention). The most notable one is the "launch a process for
debugging". Darwin seems to have added extension
labath added inline comments.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.h:282
+ // same process user id.
+ llvm::DenseSet m_pt_traced_thread_group;
};
ravitheja wrote:
> labath wrote:
> > I am confused about the purpose of this member variabl
labath created this revision.
Use c++11 thread_local variables instead. As far as I am aware, they are
supported by all compilers/targets we care about.
https://reviews.llvm.org/D34274
Files:
include/lldb/Host/Host.h
source/Core/Timer.cpp
source/Host/common/Host.cpp
source/Host/windows/
labath added a comment.
Thanks for spelling that out. However, it still does not sound like a
convincing use case to me. Why would the user start to trace just one thread,
only to later change his mind and trace the whole process instead. I'm not
saying that can't happen, but it seems like some
labath added a comment.
In https://reviews.llvm.org/D34236#783186, @emaste wrote:
> LGTM from the FreeBSD side. The launch code for FreeBSD came from the
> original (in-process) implementation that Linux and FreeBSD shared.
Thanks Ed.
If anyone's interested in archaeology, that launch code wa
labath added a comment.
In https://reviews.llvm.org/D33674#783861, @ravitheja wrote:
> Although a bit confusing, there is more flexibility for the user.I must also
> point out that the trace buffer available is not unlimited and there can be
> situations where a user might simultaneously want t
labath added inline comments.
Comment at: tools/intel-features/CMakeLists.txt:50
+install(TARGETS lldbIntelFeatures
+ LIBRARY DESTINATION bin)
"bin" sounds wrong here. Shouldn't this go ti lib${LLVM_LIBDIR_SUFFIX}?
To properly handle DLL targets (i don't know wh
labath added a comment.
In https://reviews.llvm.org/D34274#782529, @zturner wrote:
> The last time I tried to do this we couldn't because it didn't yet work on
> iOS. I checked with some Apple people though and they said `thread_local`
> support was released last year on all Apple platforms.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL305686: Delete ProcessLauncherPosix (authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D34236?vs=102667&id=103022#toc
Repository:
rL LLVM
https://reviews.llvm.org/D34236
Files:
This revision was automatically updated to reflect the committed changes.
Closed by commit rL305687: Tweak SysV_arm64 function entry unwind plan
(authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D34199?vs=102514&id=103025#toc
Repository:
rL LLVM
https://reviews.llvm.or
This revision was automatically updated to reflect the committed changes.
Closed by commit rL305689: Add pretty-printer for wait(2) statuses and
modernize the code handling them (authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D33998?vs=102309&id=103027#toc
Repository:
labath created this revision.
Herald added a subscriber: emaste.
This is basically a revert of https://reviews.llvm.org/D16107 and parts of
https://reviews.llvm.org/D10800, which were
trying to get vdso loading working. They did this by implementing a
generic load-an-elf-file from memory approach
labath added a comment.
> Well that's the whole thread group idea. Currently we have only one thread
> group i.e the process group (all existing un traced threads + newly spawned
> ones).
> With separate "trace all threads" and "trace new threads", it will be
> multiple thread groups. For e.g
This revision was automatically updated to reflect the committed changes.
Closed by commit rL305779: Remove home-grown thread-local storage wrappers
(authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D34274?vs=102810&id=103169#toc
Repository:
rL LLVM
https://reviews.llv
This revision was automatically updated to reflect the committed changes.
Closed by commit rL305780: [linux] Change the way we load vdso pseudo-module
(authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D34352?vs=103045&id=103170#toc
Repository:
rL LLVM
https://reviews.l
labath created this revision.
Herald added a subscriber: mgorny.
All implementations of the connection interface live in the Host module
already, so it makes sense for the interface itself to be defined there.
https://reviews.llvm.org/D34400
Files:
include/lldb/Core/Connection.h
include/lld
labath added a comment.
I like the direction this is going in, but I see places for more cleanup. The
main three items are:
- making destruction cleaner
- avoiding global variables
- making ReadCyclicBuffer readable
the rest are just general nits.
Comment at: source/Plugins/
labath added inline comments.
Comment at: source/Plugins/Process/Linux/ProcessorTrace.cpp:307
+
+Status ProcessorTraceMonitor::Destroy() {
+ Status error;
ravitheja wrote:
> labath wrote:
> > I'd like this work to be done in the destructor. Just like nobody shou
labath created this revision.
Herald added a subscriber: mgorny.
The new UndefinedBehaviorSanitizer plugin was breaking file path length
limits, because it's (fairly long name) appears multiple times in the
path. Cmake ends up putting the object file at path
tools/lldb/source/Plugins/Instrumentati
labath added a comment.
In https://reviews.llvm.org/D34553#789023, @kubamracek wrote:
> Oh wow, we really need to limit path lengths?
It's a bit annoying, but windows has issues with paths like that. It's actually
possible to avoid it nowadays, if you use the right APIs, but not all programs
labath created this revision.
Herald added subscribers: aprantl, mgorny.
This is a beefed-up version of https://reviews.llvm.org/D33504, which adds
support for dwarf 4
debug_frame section format.
The main difference here is that the decision whether to use eh_frame or
debug_frame is done on a pe
labath added a comment.
Hi Abhishek,
Thank you for incorporating the changes. I still see some room for improvement
in the cmake files. I realize that this is something most people are not
familiar with, and is not exactly glamorous work, but it's still part of our
codebase and we should make
labath added a comment.
Should I start looking at this or you have more changes planned? I still see
manual memory management in the Destroy function...
BTW, what's your testing strategy for this?
Comment at: source/Plugins/Process/Linux/ProcessorTrace.cpp:393
+ llvm::SmallV
labath added a comment.
In https://reviews.llvm.org/D33674#790595, @ravitheja wrote:
> Yes you can start looking at it. The thing with munmap stuff is the
> following, you basically don't want to give the user an opportunity to have
> an uninitialized or instances which have been destroyed but
labath created this revision.
Herald added subscribers: mgorny, kubamracek.
It had a dependency on StringConvert and file reading code, which is not
in Utility. I've replaced that code by equivalent llvm operations.
I've added a unit test to demonstrate that parsing a file still works.
https://
labath updated this revision to Diff 103971.
labath added a comment.
This is how things would look like if we move Connection to Utility instead. I
needed to do two things to make this happen:
- Keep Connection::CreateDefaultConnection in Host (now known as
Host::CreateDefaultConnection), becaus
labath added a comment.
In https://reviews.llvm.org/D34613#790745, @clayborg wrote:
> Glad this is happening. Does this mean we won't see the "bad eh frame"
> warnings anymore on linux? See inlined comments.
Unfortunately, I doubt it. This does nothing about eh_frame parsing, it just
adds deb
labath updated this revision to Diff 103976.
labath added a comment.
Herald added a subscriber: emaste.
Well, PluginProcessPOSIX does not seem to be using anything from Core directly,
although I doubt that is thanks to this patch. It will be a long time before
anything stops depending on Core (my
This revision was automatically updated to reflect the committed changes.
Closed by commit rL306391: Move Connection and IOObject interfaces to Utility
module (authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D34400?vs=103971&id=104125#toc
Repository:
rL LLVM
https://r
This revision was automatically updated to reflect the committed changes.
Closed by commit rL306394: Move StructuredData from Core to Utility (authored
by labath).
Changed prior to commit:
https://reviews.llvm.org/D34625?vs=103976&id=104127#toc
Repository:
rL LLVM
https://reviews.llvm.org/D
labath added inline comments.
Comment at: include/lldb/Symbol/DWARFCallFrameInfo.h:38
DWARFCallFrameInfo(ObjectFile &objfile, lldb::SectionSP §ion,
lldb::RegisterKind reg_kind, bool is_eh_frame);
clayborg wrote:
> labath wrote:
> > clay
This revision was automatically updated to reflect the committed changes.
Closed by commit rL306397: Add debug_frame section support (authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D34613?vs=103922&id=104134#toc
Repository:
rL LLVM
https://reviews.llvm.org/D34613
Fil
labath created this revision.
instead of using a boolean to differentiate between the two section
types, use an enum to make the intent clearer.
I also remove the RegisterKind argument from the constructor, as this
can be deduced from the Type argument.
https://reviews.llvm.org/D34681
Files:
labath created this revision.
Herald added subscribers: mgorny, emaste.
Fetching an input file required about five lines of code, and this was
repeated in multiple unit tests, with slight variations. Add a helper
function for doing that into the lldbUtilityMocks module (which I rename
to lldbUtili
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
I still see some room for improvement, but some of those require some
infrastructure improvements, like being able to construct llvm::StringError (or
equivalent easily), which I guess will hav
labath added a comment.
In https://reviews.llvm.org/D34681#792602, @tatyana-krasnukha wrote:
> Saying about clear intent, it would be even much more better if class name
> doesn't start with DWARF ;)
That's probably true. :) However, I think I've done enough of renaming of files
lately that I
This revision was automatically updated to reflect the committed changes.
Closed by commit rL306521: [DWARFCallFrameInfo] Add Type enum to differentiate
eh/debug_frame sections (authored by labath).
Repository:
rL LLVM
https://reviews.llvm.org/D34681
Files:
lldb/trunk/include/lldb/Symbol/DW
labath marked an inline comment as done.
labath added a comment.
In https://reviews.llvm.org/D34683#792736, @eugene wrote:
> 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
labath created this revision.
Herald added subscribers: mgorny, emaste.
The classes have no dependencies, and they are used both by lldb and
lldb-server, so it makes sense for them to live in the lowest layers.
https://reviews.llvm.org/D34746
Files:
include/lldb/Core/Timer.h
include/lldb/Co
labath created this revision.
The instruction pattern:
and $-16, %esp
sub $imm, %esp
...
lea imm(%ebp), %esp
appears when the compiler is realigning the stack (for example in
main(), or almost everywhere with -mstackrealign switch). The "and"
instruction is very difficult to model, but that's not
labath added inline comments.
Comment at:
source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp:875-876
+ row->GetCFAValue().GetRegisterNumber() == m_lldb_fp_regnum) {
+ current_sp_bytes_offset_from_cfa =
+ row->GetCFAValue().GetOffset() - s
labath added inline comments.
Comment at: source/Plugins/Process/Utility/InferiorCallPOSIX.cpp:92
clang_ast_context->GetBasicType(eBasicTypeVoid).GetPointerType();
-lldb::addr_t args[] = {addr, length, prot_arg, flags_arg, fd, offset};
+llvm::SmallVec
labath added a comment.
Thank you for making the changes. I concede that I am not always clear in my
communications, and I apologize if I was being rude.
I am happy with the cmake stuff, but I noticed a new issue now. Exceptions are
banned in llvm, so we need to figure out how to remove them fr
This revision was automatically updated to reflect the committed changes.
Closed by commit rL30: [UnwindAssembly/x86] Add support for "lea imm(%ebp),
%esp" pattern (authored by labath).
Repository:
rL LLVM
https://reviews.llvm.org/D34750
Files:
lldb/trunk/source/Plugins/UnwindAssembly/x
labath updated this revision to Diff 104624.
labath added a comment.
That's a good idea. This time I almost smuggled in a (unused) Host include into
Utility -- it was a leftover from the Host::ThreadLocalStorage times. I'll make
sure to run it in the future.
After fixing that issue, I see no diff
This revision was automatically updated to reflect the committed changes.
Closed by commit rL306668: [unittests] Add a helper function for getting an
input file (authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D34683?vs=104145&id=104627#toc
Repository:
rL LLVM
https:/
labath added inline comments.
Comment at: tools/intel-features/intel-pt/Decoder.cpp:411
+std::string image_path(image_complete_path, path_length);
+try {
+ readExecuteSectionInfos.emplace_back(
abhishek.aggarwal wrote:
> labath wrote:
> >
This revision was automatically updated to reflect the committed changes.
Closed by commit rL306682: Move Timer and TraceOptions from Core to Utility
(authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D34746?vs=104624&id=104647#toc
Repository:
rL LLVM
https://reviews.ll
labath created this revision.
Due to recent refactors, the descriptions of various modules were wildly
out of date. With this patch, I am not trying to legislate anything,
I am merely documenting the current state of affairs.
I am also deleting one copy of the architecture docs. AFAIK, this one i
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
looks good
Repository:
rL LLVM
https://reviews.llvm.org/D34929
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org
labath added a reviewer: emaste.
labath added a comment.
+ed as freebsd owner
ArrayRef cannot be returned from a function as they don't own the underlying
data. The more traditional way of using SmallVector is to "return" it as a
by-ref `SmallVectorImpl` argument, as that avoids the need to har
labath added inline comments.
Comment at: source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:112
+ if (wpid != pid || !WIFSTOPPED(wstatus)) {
+std::error_code EC(errno, std::generic_category());
+LLDB_LOG(
krytarowski wrote:
> I can imagine that in so
labath updated this revision to Diff 105065.
labath marked 9 inline comments as done.
labath added a comment.
Address review comments
https://reviews.llvm.org/D33778
Files:
include/lldb/Host/common/NativeProcessProtocol.h
source/Host/common/NativeProcessProtocol.cpp
source/Plugins/Process
labath added a reviewer: eugene.
labath added a comment.
Adding eugene, as he wrote that piece of code. Seems reasonable at a first
glance though.
https://reviews.llvm.org/D34911
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lis
This revision was automatically updated to reflect the committed changes.
Closed by commit rL307009: Use llvm::sys::RetryAfterSignal instead of a manual
while errno!=EINTR loop (authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D33831?vs=101212&id=105067#toc
Repository:
labath marked 6 inline comments as done.
labath added inline comments.
Comment at: source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:85
+ int wstatus;
+ ::pid_t wpid = llvm::sys::RetryAfterSignal(-1, ::waitpid, pid, &wstatus, 0);
+ assert(wpid == pid);
kry
labath added inline comments.
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:3203
+error.SetError(response.GetError(), eErrorTypeGeneric);
+ LLDB_LOG(log, "Target does not support Tracing , error {0}",
error.AsCString());
} else
1201 - 1300 of 6408 matches
Mail list logo
