labath (Pavel Labath)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Today

labath added inline comments to D33434: Added new API to SBStructuredData class.
Wed, May 24, 10:13 AM
labath requested review of D33283: RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false.

Let me know what you think of the fix, and please confirm whether the ignoring of the breakpoint condition is a bug.

Wed, May 24, 8:41 AM
labath updated the diff for D33283: RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false.

Fixed version.

Wed, May 24, 8:39 AM
labath reopened D33283: RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false.

Reopening for a re-review of a fix.

Wed, May 24, 8:31 AM
labath committed rL303740: Revert "RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false".
Revert "RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false"
Wed, May 24, 4:57 AM
labath added a comment to D32366: Set "success" status correctly.

that sounds like an excellent idea, as it will check all executed commands, and not the ones we've remembered checking. It should probably be an lldbassert though. (And we'd need to check that the existing tests still pass after that.)

Wed, May 24, 2:49 AM · Restricted Project
labath committed rL303732: RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false.
RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false
Wed, May 24, 2:47 AM
labath closed D33283: RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false by committing rL303732: RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false.
Wed, May 24, 2:47 AM
labath accepted D32585: Implementation of remote packets for Trace data..

Looks good as far as I am concerned (please wait for greg's ok though)

Wed, May 24, 2:42 AM
labath added a comment to D33426: Introduce new command: thread backtrace unique.

Thanks for writing the test. We just need to make sure it runs in a stable manner.

Wed, May 24, 2:40 AM
labath added a comment to D32149: Correct handling NetBSD core(5) files with threads.

What was your decision on the core files? I was under the impression you were gonna add the zip files as well. If so, then they should go in at the same time.

Wed, May 24, 2:14 AM · Restricted Project
labath added a comment to D33283: RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false.

thank you

Wed, May 24, 2:10 AM

Yesterday

labath committed rL303627: Add support for new (3.0.11+) swigs.
Add support for new (3.0.11+) swigs
Tue, May 23, 3:55 AM
labath closed D33409: Add support for new (3.0.11+) swigs by committing rL303627: Add support for new (3.0.11+) swigs.
Tue, May 23, 3:55 AM
labath added inline comments to D32585: Implementation of remote packets for Trace data..
Tue, May 23, 3:19 AM
labath added inline comments to D32585: Implementation of remote packets for Trace data..
Tue, May 23, 3:13 AM
labath added a comment to D33426: Introduce new command: thread backtrace unique.

Sounds like an awesome feature.

Tue, May 23, 3:05 AM

Mon, May 22

labath closed D33347: Fix incorrect Status -> Error rename in IOHandler.
Mon, May 22, 7:14 AM
labath accepted D33347: Fix incorrect Status -> Error rename in IOHandler.

Committed as r303553. Thanks for the patch.

Mon, May 22, 7:14 AM
labath committed rL303553: Fix incorrect Status -> Error rename in IOHandler.
Fix incorrect Status -> Error rename in IOHandler
Mon, May 22, 7:13 AM
labath added inline comments to D32522: Test case for the issue raised in D32271.
Mon, May 22, 7:01 AM
labath created D33409: Add support for new (3.0.11+) swigs.
Mon, May 22, 6:49 AM
labath added a comment to D32585: Implementation of remote packets for Trace data..

Thank you for taking the time to write the test. Just a couple of more comments on things I noticed when going through this again:

Mon, May 22, 3:26 AM
labath added a comment to D32930: New framework for lldb client-server communication tests..

Ok, so the comments below are basically a collection of nits. The only two major issues that still need addressing are:

  • getting rid of the sleep in the startup code
  • deciding on the naming of members
Mon, May 22, 2:32 AM

Fri, May 19

labath added inline comments to D32930: New framework for lldb client-server communication tests..
Fri, May 19, 2:59 AM

Thu, May 18

labath updated the diff for D33283: RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false.

New version

Thu, May 18, 7:54 AM
labath added a comment to D33283: RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false.

Ok, I've missed the distinction between plan completing (aka being "done") and completing sucessfully. Things make a bit more sense after that.

Thu, May 18, 7:50 AM
labath committed rL303348: Add Status -- llvm::Error glue.
Add Status -- llvm::Error glue
Thu, May 18, 6:00 AM
labath closed D33241: Add Status -- llvm::Error glue by committing rL303348: Add Status -- llvm::Error glue.
Thu, May 18, 6:00 AM
labath added a comment to D32930: New framework for lldb client-server communication tests..

I think we're getting close, but I see a couple more issues here.

Thu, May 18, 5:50 AM
labath added a comment to D32585: Implementation of remote packets for Trace data..

Well nothings preventing me from doing so, I have the changes for that were suggested to me for this revision. I thought I would first upload them, so that people can look at the parallel while I upload the linux server code and Unit tests.

Ok so I will upload what I have right now, and let the people review it in parallel, while I work on the unit tests and I will add them here in the next differential to this patch. I won't merge this patch until then.

Thu, May 18, 5:25 AM
labath added a comment to D32585: Implementation of remote packets for Trace data..

llvm policy is to commit tests alongside the code under test. I also think it's easier to review as you have the code and the test on the same screen.

Thu, May 18, 4:50 AM

Wed, May 17

labath added a comment to D33283: RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false.

I'm not sure I understand what you're saying. Did you mean to say that I should add the "thread plan didn't successfully complete." (line 5281) block to the "Halt" branch as well ? (possibly by including it into the factored out function)

Wed, May 17, 1:31 PM
labath created D33283: RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false.
Wed, May 17, 8:21 AM
labath updated the diff for D33241: Add Status -- llvm::Error glue.

use a separate function instead of a conversion operator

Wed, May 17, 6:19 AM
labath committed rL303248: Make TestConflictingSymbol run on non-darwin targets.
Make TestConflictingSymbol run on non-darwin targets
Wed, May 17, 5:01 AM
labath added a comment to D33241: Add Status -- llvm::Error glue.

Mostly just that implicit conversion operators are a very subtle source of bugs, and you can't even find where they're being used because it's impossible to grep for it.

Wed, May 17, 3:08 AM
labath created D33269: MCJIT: add ProcessAllSections smoke test.
Wed, May 17, 2:20 AM
labath committed rL303239: [RuntimeDyld] Fix debug section relocation (pr20457).
[RuntimeDyld] Fix debug section relocation (pr20457)
Wed, May 17, 2:00 AM
labath closed D32899: [RuntimeDyld] Fix debug section relocation (pr20457) by committing rL303239: [RuntimeDyld] Fix debug section relocation (pr20457).
Wed, May 17, 2:00 AM

Tue, May 16

labath created D33241: Add Status -- llvm::Error glue.
Tue, May 16, 8:03 AM
labath committed rL303160: Skip TestWatchedVarHitWhenInScope on android arm because it triggers a kernel….
Skip TestWatchedVarHitWhenInScope on android arm because it triggers a kernel…
Tue, May 16, 5:11 AM
labath added inline comments to D33083: [Expression parser] Look up module symbols before hunting globally.
Tue, May 16, 2:06 AM

Mon, May 15

labath committed rL303076: Disable a test in TestReturnValue on arm64 linux.
Disable a test in TestReturnValue on arm64 linux
Mon, May 15, 9:38 AM
labath committed rL303061: Fix darwin build for r303058.
Fix darwin build for r303058
Mon, May 15, 6:55 AM
labath committed rL303058: Remove an expensive lock from Timer.
Remove an expensive lock from Timer
Mon, May 15, 6:16 AM
labath closed D32823: Remove an expensive lock from Timer by committing rL303058: Remove an expensive lock from Timer.
Mon, May 15, 6:16 AM
labath added a comment to D33167: Get rid of some uses of StringConvert and reduce some indentation.

I think this is a step in the right direction. Besides reducing boilerplate, this will also help us ensure correctness, as we get a constant trickle of bug reports for commands which forgot to set the result status.

Mon, May 15, 5:58 AM
labath added a comment to D32930: New framework for lldb client-server communication tests..

A bunch more pedantic comments from me.

Mon, May 15, 5:44 AM
labath added a comment to D33083: [Expression parser] Look up module symbols before hunting globally.

Is this feature really darwin specific? Isn't the __private_extern__ thingy equivalent to __attribute__(visibility("hidden"))), which is supported by gcc and clang alike?

Mon, May 15, 3:42 AM
labath added a comment to D32585: Implementation of remote packets for Trace data..

I quite like that you have added just the packet plumbing code without an concrete implementation. However, that is still a significant amount of parsing code that should be accompanied by a test. The test suite for the client side of the protocol is ready (TestGdbRemoteCommunicationClient), so I'd like to see at least that.

@labath I was considering writing Unit tests for the remote packets but I thought then I have to write the mock implementation for the trace operations as well, which might end up being a bigger piece of code than the actual packet parsing code.

I don't think that is the case. If you look at the tests in TestGdbRemoteCommunicationClient.cpp, all of them are quite simple. E.g. a test for TestGdbRemoteCommunicationClient could be something like:

TraceOptions opt;
opt.setType(...);
opt.setBufferSize(...);
opt.setMetaBufferSize(); // with an appropriate TraceOptions constructor, this could be a one-liner
  std::future<user_id_t> result = std::async(std::launch::async, [&] {
    return client.StartTrace(opt, error);
  });
HandlePacket(server, "JTrace:start:type:dead:buffersize:beef:metabuffersize:baad:threadid:f00d:jparams:...", "47");
ASSERT_EQ(47, result.get());
ASSERT_TRUE(error.Success());

This depends on the packet code being in GdbRemoteCommunicationClient and not ProcessGdbRemote as it is now, but moving it there is a good idea anyway -- ProcessGdbRemote should do higher level stuff, packet parsing should be left in the GDBRCClient.

Mon, May 15, 3:32 AM
labath abandoned D32295: [RFC] Fix crash in expression evaluation.

Fix is in D32899.

Mon, May 15, 3:06 AM
labath added a comment to D32899: [RuntimeDyld] Fix debug section relocation (pr20457).

Thanks for the help, this definitely looks much better. :)

