labath (Pavel Labath)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 4 2013, 6:02 AM (211 w, 5 d)

Recent Activity

Today

labath created D34613: Add debug_frame section support.
Mon, Jun 26, 4:44 AM
labath committed rL306278: Shorten sanitizer plugin names.
Shorten sanitizer plugin names
Mon, Jun 26, 2:44 AM
labath closed D34553: Shorten sanitizer plugin names by committing rL306278: Shorten sanitizer plugin names.
Mon, Jun 26, 2:43 AM

Fri, Jun 23

labath added a comment to D34553: Shorten sanitizer plugin names.

Oh wow, we really need to limit path lengths?

Fri, Jun 23, 8:13 AM
labath committed rL306098: Fix build breakage caused by r306096.
Fix build breakage caused by r306096
Fri, Jun 23, 6:14 AM
labath committed rL306096: [ADT] Add llvm::to_float.
[ADT] Add llvm::to_float
Fri, Jun 23, 5:56 AM
labath closed D34518: [ADT] Add llvm::to_float by committing rL306096: [ADT] Add llvm::to_float.
Fri, Jun 23, 5:55 AM
labath created D34553: Shorten sanitizer plugin names.
Fri, Jun 23, 5:48 AM
labath added a comment to D34518: [ADT] Add llvm::to_float.

it looks like it doesn't easily support long double

APFloat::convertFromString works for every floating-point type supported by APFloat; clang uses it to parse floating-point literals. There isn't any convenient way to convert an APFloat to a long double, but that's mostly because there hasn't been any demand for it; the format of long double is platform-dependent.

Fri, Jun 23, 3:34 AM

Thu, Jun 22

labath created D34518: [ADT] Add llvm::to_float.
Thu, Jun 22, 10:37 AM
labath committed rL306013: Simplify the gdb-remote unit tests.
Simplify the gdb-remote unit tests
Thu, Jun 22, 8:55 AM
labath updated the diff for D33895: [Support] Add TempFailureRetry helper function.

Go back to the decltype version

Thu, Jun 22, 8:32 AM
labath added inline comments to D33674: Implementation of Intel(R) Processor Trace support for Linux.
Thu, Jun 22, 8:25 AM
labath added a comment to D33895: [Support] Add TempFailureRetry helper function.

Do you need someone to commit this?

Thu, Jun 22, 8:18 AM
labath requested review of D33895: [Support] Add TempFailureRetry helper function.
Thu, Jun 22, 7:36 AM
labath reopened D33895: [Support] Add TempFailureRetry helper function.
Thu, Jun 22, 7:35 AM
labath committed rL306005: Revert "[Support] Add RetryAfterSignal helper function" and subsequent fix.
Revert "[Support] Add RetryAfterSignal helper function" and subsequent fix
Thu, Jun 22, 7:19 AM
labath committed rL306003: [Support] Fix return type deduction in RetryAfterSignal.
[Support] Fix return type deduction in RetryAfterSignal
Thu, Jun 22, 6:56 AM
labath committed rL306001: [Testing/Support] Remove the const_cast in TakeExpected.
[Testing/Support] Remove the const_cast in TakeExpected
Thu, Jun 22, 6:12 AM
labath closed D34405: [Testing/Support] Remove the const_cast in TakeExpected by committing rL306001: [Testing/Support] Remove the const_cast in TakeExpected.
Thu, Jun 22, 6:12 AM

Wed, Jun 21

labath added a comment to D33674: Implementation of Intel(R) Processor Trace support for Linux.

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
Wed, Jun 21, 8:51 AM
labath committed rL305895: Fix build after r305892.
Fix build after r305892
Wed, Jun 21, 4:10 AM
labath committed rL305892: [Support] Add RetryAfterSignal helper function.
[Support] Add RetryAfterSignal helper function
Wed, Jun 21, 3:56 AM
labath closed D33895: [Support] Add TempFailureRetry helper function by committing rL305892: [Support] Add RetryAfterSignal helper function.
Wed, Jun 21, 3:56 AM
labath added a comment to D33895: [Support] Add TempFailureRetry helper function.

