labath (Pavel Labath)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 4 2013, 6:02 AM (228 w, 1 d)

Recent Activity

Today

labath committed rL316106: lldb-server tests: Fix undefined behavior.
lldb-server tests: Fix undefined behavior
Wed, Oct 18, 11:52 AM

Yesterday

labath committed rL316038: Silence some "implicit conversion of string literal" warnings.
Silence some "implicit conversion of string literal" warnings
Tue, Oct 17, 2:52 PM
labath created D39010: lldb-server tests: Propagate environment variables (pr34192).
Tue, Oct 17, 10:02 AM
labath committed rL316010: lldb-server tests: Add support for testing debugserver.
lldb-server tests: Add support for testing debugserver
Tue, Oct 17, 9:28 AM
labath closed D35311: lldb-server tests: Add support for testing debugserver by committing rL316010: lldb-server tests: Add support for testing debugserver.
Tue, Oct 17, 9:28 AM
labath added a comment to D35311: lldb-server tests: Add support for testing debugserver.

This all looks fine to me. The one thing I'd like you to consider is that someday (when lldb-server is supported on Darwin) we will want to run these tests against both debugserver and lldb-server. I'm not asking you to make those changes to this patch, but if there are things you would change about how you're doing things to make that easier to support I'd ask that you consider that.

I have one specific comment below that is along those lines. Otherwise, this LGTM.

Tue, Oct 17, 9:17 AM
labath committed rL316007: Remove shared_pointer from NativeThreadProtocol.
Remove shared_pointer from NativeThreadProtocol
Tue, Oct 17, 8:52 AM
labath closed D35618: Remove shared_pointer from NativeThreadProtocol by committing rL316007: Remove shared_pointer from NativeThreadProtocol.
Tue, Oct 17, 8:52 AM
labath updated the diff for D38938: Logging: provide a way to safely disable logging in a forked process.

Solve the problem using pthread_atfork().

Tue, Oct 17, 6:57 AM
labath added a comment to D38992: Support formatv of TimePoint with strftime-style formats..

I wanted to add something like this myself, but never got around to doing that. Thanks.

Tue, Oct 17, 6:07 AM

Sun, Oct 15

labath created D38938: Logging: provide a way to safely disable logging in a forked process.
Sun, Oct 15, 2:42 PM

Sun, Oct 8

labath added a comment to D38323: Enable breakpoints and read/write GPRs for ppc64le.

Looks fine to me. Sorry about the delay.

@eugene should be able to help you commit this.

Wrt. the extra register context discussion, I believe you will need to implement an extra class or two when you get around to debugging core files, but that should not be necessary right now. (And yes, linux now uses lldb-server for local debugging as well).

Thanks for the review.
What are the classes/structures that I should implement?

Sun, Oct 8, 11:00 AM

Thu, Oct 5

labath accepted D38323: Enable breakpoints and read/write GPRs for ppc64le.

Looks fine to me. Sorry about the delay.

Thu, Oct 5, 10:29 AM

Aug 30 2017

labath accepted D36977: Add 'break' into GDBRemoteClientBase::SendContinuePacketAndWaitForResponse to fix a warning.

looks good. plus, it seems to be a no-op, as the following case block immediately breaks.

Aug 30 2017, 2:49 AM
labath updated subscribers of D36804: Add initial support to PowerPC64 little endian (POWER8).

IIUC, the conclusion was that we don't need the mach-o entry. Could you submit a version of the patch without it. I can't commit this right now, but @eugene should be able to do that for you.

Aug 30 2017, 2:24 AM

Aug 16 2017

labath accepted D34776: Make i386-*-freebsd expression work on JIT path.

lgtm

Aug 16 2017, 2:22 AM

Aug 8 2017

labath added a comment to D35223: Report inferior SIGSEGV/SIGILL/SIGBUS/SIGFPE as a signal instead of an exception on freebsd.

With this patch I observed three new failures on FreeBSD and three new unexpected passes on FreeBSD. An example of a new failure:

======================================================================
FAIL: test_inferior_crashing_expr_step_and_expr_dwarf (TestInferiorCrashing.CrashingInferiorTestCase)
   Test that lldb expressions work before and after stepping after a crash.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1732, in dwarf_test_method
    return attrvalue(self)
  File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/decorators.py", line 110, in wrapper
    func(*args, **kwargs)
  File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/inferior-crashing/TestInferiorCrashing.py", line 82, in test_inferior_crashing_expr_step_and_expr
    self.inferior_crashing_expr_step_expr()
  File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/inferior-crashing/TestInferiorCrashing.py", line 249, in inferior_crashing_expr_step_expr
    self.check_stop_reason()
  File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/inferior-crashing/TestInferiorCrashing.py", line 93, in check_stop_reason
    STOPPED_DUE_TO_EXC_BAD_ACCESS)
AssertionError: 0 != 1 : Process should be stopped due to bad access exception
Config=x86_64-/usr/bin/cc
----------------------------------------------------------------------
Ran 7 tests in 4.320s

From lldbsuite/test/lldbutil.py:

def is_thread_crashed(test, thread):
    """In the test suite we dereference a null pointer to simulate a crash. The way this is
    reported depends on the platform."""
    if test.platformIsDarwin():
        return thread.GetStopReason(
        ) == lldb.eStopReasonException and "EXC_BAD_ACCESS" in thread.GetStopDescription(100)
    elif test.getPlatform() == "linux":
        return thread.GetStopReason() == lldb.eStopReasonSignal and thread.GetStopReasonDataAtIndex(
            0) == thread.GetProcess().GetUnixSignals().GetSignalNumberFromName("SIGSEGV")
    else:
        return "invalid address" in thread.GetStopDescription(100)

Presumably we want the second case to apply on FreeBSD as well when this patch is in, although I don't see why the existing else case shouldn't continue to work correctly.

Aug 8 2017, 12:02 AM

Jul 28 2017

labath added inline comments to D11465: Fix "process load/unload" on android.
Jul 28 2017, 11:34 AM
labath added a comment to D35740: Fix PR33875 by distinguishing between DWO and clang modules.

I looked at this briefly last week but I could not find a good fix in the amount of time I had left. This fixes the current test failure, but it does not fix the underlying bug, which is that we (sometimes?) set the compile unit offset for non-dwo, non-dsym DIEs as 0. This works fine if all compile units are regular dwarf as we have logic to locate the containing CU if the offset is zero. However, it can fail in case of mixed dwarf and dwo compile units. My guess is you could still reproduce this bug with a so file that has a dwo compile unit at offset zero.

Jul 28 2017, 11:19 AM
labath added a reviewer for D33213: Use socketpair on all Unix platforms: eugene.

I am not able to test this right now. Eugene, can you take a look?

Jul 28 2017, 11:06 AM

Jul 21 2017

labath committed rL308732: XFail TestWithModuleDebugging on linux (bug 33875).
XFail TestWithModuleDebugging on linux (bug 33875)
Jul 21 2017, 5:54 AM

Jul 20 2017

labath created D35666: Add data formatter for libc++ std::queue.
Jul 20 2017, 2:18 AM
labath added a comment to D35607: Extend 'target symbols add' to set symbol file for a given module.

So we should just make sure this works:

(lldb) target symbols add --shlib a.out debug_binaries/a.out

The --uuid option is there to help you match up without having to specify the file path. If I am understanding correctly, "a.out" has no UUID or build ID, but "debug_binaries/a.out" does?

In this case, neither of the files has a build id/UUID. However, the tricky part here is that in this case we "invent" a UUID by doing a checksum of the whole file. This is done to support the .gnu_debuglink https://sourceware.org/gdb/onlinedocs/gdb/Separate-Debug-Files.html method of finding symbols. It works like this:

  • if a file has a gnu_debuglink section, we set the UUID to the crc inside that section
  • if a file does not have a gnu_debuglink section, we *assume* it will be used as a gnu_debuglink target, and also set the UUID to the crc we compute from the whole file contents.