Mon, May 15, 3:05 AM
labath updated the diff for D32899: [RuntimeDyld] Fix debug section relocation (pr20457).

Add llvm-rtdyld test case

Mon, May 15, 2:59 AM
labath added a comment to D33077: [TypeSystem] Fix inspection of Objective-C object types.

As this is an objective C feature, wouldn't a better place for it be in testcases/lang/objc ?

Mon, May 15, 2:31 AM
labath added a comment to D33035: Tool for using Intel(R) Processor Trace hardware feature.

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" with the existing "intel-mpx plugin" to create one "intel support" library?

Mon, May 15, 2:05 AM

Fri, May 12

labath added inline comments to D32930: New framework for lldb client-server communication tests..
Fri, May 12, 4:17 PM
labath added a comment to D32930: New framework for lldb client-server communication tests..

next batch of comments from me (I expect to have more on monday). :)

Fri, May 12, 3:52 PM

Mon, May 8

labath added a comment to D32597: Initiate loading of shared libraries in parallel.

I'm out of office this week. Could you hold until I get back? Hopefully we will see some development on the llvm/lld front in the meanwhile.

Mon, May 8, 1:33 PM
labath added a comment to D32930: New framework for lldb client-server communication tests..

The largest issue I see here is the use of exceptions, which are banned in llvm. We'll need to get rid of those before we start discussing anything else. This means we'll need to propagate errors manually, and we should design it in a way that it is easy to ASSERT on them. One option is to use llvm::Expected<T> for this, which is very good for this use, as it can actually contain a list of errors, which we can print out with a custom ASSERT_NO_ERROR macro. The usage would then be something like:

Expected<Foo> FooOr = getFoo();
ASSERT_NO_ERROR(FooOr);
// use FooOr.get()

However this has a disadvantage of the error message not containing the expression that failed, so it could be better having the functions just return llvm::Error and have the real return be in a by-ref argument:

Foo foo;
ASSERT_NO_ERROR(getFoo(foo));
Mon, May 8, 5:27 AM

Fri, May 5

labath accepted D32823: Remove an expensive lock from Timer.

lgtm, thank you.

Fri, May 5, 10:09 AM
labath added a comment to D32022: Fix backtrace of noreturn functions situated at the end of a module.

Jason, any thoughts on my comments above?

Fri, May 5, 6:09 AM
labath committed rL302225: Fix segfault resulting from empty print prompt.
Fix segfault resulting from empty print prompt
Fri, May 5, 5:22 AM
labath added a comment to D32421: Fix segfault resulting from empty print prompt.

Committed as 302225. I've fixed the indentation in your test, and added a couple of decorators to match other pexpect tests. Thanks for the patch!

Fri, May 5, 5:08 AM
labath closed D32421: Fix segfault resulting from empty print prompt by committing rL302225: Fix segfault resulting from empty print prompt.
Fri, May 5, 5:04 AM
labath committed rL302223: Add TaskMap for iterating a function over a set of integers.
Add TaskMap for iterating a function over a set of integers
Fri, May 5, 4:30 AM
labath closed D32757: Add TaskMap for iterating a function over a set of integers by committing rL302223: Add TaskMap for iterating a function over a set of integers.
Fri, May 5, 4:30 AM
labath accepted D32757: Add TaskMap for iterating a function over a set of integers.