Thank you.

Wed, Jun 21, 3:36 AM
labath added a comment to D34405: [Testing/Support] Remove the const_cast in TakeExpected.

I am not entirely thrilled by that, as that adds more bloat to the error message ("llvm::detail::TakeExpected(std::move(original_assert_expression)) failed"), but I could go with that.

I think we can fix that by using a full blown class based implementation of MatcherImpl instead of the MATCHER_P macro. But it was a bit tricky so I didn't attempt it yet.

Wed, Jun 21, 3:35 AM

Tue, Jun 20

labath added a comment to D34405: [Testing/Support] Remove the const_cast in TakeExpected.

Right, but we have the ASSERT_THAT_EXPECTED macro, which can be re-written as ASSERT_THAT(llvm::detail::Expected(std::move(Err), Matcher))

Tue, Jun 20, 10:02 AM
labath added inline comments to D34405: [Testing/Support] Remove the const_cast in TakeExpected.
Tue, Jun 20, 9:38 AM
labath created D34405: [Testing/Support] Remove the const_cast in TakeExpected.
Tue, Jun 20, 9:02 AM
labath added a comment to D33895: [Support] Add TempFailureRetry helper function.

If there are no further comments, can I commit this?

Tue, Jun 20, 7:18 AM
labath created D34400: Move Connection from Core to Host.
Tue, Jun 20, 5:56 AM
labath committed rL305789: Delete a lot of empty directories in test/.
Delete a lot of empty directories in test/
Tue, Jun 20, 4:25 AM
labath committed rL305780: [linux] Change the way we load vdso pseudo-module.
[linux] Change the way we load vdso pseudo-module
Tue, Jun 20, 1:12 AM
labath closed D34352: [linux] Change the way we load vdso pseudo-module by committing rL305780: [linux] Change the way we load vdso pseudo-module.
Tue, Jun 20, 1:12 AM
labath committed rL305779: Remove home-grown thread-local storage wrappers.
Remove home-grown thread-local storage wrappers
Tue, Jun 20, 1:12 AM
labath closed D34274: Remove home-grown thread-local storage wrappers by committing rL305779: Remove home-grown thread-local storage wrappers.
Tue, Jun 20, 1:12 AM
labath committed rL305778: ProcessLauncherPosixFork: Fetch errno early.
ProcessLauncherPosixFork: Fetch errno early
Tue, Jun 20, 1:12 AM

Mon, Jun 19

labath added a comment to D33674: Implementation of Intel(R) Processor Trace support for Linux.

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

Lets say an application spawns 10 threads, now here we can end up with 9 thread groups in the worst case if the user calls "trace all threads" after each
new spawned thread.

Now I wanted to avoid having multiple thread groups coz the implementation will become more complex.

Mon, Jun 19, 8:26 AM
labath created D34352: [linux] Change the way we load vdso pseudo-module.
Mon, Jun 19, 8:08 AM
labath committed rL305689: Add pretty-printer for wait(2) statuses and modernize the code handling them.
Add pretty-printer for wait(2) statuses and modernize the code handling them
Mon, Jun 19, 5:48 AM
labath closed D33998: Add pretty-printer for wait(2) statuses and modernize the code handling them by committing rL305689: Add pretty-printer for wait(2) statuses and modernize the code handling them.
Mon, Jun 19, 5:48 AM
labath committed rL305687: Tweak SysV_arm64 function entry unwind plan.
Tweak SysV_arm64 function entry unwind plan
Mon, Jun 19, 5:40 AM
labath closed D34199: Tweak SysV_arm64 function entry unwind plan by committing rL305687: Tweak SysV_arm64 function entry unwind plan.
Mon, Jun 19, 5:40 AM
labath committed rL305686: Delete ProcessLauncherPosix.
Delete ProcessLauncherPosix
Mon, Jun 19, 5:27 AM
labath closed D34236: Delete ProcessLauncherPosix by committing rL305686: Delete ProcessLauncherPosix.
Mon, Jun 19, 5:27 AM
labath added a comment to D34274: Remove home-grown thread-local storage wrappers.

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. So hopefully there's no more hurdles to getting this in.