This works fine for the gnu_debuglink case, as then the rest of the machinery will match the files based on their UUIDs. However, it does not work for the general case without the debug link, as then the files will end up with two different "UUID"s (because they are based on the file content), and everything will consider them as different.

Jul 20 2017, 1:59 AM · Restricted Project
labath added a comment to D35311: lldb-server tests: Add support for testing debugserver.

This isn't about this patch, but replying to:

debugserver replies to the k packet instead of just dropping the connection - stopping code adjusted, although we should probably consider aligning the behavior of the two stubs in this case

Especially when dealing with flakey connections, knowing that you are done with a target was really handy. People what to hit "rerun" in some UI and have the rerun happen instantly, but if you have no way of knowing when the former process actually died, you have to introduce delays, which slows this down. This is particularly true when talking to an OS that really wants only one copy of an "app" running at a time (e.g. iOS) but is more generally useful for responsiveness. And if something goes wrong with killing the target, we could even use the new error reply strings to give more info about this.

Jul 20 2017, 1:41 AM

Jul 19 2017

labath added a comment to D35622: Fix LLDB crash accessing DWZ (Fedora) debug info.

If you can forward a DWARF file that exhibits this behavior I can take a look.

Jul 19 2017, 9:37 AM · Restricted Project
labath added a comment to D35311: lldb-server tests: Add support for testing debugserver.

@beanz: It looks like you're doing some debugserver work. :) Any thoughts on this?

Jul 19 2017, 8:29 AM
labath added a comment to D35607: Extend 'target symbols add' to set symbol file for a given module.

Btw, will this work correctly if we are cross-debugging from a mac? (because then we will use the fancy DownloadObjectAndSymbolFile implementation)

Jul 19 2017, 8:10 AM · Restricted Project
labath edited reviewers for D35622: Fix LLDB crash accessing DWZ (Fedora) debug info, added: clayborg; removed: labath.

I definitely agree we shouldn't crash if we see things we don't recognize, but I have no idea if it should be done this way. Greg should review this, as he's more familiar with the code.

Jul 19 2017, 8:08 AM · Restricted Project
labath created D35618: Remove shared_pointer from NativeThreadProtocol.
Jul 19 2017, 5:42 AM
labath created D35615: Add data formatter for libc++ std::tuple.
Jul 19 2017, 4:38 AM
labath accepted D31283: Expose hit count via SBBreakpointLocation..

The test looks well written. I've added a couple of suggestions you can consider including.

Jul 19 2017, 3:35 AM
labath accepted D35614: Update API headers..

thanks

Jul 19 2017, 2:24 AM
labath added a reviewer for D35607: Extend 'target symbols add' to set symbol file for a given module: jingham.

I think Jim (or maybe Greg?) should take a look at this.

Jul 19 2017, 2:22 AM · Restricted Project
labath added a comment to D33213: Use socketpair on all Unix platforms.

Before any changes are submitted, we assume you are getting a clean test suite run.

On my system, I get build failures in clang.

Jul 19 2017, 1:56 AM

Jul 18 2017

labath committed rL308307: Fix NetBSD/FreeBSD build after r308304.
Fix NetBSD/FreeBSD build after r308304
Jul 18 2017, 7:04 AM
labath closed D30961: Fix build with gcc 7.
Jul 18 2017, 6:40 AM
labath committed rL308304: Clean up lldb-types.h.
Clean up lldb-types.h
Jul 18 2017, 6:14 AM
labath closed D35113: Clean up lldb-types.h by committing rL308304: Clean up lldb-types.h.
Jul 18 2017, 6:14 AM
labath created D35556: Add data formatter for libc++'s forward_list.
Jul 18 2017, 5:51 AM
labath committed rL308292: Fix linux arm and mips builds broken by r308282.
Fix linux arm and mips builds broken by r308282
Jul 18 2017, 3:43 AM
labath requested changes to D33213: Use socketpair on all Unix platforms.

This is completely wrong. The --pipe argument is used to write the port that lldb-server listens on. Making --fd an alias to that will not help.