I can do the pushing. :)

Fri, May 5, 4:29 AM
labath added inline comments to D32820: Parallelize demangling.
Fri, May 5, 4:09 AM
labath committed rL302220: ABISysV_arm64: compute return value for large vectors correctly.
ABISysV_arm64: compute return value for large vectors correctly
Fri, May 5, 4:03 AM
labath closed D32813: ABISysV_arm64: compute return value for large vectors correctly by committing rL302220: ABISysV_arm64: compute return value for large vectors correctly.
Fri, May 5, 4:03 AM
labath created D32899: [RuntimeDyld] Fix debug section relocation (pr20457).
Fri, May 5, 3:58 AM
labath added a comment to D32823: Remove an expensive lock from Timer.

Ok, then let's keep them. I don't mind changing all call sites -- having a separate category object is the cleanest solution with least magic. However see my comments about category naming and merging.

Fri, May 5, 2:32 AM

Thu, May 4

labath added a comment to D32813: ABISysV_arm64: compute return value for large vectors correctly.

I am a bit confused by the correlation between your change and commit message. In the commit message you say that 32 byte structs

I mean 32-byte vectors. I.e. variables declared as float foo __attribute__((__vector_size__(32)));

Thu, May 4, 7:16 AM
labath accepted D32711: Add missing 'arch' key to valid qHostInfo keys.
Thu, May 4, 3:30 AM
labath committed rL302133: MainLoop: Add unit tests.
MainLoop: Add unit tests
Thu, May 4, 3:24 AM
labath closed D32753: MainLoop: Add unit tests by committing rL302133: MainLoop: Add unit tests.
Thu, May 4, 3:24 AM
labath added a comment to D32832: Make ConstString creation and access lockfree.

I have feeling you gave up on the llvm change too quickly. My interpretation of that thread was that there was general support for the hash function switch, and people only wanted some confirmation it will not regress.

Thu, May 4, 3:05 AM
labath accepted D32421: Fix segfault resulting from empty print prompt.

Perfect, thank you.

Thu, May 4, 2:36 AM
labath added a comment to D32823: Remove an expensive lock from Timer.

Don't forget to update the usages in unit tests (and make sure the check-lldb-unit target passes).

Thu, May 4, 2:35 AM

Wed, May 3

labath added inline comments to D32826: Move Parallel.h from LLD to LLVM.
Wed, May 3, 2:04 PM
labath added inline comments to D32826: Move Parallel.h from LLD to LLVM.
Wed, May 3, 1:32 PM
labath added a comment to D32787: Fix build error: no viable conversion from returned value of type 'int' to function return type 'sigset_t' (aka '__sigset_t').

Could it be that you just have stale cmake cache?