Mon, Jun 19, 5:23 AM
labath added inline comments to D33035: Tool for using Intel(R) Processor Trace hardware feature.
Mon, Jun 19, 5:18 AM
labath added a comment to D33674: Implementation of Intel(R) Processor Trace support for Linux.

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 to trace newly spawned threads with a smaller buffer and trace an individual thread with perhaps a bigger buffer size. Tracing all threads is definitely important coz the user might not want to separately start tracing on each new thread. Now the current design might be a bit confusing but I am willing to document it well with examples and uses cases.

Mon, Jun 19, 4:43 AM
labath added a comment to D34236: Delete ProcessLauncherPosix.

LGTM from the FreeBSD side. The launch code for FreeBSD came from the original (in-process) implementation that Linux and FreeBSD shared.

Mon, Jun 19, 4:20 AM
labath added a comment to D33674: Implementation of Intel(R) Processor Trace support for Linux.

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 something that we should discourage rather than try to support. The messaging I'd give to the user is: "you should trace the whole process unless you have a good reason not to". In case of a single threaded application, it won't make a difference, and if that app suddenly turns multithreaded, it will be traced as well. If he happens to have a single-thread trace running and later chooses to have a full process traced, he can always cancel that one and initiate a full trace instead.

Mon, Jun 19, 2:57 AM
labath resigned from D34281: [Format] Simplify some of the formatter template glue.

I am not sure I can bring much to this, so I'll take a back seat.

Mon, Jun 19, 2:14 AM

Fri, Jun 16

labath created D34274: Remove home-grown thread-local storage wrappers.
Fri, Jun 16, 5:28 AM
labath added inline comments to D33895: [Support] Add TempFailureRetry helper function.
Fri, Jun 16, 2:28 AM
labath updated the diff for D33895: [Support] Add TempFailureRetry helper function.

Fix the function, as suggested by EricWF.

Fri, Jun 16, 2:27 AM
labath added inline comments to D33674: Implementation of Intel(R) Processor Trace support for Linux.
Fri, Jun 16, 2:17 AM
labath added a comment to D34236: Delete ProcessLauncherPosix.

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 extensions which make it possible, so it makes sense to keep using it. However, it also makes the usage of that function non-portable. The only reason FreeBSD was able to get away with it was because it rolled it's own version of "launch for debug" code (source/Plugins/Process/FreeBSD/ProcessMonitor.cpp). After switching to a more generic launching primitive, that code could disappear.

Fri, Jun 16, 1:43 AM

Thu, Jun 15

labath added a comment to D33895: [Support] Add TempFailureRetry helper function.

Is this good to go now? :)

Thu, Jun 15, 6:37 AM
labath created D34236: Delete ProcessLauncherPosix.
Thu, Jun 15, 6:25 AM
labath added a comment to D33674: Implementation of Intel(R) Processor Trace support for Linux.

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.

Thu, Jun 15, 5:23 AM
labath committed rL305462: Add llvm::Error assignment operator to Status class.
Add llvm::Error assignment operator to Status class
Thu, Jun 15, 4:24 AM
labath committed rL305461: [swig] Improve the native module import logic.
[swig] Improve the native module import logic
Thu, Jun 15, 4:24 AM

Wed, Jun 14

labath updated the diff for D33895: [Support] Add TempFailureRetry helper function.

I've renamed the function.

Wed, Jun 14, 7:11 AM
labath added a comment to D33895: [Support] Add TempFailureRetry helper function.

Any thoughts on this? Or suggestions for other reviewers?

Wed, Jun 14, 5:33 AM
labath added a comment to D33504: Fix FDE indexing while scan debug_info section.