Jul 18 2017, 2:38 AM
labath committed rL308282: Remove shared pointer from NativeProcessProtocol.
Remove shared pointer from NativeProcessProtocol
Jul 18 2017, 2:25 AM
labath closed D35123: Remove shared pointer from NativeProcessProtocol by committing rL308282: Remove shared pointer from NativeProcessProtocol.
Jul 18 2017, 2:25 AM

Jul 14 2017

labath added a comment to D31172: Move stop info override callback code from ArchSpec into Process.
  • confine all the changes to m_arch to the SetArchitecture method. Right now only one of the assignments to m_arch is done outside of this method, and that one can be easily converted to a call to SetArchitecture(). This should make sure the two values never get out of sync.

That would be fine too.

I've tried that, and eventually decided against it, as it would create an uncomfortable mutual recursion between SetArchitecture and SetExecutableModule.

Jul 14 2017, 2:54 AM
labath updated the diff for D31172: Move stop info override callback code from ArchSpec into Process.

The version with a container object for ArchSpec+Plugin combo

Jul 14 2017, 2:53 AM

Jul 13 2017

labath added a comment to D31172: Move stop info override callback code from ArchSpec into Process.

Added a suggestion to make Target::GetArchitecturePlugin lazy and to ask if the arch is supported. Most of the time we will set this once and never need to change it, even when we attach, change arch, etc. The new suggestion will allow us to use the same arch plug-in without re-fetching it. Let me know what you think as the changes are thrown out there to see what people's opinions are.

Jul 13 2017, 8:57 AM
labath created D35356: [zorg] Enable assertions on the linux lldb bot.
Jul 13 2017, 7:59 AM
labath updated the diff for D31172: Move stop info override callback code from ArchSpec into Process.

Moving the plugin to Target and other minor fixups.

Jul 13 2017, 4:19 AM
labath added a comment to D31172: Move stop info override callback code from ArchSpec into Process.

Sorry I wasn't clear. I meant that since the Target knows everything it needs to know to vend the correct Architecture plugin, you should get it from the Target, not the Process. In general, I think that the highest class in the stack that can vend a plugin is the one that should.

Jul 13 2017, 4:16 AM

Jul 12 2017

labath added a comment to D31172: Move stop info override callback code from ArchSpec into Process.

This is a good addition. However, it seems to me that since you only need an ArchSpec to make one of these Architecture plugins, which plugin you get seems fully determined by the Target, not the Process. I understand that the only need for it at present is to do a Process-related task. But that task actually takes a Thread as a parameter, so the Architecture plugin doesn't need to know it's containing process to do its job. And it seems like it diminishes the plugin's future utility to have it more limited in scope than it needs to be.

What do you think?

Jul 12 2017, 1:03 PM
labath requested changes to D33213: Use socketpair on all Unix platforms.