That could easily be true. Rerunning cmake didn't fix it; short of deleting the entire build directory, is there a way to refresh the cache?

Wed, May 3, 12:53 PM
labath added a comment to D32787: Fix build error: no viable conversion from returned value of type 'int' to function return type 'sigset_t' (aka '__sigset_t').

Looking at CMakeError.log, the test program does #include <poll.h> but does not #define _GNU_SOURCE. The define has to be there for your example to compile on my Ubuntu.

Wed, May 3, 11:46 AM
labath created D32813: ABISysV_arm64: compute return value for large vectors correctly.
Wed, May 3, 10:13 AM
labath committed rL302013: Windows fix for TestConflictingDefinition makefile.
Windows fix for TestConflictingDefinition makefile
Wed, May 3, 4:40 AM
labath committed rL302008: Check for lack of C++ context first when demangling.
Check for lack of C++ context first when demangling
Wed, May 3, 3:13 AM
labath closed D32708: Check for lack of C++ context first when demangling by committing rL302008: Check for lack of C++ context first when demangling.
Wed, May 3, 3:13 AM
labath added a comment to D32787: Fix build error: no viable conversion from returned value of type 'int' to function return type 'sigset_t' (aka '__sigset_t').

Hi Pavel,

Could one of you guys check whether you really don't have the ppoll syscall (for example, are able to compile a simple program using ppoll (*))

It has! clang is able to compile the simple program!

If you do, then I may need a bit more help to debug the cmake detection logic

Yes, it needs to add the detection logic in the CMakeLists.txt :)

Wed, May 3, 2:58 AM
labath requested changes to D32787: Fix build error: no viable conversion from returned value of type 'int' to function return type 'sigset_t' (aka '__sigset_t').

This will not compile on windows, which really is the only branch we expected to have SIGNAL_POLLING_UNSUPPORTED set. In this sense Pauls (cc'ed) fix is better, but it does not actually get lldb going on the affected systems. Could one of you guys check whether you really don't have the ppoll syscall (for example, are able to compile a simple program using ppoll (*)). If you really don't have ppoll, then I can hook up cmake so it falls back to pselect. If you do, then I may need a bit more help to debug the cmake detection logic.

Wed, May 3, 2:26 AM
labath added a comment to D32421: Fix segfault resulting from empty print prompt.

It's definitely still a bug worth fixing, we cannot rely on undefined behavior like that.

Wed, May 3, 2:12 AM

Tue, May 2

labath created D32753: MainLoop: Add unit tests.
Tue, May 2, 9:53 AM
labath accepted D32719: Don't attempt to use mpx registers on unsupported platforms.

I don't like either of the solutions too much, but this one is at least less code. :)

Tue, May 2, 7:30 AM
labath committed rL301918: Android.rules: set "ar" path correctly.
Android.rules: set "ar" path correctly
Tue, May 2, 6:27 AM
labath committed rL301917: ObjectFileELF: Fix symbol lookup in bss section.
ObjectFileELF: Fix symbol lookup in bss section
Tue, May 2, 5:53 AM
labath closed D32434: ObjectFileELF: Fix symbol lookup in bss section by committing rL301917: ObjectFileELF: Fix symbol lookup in bss section.
Tue, May 2, 5:53 AM
labath committed rL301908: Change UniqueCStringMap to use ConstString as the key.
Change UniqueCStringMap to use ConstString as the key
Tue, May 2, 3:30 AM
labath closed D32316: Change UniqueCStringMap to use ConstString as the key by committing rL301908: Change UniqueCStringMap to use ConstString as the key.
Tue, May 2, 3:30 AM
labath added a comment to D32708: Check for lack of C++ context first when demangling.

I am having trouble applying this. Do you need to rebase or something?

Tue, May 2, 3:11 AM
labath updated subscribers of D32732: "target variable" not showing all global variable if we print any global variable before execution starts.

Thanks for the patch. Could you also write a test case for the bug? It sounds like all that is necessary is to move the commands from your commit message into a test.

Tue, May 2, 2:20 AM