I've looked into the other test failures I was seeing on arm64 after merging this in. I have a fix for this up in D34199.

Wed, Jun 14, 3:27 AM
labath created D34199: Tweak SysV_arm64 function entry unwind plan.
Wed, Jun 14, 3:16 AM

Tue, Jun 13

labath committed rL305286: Mark TestCallThatRestarts as flaky on android.
Mark TestCallThatRestarts as flaky on android
Tue, Jun 13, 4:13 AM
labath updated the diff for D33998: Add pretty-printer for wait(2) statuses and modernize the code handling them.

Use a switch instead of indexing the array with an enum value

Tue, Jun 13, 3:45 AM
labath added inline comments to D33998: Add pretty-printer for wait(2) statuses and modernize the code handling them.
Tue, Jun 13, 3:44 AM
labath added a comment to D33059: Create an shared include directory for gtest helper functions.

I think it is possible to avoid the explosion of EXPECT/ASSERT_THAT_<FOO> macros (I can't think of any right now, but I'm sure something will crop up eventually, which will require specific handling).

There are a lot of things we can investigate to make this better. However, I don't think they belong in this patch. This patch is *mostly* about consolidating where things live to a common home. It does so. It updates some custom macro expectations to be slightly different custom macro expectations with specific benefits Zach outlined.

Because these were already custom macros, I consider this "no harm done". I wouldn't want to go farther than that in this patch though, I'd prefer to get this (or something like it) in place to prove the concept of building more robust libraries that make testing easy and effective. Once we have that, we can write a much more focused patch that *just* tries to improve these particular cases to not require custom macros.

I'm refraining from commenting on any particulars of the approach you suggest or alternatives for that future time / place / discussion.

Tue, Jun 13, 2:14 AM
labath added a comment to D33059: Create an shared include directory for gtest helper functions.

about the directories: 'meh'. I'm fine with whatever works.

  • Why is a patch that claims to create a shared include directory actually changing so many unit tests?

I felt like we needed a motivating example of why this effort is useful. The error macros were copy/pasted amongst many different files, so it was the perfect candidate.

  • EXPECT_THAT_ERROR(x, Succeeded()) seems bad. SHouldn't this rather be EXPECT_THAT(x, succeeded()) to look more natural?

You're right, it is bad. But unfortunately there is not a good alternative. Error is a move only type, and gmock a) can't handle move only types, and b) can't exhibits undefined behavior if you change the state of an object you're matching inside of a matcher. Because of that, we have to convert the Error into something else by basically consuming its failure/success status and error messge and storing them in a different type, and then matching against that type. So we need the macro to convert the Error / Expected into the type we actually match against.

It's not ideal, but it seems like the best we can do for this one specific class.

Tue, Jun 13, 1:43 AM

Mon, Jun 12