Have you tried that this actually works? (It doesn't work for me)

Jul 12 2017, 9:57 AM
labath created D35311: lldb-server tests: Add support for testing debugserver.
Jul 12 2017, 9:46 AM
labath created D35305: Remove uint32_t assignment operator from Status.
Jul 12 2017, 7:27 AM
labath accepted D35298: [MainLoop] Fix possible use of an invalid iterator.

Looks great, thanks for catching this.

Jul 12 2017, 3:25 AM
labath updated the diff for D31172: Move stop info override callback code from ArchSpec into Process.

The architecture plugin idea

Jul 12 2017, 3:02 AM
labath commandeered D31172: Move stop info override callback code from ArchSpec into Process.

It looks like this has ground to a halt, but it seems the consensus was to try the Architecture plugin idea. I'm going to hijack this revision to maintain context, and upload a new version implementing that.

Jul 12 2017, 3:01 AM

Jul 11 2017

labath accepted D34945: Adding Support for Error Strings in Remote Packets.
Jul 11 2017, 8:30 AM
labath added a comment to D35083: [TypeSystem] Guard the global `ASTSourceMap` with a mutex.

As a general practice requiring a wrapper like this for every use of a should be locked object would make the code noisy and hard to read. The only error you would be protecting against is that somebody used the entity after the lock went out of scope. You could use some kind of markup to enforce this requirement, but you also have to be pretty sloppy to make this kind of error, so I'm not sure this it worth going to great lengths to protect against.

In this limited use, I guess I don't object seriously, but don't like this as a general pattern.

Jul 11 2017, 6:24 AM
labath committed rL307644: NativeProcessLinux: Fix handling of raise(SIGTRAP).
NativeProcessLinux: Fix handling of raise(SIGTRAP)
Jul 11 2017, 3:39 AM
labath updated the diff for D35113: Clean up lldb-types.h.

Use csignal instead of signal.h

Jul 11 2017, 2:39 AM
labath committed rL307636: NativeProcessLinux: Fix some compiler warnings.
NativeProcessLinux: Fix some compiler warnings
Jul 11 2017, 2:04 AM
labath committed rL307632: [LLDB][ppc64le] Rename enums in AuxVector.
[LLDB][ppc64le] Rename enums in AuxVector
Jul 11 2017, 1:40 AM
labath closed D35065: [LLDB][ppc64le] Rename enums in AuxVector by committing rL307632: [LLDB][ppc64le] Rename enums in AuxVector.
Jul 11 2017, 1:40 AM
labath added a reviewer for D35223: Report inferior SIGSEGV/SIGILL/SIGBUS/SIGFPE as a signal instead of an exception on freebsd: emaste.

Adding ed as freebsd owner.

Jul 11 2017, 12:53 AM

Jul 10 2017

labath added a comment to D35113: Clean up lldb-types.h.
Jul 10 2017, 4:43 AM
labath accepted D35065: [LLDB][ppc64le] Rename enums in AuxVector.

cool. thanks

Jul 10 2017, 4:17 AM

Jul 9 2017

labath added a comment to D35083: [TypeSystem] Guard the global `ASTSourceMap` with a mutex.

Ah, maybe you meant applying the thread safety annotation to Sean's solution to enforce the contract. That's an interesting solution, but makes a non-straightforward solution even more non-straightforward, so I agree this isn't the best example...

Jul 9 2017, 6:13 AM

Jul 7 2017

labath created D35123: Remove shared pointer from NativeProcessProtocol.
Jul 7 2017, 6:56 AM
labath committed rL307390: Add a NativeProcessProtocol Factory class.
Add a NativeProcessProtocol Factory class
Jul 7 2017, 4:03 AM
labath committed rL307391: Disable TestGoASTContext.
Disable TestGoASTContext
Jul 7 2017, 4:03 AM
labath closed D33778: Add a NativeProcessProtocol Factory class by committing rL307390: Add a NativeProcessProtocol Factory class.
Jul 7 2017, 4:02 AM
labath updated the diff for D35113: Clean up lldb-types.h.

Remove unused include from auxvector.h

Jul 7 2017, 3:29 AM
labath added inline comments to D35113: Clean up lldb-types.h.
Jul 7 2017, 3:29 AM
labath created D35113: Clean up lldb-types.h.
Jul 7 2017, 2:35 AM
labath added a comment to D35083: [TypeSystem] Guard the global `ASTSourceMap` with a mutex.

I would prefer the unique_lock idea also. Lamdas make the code look ugly.

Jul 7 2017, 1:49 AM
labath added inline comments to D35065: [LLDB][ppc64le] Rename enums in AuxVector.
Jul 7 2017, 1:17 AM

Jul 6 2017

labath added inline comments to D35065: [LLDB][ppc64le] Rename enums in AuxVector.
Jul 6 2017, 10:17 AM
labath committed rL307253: Fix a copy-paste error in r307161.
Fix a copy-paste error in r307161
Jul 6 2017, 4:44 AM
labath committed rL307252: Revert "Android.rules: build x86 tests with -mstackrealign".
Revert "Android.rules: build x86 tests with -mstackrealign"
Jul 6 2017, 4:43 AM
labath added a comment to D34945: Adding Support for Error Strings in Remote Packets.

I am generally happy with this, just a couple of things I noticed below:

Jul 6 2017, 2:42 AM

Jul 5 2017

labath added inline comments to D34945: Adding Support for Error Strings in Remote Packets.
Jul 5 2017, 8:19 AM
labath committed rL307161: Fix assorted compiler warnings (mismatched signedness and printf specifiers).
Fix assorted compiler warnings (mismatched signedness and printf specifiers)
Jul 5 2017, 7:55 AM
labath committed rL307160: Fix "process load" on new android targets.
Fix "process load" on new android targets
Jul 5 2017, 7:55 AM

Jul 4 2017

labath added a comment to D33035: Tool for using Intel(R) Processor Trace hardware feature.

Hm... that's a bit of a drag. I guess the SB API never ran into this problem because it always provides it's own vector-like classes (SBModuleList, SBFileSpecList, etc.). I guess the most canonical way would be to follow that example and have your own class for a list of instructions. However, that is a bit annoying, so I do see a case for making code generated by swig as an exception to the rule.

Hmm .. Is the decision about enabling exception handling only for swig generated code lie in LLDB's community's hands or we will have to ask LLVM community for this too? I can add another class to resolve this problem but I am not sure whether it is the right way to go.

Jul 4 2017, 7:56 AM
labath added a comment to D33035: Tool for using Intel(R) Processor Trace hardware feature.

Hi Pavel .. I could remove all exception handling code from source files. However, I am still wondering how to disable exception handling while providing python functions for C++ APIs of the features. Can you suggest something here? The whole problem is occurring because of the use of vector<> as an argument to one of the C++ APIs. To provide a python interface for these APIs, I am forced to include std_vector.i (please see tools/intel-features/intel-pt/interface/PTDecoder.i file) which is introducing exception handling code.

Jul 4 2017, 6:57 AM
labath committed rL307072: Update lldb architecture docs.
Update lldb architecture docs
Jul 4 2017, 5:30 AM
labath closed D34872: Update lldb architecture docs by committing rL307072: Update lldb architecture docs.
Jul 4 2017, 5:30 AM
labath committed rL307071: Fix some warnings in ProcessorTraceTest.cpp.
Fix some warnings in ProcessorTraceTest.cpp
Jul 4 2017, 5:30 AM
labath added a comment to D34945: Adding Support for Error Strings in Remote Packets.

With this patch, I see the extra packet to query from server is probably unnecessary. I mean the error code is still there and it should be sufficient for the client to detect an error. As long as it does not mistake it for something else it should not be a problem ? .

Jul 4 2017, 4:41 AM

Jul 3 2017

labath updated the diff for D33778: Add a NativeProcessProtocol Factory class.

remove extra pid check

Jul 3 2017, 8:55 AM
labath added inline comments to D34945: Adding Support for Error Strings in Remote Packets.
Jul 3 2017, 8:51 AM
labath added inline comments to D33778: Add a NativeProcessProtocol Factory class.
Jul 3 2017, 8:36 AM
labath accepted D34946: Fixing warnings for unused variables and copy ellision.

cool

Jul 3 2017, 7:39 AM
labath committed rL307018: Fix typo/unbreak windows build broken by r307009.
Fix typo/unbreak windows build broken by r307009
Jul 3 2017, 7:37 AM
labath committed rL307009: Use llvm::sys::RetryAfterSignal instead of a manual while errno!=EINTR loop.
Use llvm::sys::RetryAfterSignal instead of a manual while errno!=EINTR loop
Jul 3 2017, 7:37 AM
labath closed D33831: Add temp_failure_retry helper function by committing rL307009: Use llvm::sys::RetryAfterSignal instead of a manual while errno!=EINTR loop.
Jul 3 2017, 7:36 AM
labath added inline comments to D33778: Add a NativeProcessProtocol Factory class.
Jul 3 2017, 7:34 AM
labath updated the diff for D33778: Add a NativeProcessProtocol Factory class.

Address review comments

Jul 3 2017, 7:34 AM
labath accepted D34929: respect target.x86-disassembly-flavor when using `memory read`.

looks good

Jul 3 2017, 7:33 AM · Restricted Project
labath added a reviewer for D34776: Make i386-*-freebsd expression work on JIT path: emaste.

+ed as freebsd owner

Jul 3 2017, 7:33 AM