labath added a comment to D33426: Introduce new command: thread backtrace unique.

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 differences: (spoiler: the magic command that worked was thread continue #id)

Mon, Jun 12, 9:28 AM
labath committed rL305197: Introduce new command: thread backtrace unique.
Introduce new command: thread backtrace unique
Mon, Jun 12, 9:26 AM
labath closed D33426: Introduce new command: thread backtrace unique by committing rL305197: Introduce new command: thread backtrace unique.
Mon, Jun 12, 9:26 AM

Fri, Jun 9

labath committed rL305062: Skip TestNoreturnUnwind on linux+clang+arm.
Skip TestNoreturnUnwind on linux+clang+arm
Fri, Jun 9, 1:34 AM

Thu, Jun 8

labath committed rL304976: Fix backtrace of noreturn functions situated at the end of a module.
Fix backtrace of noreturn functions situated at the end of a module
Thu, Jun 8, 6:27 AM
labath closed D32022: Fix backtrace of noreturn functions situated at the end of a module by committing rL304976: Fix backtrace of noreturn functions situated at the end of a module.
Thu, Jun 8, 6:27 AM
labath added a comment to D32022: Fix backtrace of noreturn functions situated at the end of a module.

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. RegisterContextLLDB (computes the backtrace) and StackFrame (computes the symbol name to display). This avoids the duplication, but adds another argument to a bunch of functions. I don't really have an idea on how to make this better, but maybe you will think of something.

Thu, Jun 8, 5:30 AM

Wed, Jun 7

labath added inline comments to D33998: Add pretty-printer for wait(2) statuses and modernize the code handling them.
Wed, Jun 7, 11:18 AM
labath updated the diff for D33998: Add pretty-printer for wait(2) statuses and modernize the code handling them.

Fix typo

Wed, Jun 7, 9:44 AM
labath created D33998: Add pretty-printer for wait(2) statuses and modernize the code handling them.
Wed, Jun 7, 9:36 AM
labath committed rL304924: Switch TaskMapOverInt to llvm::function_ref.
Switch TaskMapOverInt to llvm::function_ref
Wed, Jun 7, 9:28 AM
labath added inline comments to D33674: Implementation of Intel(R) Processor Trace support for Linux.
Wed, Jun 7, 8:27 AM
labath added a comment to D33426: Introduce new command: thread backtrace unique.

Cool. Sorry about all the back-and-forth and thanks for the patience.

Wed, Jun 7, 1:35 AM

Tue, Jun 6

labath committed rL304796: Fix assorted compiler warnings. NFC.
Fix assorted compiler warnings. NFC
Tue, Jun 6, 7:06 AM
labath committed rL304795: replace uses of strerror with llvm::sys::StrError.
replace uses of strerror with llvm::sys::StrError
Tue, Jun 6, 7:06 AM
labath added a comment to D32022: Fix backtrace of noreturn functions situated at the end of a module.

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.

Tue, Jun 6, 6:53 AM
labath updated the diff for D32022: Fix backtrace of noreturn functions situated at the end of a module.

Add documentation.

Tue, Jun 6, 6:50 AM
labath committed rL304793: New framework for lldb client-server communication tests..
New framework for lldb client-server communication tests.
Tue, Jun 6, 6:41 AM
labath committed rL304794: Remove unused variables. NFC.
Remove unused variables. NFC
Tue, Jun 6, 6:41 AM
labath closed D32930: New framework for lldb client-server communication tests. by committing rL304793: New framework for lldb client-server communication tests..
Tue, Jun 6, 6:40 AM
labath added a comment to D32930: New framework for lldb client-server communication tests..

I am going to commit this, so we can move forward with the remote testing aspect.

Tue, Jun 6, 6:39 AM

Mon, Jun 5

labath added inline comments to D33504: Fix FDE indexing while scan debug_info section.
Mon, Jun 5, 10:11 AM
labath created D33895: [Support] Add TempFailureRetry helper function.
Mon, Jun 5, 7:24 AM
labath added a comment to D33504: Fix FDE indexing while scan debug_info section.

Thank you for adding the support for dwarf 4 CIE. I think it's a good start, but I believe the version check needs to be done in a smarter way.

Mon, Jun 5, 6:23 AM
labath added a comment to D33426: Introduce new command: thread backtrace unique.

@jingham @labath do you have any more feedback?

Mon, Jun 5, 4:05 AM

Fri, Jun 2

labath added inline comments to D33831: Add temp_failure_retry helper function.
Fri, Jun 2, 8:00 AM
labath updated the diff for D33831: Add temp_failure_retry helper function.
  • fix freebsd typo
  • use ::waitpid consistently
Fri, Jun 2, 7:51 AM
labath created D33831: Add temp_failure_retry helper function.
Fri, Jun 2, 7:35 AM
labath planned changes to D33778: Add a NativeProcessProtocol Factory class.

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.

Fri, Jun 2, 7:25 AM
labath committed rL304544: cmake: Put PROCESS_VM_READV detection results into Config.h.
cmake: Put PROCESS_VM_READV detection results into Config.h
Fri, Jun 2, 5:29 AM