Page MenuHomePhabricator

labath (Pavel Labath)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Today

labath committed rGae316ac66fff: [lldb/qemu] Sort entries in QEMU_(UN)SET_ENV (authored by labath).
[lldb/qemu] Sort entries in QEMU_(UN)SET_ENV
Wed, Dec 8, 6:57 AM
labath committed rG88c183e978ed: [lldb] Fix TestDataFormatterGenericList (authored by labath).
[lldb] Fix TestDataFormatterGenericList
Wed, Dec 8, 4:44 AM
labath committed rG5ce0f8763261: [lldb] Unify two versions of TestMemoryRead (authored by labath).
[lldb] Unify two versions of TestMemoryRead
Wed, Dec 8, 4:28 AM
labath committed rG45aa435661d8: [lldb/qemu] Separate host and target environments (authored by labath).
[lldb/qemu] Separate host and target environments
Wed, Dec 8, 4:09 AM
labath closed D115246: [lldb/qemu] Separate host and target environments.
Wed, Dec 8, 4:08 AM · Restricted Project
labath added a comment to D115246: [lldb/qemu] Separate host and target environments.

Thanks for all the reviews. I'll probably take a break with this now. I'll need to integrate the existing stuff into our internal infrastructure before adding more features.

Wed, Dec 8, 2:56 AM · Restricted Project
labath added inline comments to D115246: [lldb/qemu] Separate host and target environments.
Wed, Dec 8, 2:53 AM · Restricted Project
labath added a comment to D115126: [LLDB] Add unit tests for Editline keyboard shortcuts.

Well.. I would say that the most user-facing aspect is the /action/ that happens after the user pressed the appropriate key. But at the end of the day, that doesn't matter that much -- we can (and do) test non-user-facing interfaces as well. But those tests make most sense when there is some complex logic in the code. Making sure that a table does not change by keeping another copy of the table is the very definition of a change-detector test (https://testing.googleblog.com/2015/01/testing-on-toilet-change-detector-tests.html). Like, it may have some value while you're doing the refactoring, but that value quickly drops to zero (or even becomes negative) after you complete it.

Wed, Dec 8, 2:36 AM · Restricted Project
labath updated the diff for D115246: [lldb/qemu] Separate host and target environments.
  • add some explanatory comments
  • make the function return a new environment
Wed, Dec 8, 1:33 AM · Restricted Project
labath added inline comments to D115246: [lldb/qemu] Separate host and target environments.
Wed, Dec 8, 1:33 AM · Restricted Project

Yesterday

labath accepted D115211: [lldb] Make the LLDB version a first class citizen.
Tue, Dec 7, 11:37 PM · Restricted Project
labath added a comment to D115308: [LLDB] Uniquify Type in type list..

Thanks for fixing this. I assume that the type is already being added to the type list somewhere else -- it'd be nice to say, for future archaeologists' sake, where that actually happens.

Tue, Dec 7, 11:10 PM · Restricted Project
labath added a comment to D115211: [lldb] Make the LLDB version a first class citizen.

The main reason I wanted to avoid a separate library is because I could not imagine how would it fit into the general library landscape in lldb. It's also moderately strange to have a library for just a single function, but that is something I can live with.

Tue, Dec 7, 11:10 AM · Restricted Project
labath requested review of D115246: [lldb/qemu] Separate host and target environments.
Tue, Dec 7, 5:48 AM · Restricted Project
labath committed rGd4083a296ac8: [lldb] Fix flakyness in TestQemuLaunch.test_stdio_redirect (authored by labath).
[lldb] Fix flakyness in TestQemuLaunch.test_stdio_redirect
Tue, Dec 7, 5:20 AM
labath committed rG611fdde4c765: [lldb/qemu] Add emulator-args setting (authored by labath).
[lldb/qemu] Add emulator-args setting
Tue, Dec 7, 5:20 AM
labath closed D115151: [lldb/qemu] Add emulator-args setting.
Tue, Dec 7, 5:19 AM · Restricted Project
labath accepted D113930: [LLDB][NativePDB] Fix function decl creation for class methods.

I am still not sure this is the right solution, but a) I don't know what *is* the right solution; and (b) I don't think this makes the situation worse; so I am hitting accept.

Tue, Dec 7, 12:40 AM · Restricted Project
labath accepted D115073: Modify DataEncoder to be able to encode data in an object owned buffer..

This is what I hand in mind. Thanks.

Tue, Dec 7, 12:07 AM · Restricted Project
labath added a comment to D114746: [lldb] Generalize ParsedDWARFTypeAttributes.

I can't help the feeling that the boilerplate introduced by the enum compression is just not worth the benefit it brings, but otherwise this looks ok apart from the two inline comments.

Tue, Dec 7, 12:01 AM · Restricted Project

Mon, Dec 6

labath added a comment to D115211: [lldb] Make the LLDB version a first class citizen.

I am definitely not sad to see lldbBase go away, but I am wondering if we really need a separate library for this functionality (function).

Mon, Dec 6, 11:45 PM · Restricted Project
labath added a comment to D113930: [LLDB][NativePDB] Fix function decl creation for class methods.

Thanks for the explanation. This makes things a lot clearer (though it doesn't convince me that this is the right approach).

Mon, Dec 6, 12:39 PM · Restricted Project
labath added a comment to D115073: Modify DataEncoder to be able to encode data in an object owned buffer..

I don't think we actually disagree here. I'm aware of your use case (let's call it "dynamic mode") for appending to a buffer. I agree it's useful and I don't want to change that. What I want to remove is the other mode (non-dynamic). Presently, this is the only mode, but we're not actually making a good use of it. All of the existing use cases can be implemented with a "dynamic" buffer. And the code would be much simpler since there is only one mode to support -- one in which the data encoder owns the buffer it is writing to and can resize it at will.

Mon, Dec 6, 12:08 PM · Restricted Project
labath added a reviewer for D114722: [LLDB] Fix Python GIL-not-held issues: JDevlieghere.
Mon, Dec 6, 7:37 AM · Restricted Project
labath requested review of D115151: [lldb/qemu] Add emulator-args setting.
Mon, Dec 6, 6:17 AM · Restricted Project
labath committed rG5c4cb323e86a: [lldb/qemu] Add support for pty redirection (authored by labath).
[lldb/qemu] Add support for pty redirection
Mon, Dec 6, 6:06 AM
labath closed D114796: [lldb/qemu] Add support for pty redirection.
Mon, Dec 6, 6:06 AM · Restricted Project
labath added a comment to D115074: [lldb/lua] Suppress warnings about C-linkage in generated wrapper.

I just went ahead and did that in a52af6d3714.

Mon, Dec 6, 6:00 AM · Restricted Project
labath committed rG85578db68aa9: [lldb/lua] Add a file that should have been a part of a52af6d3 (authored by labath).
[lldb/lua] Add a file that should have been a part of a52af6d3
Mon, Dec 6, 5:59 AM
labath committed rGa52af6d3714f: [lldb] Remove extern "C" from lldb-swig-lua interface (authored by labath).
[lldb] Remove extern "C" from lldb-swig-lua interface
Mon, Dec 6, 5:58 AM
labath added a comment to D114911: [lldb] Introduce a FreeBSDKernel plugin for vmcores.

I don't have a problem with incomplete patches. I (and llvm, in general) actually strongly incremental development.

Mon, Dec 6, 5:37 AM
labath accepted D114967: [lldb] [Process/elf-core] Disable for FreeBSD vmcores.
Mon, Dec 6, 5:30 AM · Restricted Project
labath added a comment to D115126: [LLDB] Add unit tests for Editline keyboard shortcuts.

All of the changes seem fine, but it's not clear to me what is the value of the new test. To me, it just seems like a change-detector forcing the two versions of the keybindings to stay in sync. The need for the friend declarations and everything also enforces the feeling that this test is not checking what it should be. Did you actually run into some kind of a problem/bug here that you think this test would have prevented?

Mon, Dec 6, 5:08 AM · Restricted Project
labath accepted D112759: [llvm] [Debuginfo] Add llvm-debuginfod-find tool and end-to-end-tests..

Regarding the output behavior, Elfutils debuginfod-find prints the path of the fetched/cached artifact file to stdout. By default, llvm-debuginfod-find does the same. I've also added a command line flag llvm-debuginfod-find --dump which dumps the contents of the artifact to stdout instead of its file path.
If you can only print the path to the cached file, you have to read the stdout and then use that path as the argument to another program (e.g. cat). This is easy to do in a full shell using xargs, or a subshell or variable, but inconvenient in LLVM Lit scripts which use a sequence of LLVM commands chained by pipes.

Does this output behavior of llvm-debuginfod-find and the --dump flag seem reasonable to you? If it's more clear we could change the name to --print-contents or something else. Thanks!

Mon, Dec 6, 4:55 AM · Restricted Project
labath accepted D115104: [lldb] Fix guessing of windows path style.
Mon, Dec 6, 4:38 AM · Restricted Project
labath added a comment to D113930: [LLDB][NativePDB] Fix function decl creation for class methods.

I am somewhat confused by this deduplication logic. I've read https://reviews.llvm.org/D113930#3136404, so I understand why the duplication is happening, but I don't understand _should_ it be happening. IIUC, SymbolFileNativePDB::ParseTypes tries to force the completion of all types. If it succeeds, then there should be no need to add anything else to that type. In fact, adding methods to an already-complete type sounds like a pretty bad idea.

Mon, Dec 6, 4:36 AM · Restricted Project
labath added a comment to D115073: Modify DataEncoder to be able to encode data in an object owned buffer..

It seems to me that the DataEncoder class is a lot more complicated than what is necessary for our existing use cases. All of them involve creating a new buffer (either empty, or with some pre-existing content), modifying it, and passing it on. A lot of the DataEncoder complexity (IsDynamic, UpdateMemberVariables, ...) stems from the fact that it wants to support writing into unowned buffers, functionality that we don't even use.

Mon, Dec 6, 3:09 AM · Restricted Project
labath added a comment to D115062: [LLDB][Clang] add AccessSpecDecl for methods and fields in RecordType.

I like this feature, but I gotta say that a map feels a bit wasteful as well, given that the last access specifier is only needed for the short duration of time while the class definition is being parsed.

Mon, Dec 6, 2:24 AM · Restricted Project
labath added a comment to D114819: [lldb] Split TestCxxChar8_t.

Looks good. Thanks for fixing this up.

Mon, Dec 6, 1:48 AM · Restricted Project
labath added a reviewer for D114862: Replace StackID's operator "<" with a function returning FrameComparison: jingham.

I don't really know what to make of this, but adding Jim for the thread plan changes.

Mon, Dec 6, 1:47 AM · Restricted Project
labath added a comment to D114861: Don't consider frames with different PCs as a loop.

It would be better if Jason reviewed this, but my first thoughts after seeing this are:

Mon, Dec 6, 1:46 AM · Restricted Project
labath accepted D114877: [lldb] Add missing space in C string format memory read warning.
Mon, Dec 6, 1:08 AM · Restricted Project

Tue, Nov 30

labath accepted D114288: [NFC] Refactor symbol table parsing..
Tue, Nov 30, 7:47 AM · Restricted Project
labath updated the diff for D114796: [lldb/qemu] Add support for pty redirection.

fix typo

Tue, Nov 30, 7:39 AM · Restricted Project
labath added inline comments to D114796: [lldb/qemu] Add support for pty redirection.
Tue, Nov 30, 7:39 AM · Restricted Project
labath requested review of D114796: [lldb/qemu] Add support for pty redirection.
Tue, Nov 30, 6:19 AM · Restricted Project
labath added inline comments to D114791: [lldb] Clarify StructuredDataImpl ownership.
Tue, Nov 30, 6:11 AM · Restricted Project
labath requested review of D114791: [lldb] Clarify StructuredDataImpl ownership.
Tue, Nov 30, 5:37 AM · Restricted Project
labath committed rG1408684957bb: [lldb] Introduce PlatformQemuUser (authored by labath).
[lldb] Introduce PlatformQemuUser
Tue, Nov 30, 5:22 AM
labath committed rGa6e673643c44: [lldb] Inline Platform::LoadCachedExecutable into its (single) caller (authored by labath).
[lldb] Inline Platform::LoadCachedExecutable into its (single) caller
Tue, Nov 30, 5:22 AM
labath closed D114509: [lldb] Introduce PlatformQemuUser.
Tue, Nov 30, 5:22 AM · Restricted Project
labath added a comment to D114675: [lldb] [Target] Support fallback to file address in ReadMemory().

Well, that's what happens with the core file I have and what you're asking for is really above my paygrade.

Tue, Nov 30, 2:17 AM
labath committed rG9a14adeae000: [lldb] Remove 'extern "C"' from the lldb-swig-python interface (authored by labath).
[lldb] Remove 'extern "C"' from the lldb-swig-python interface
Tue, Nov 30, 2:06 AM
labath closed D114369: [lldb] Remove 'extern "C"' from the lldb-swig-python interface.
Tue, Nov 30, 2:06 AM · Restricted Project
labath added a comment to D114746: [lldb] Generalize ParsedDWARFTypeAttributes.

I don't think we should be putting this into the DWARFAttribute file. It is substantially higher-level than the rest of the file, and the DWARFAttribute file is destined to be merged with the llvm implementation at some point. Is there a reason for not putting it into DWARFASTParser (like the other patch)?

Tue, Nov 30, 2:03 AM · Restricted Project
labath added inline comments to D112758: [llvm] [Debuginfo] Debuginfod client library..
Tue, Nov 30, 1:46 AM · Restricted Project
labath added a comment to D114467: [LLDB][NativePDB] Allow find functions by full names.

What is the reason for these differences? The test in question appears to be fairly hermetic (in-tree compiler and linker), so it sounds like something is going wrong inside lldb...

Tue, Nov 30, 1:42 AM · Restricted Project

Mon, Nov 29

labath added inline comments to D112759: [llvm] [Debuginfo] Add llvm-debuginfod-find tool and end-to-end-tests..
Mon, Nov 29, 1:20 AM · Restricted Project
labath added inline comments to D112758: [llvm] [Debuginfo] Debuginfod client library..
Mon, Nov 29, 12:48 AM · Restricted Project
labath added a comment to D114675: [lldb] [Target] Support fallback to file address in ReadMemory().

I don't think this is the right solution to this problem.

If it's not, then the expression parser should probably be changed for consistency.

I don't think we'd want to do that (*), but we could definitely make sure that the memory read command produces a meaningful error message instead of a bunch of zeroes. But lets wait to see what Jim thinks about that.

Mon, Nov 29, 12:47 AM
labath requested changes to D114675: [lldb] [Target] Support fallback to file address in ReadMemory().

I don't think this is the right solution to this problem.

Mon, Nov 29, 12:26 AM

Fri, Nov 26

labath added a comment to D114008: [formatters] Add a deque formatter for libstdcpp and fix the libcxx one.

You need to construct issue appropriately nested ValueCheck objects. Something like this:

children.insert(0,ValueCheck(children=[ValueCheck(value=i), ValueCheck(value=i+1), ValueCheck(i + 2)])

If that doesn't work then that's probably a bug in the ValueCheck matching implementation, as it was definitely written with that mind.

Fri, Nov 26, 6:48 AM · Restricted Project
labath requested changes to D86205: [lldb] Move TestDarwinNSLogOutput to PExpectTest and allow checking for missing substrs in PExpectTest.

I still like the idea from the inline comment, but mainly clicking request changes to get this off my queue.

Fri, Nov 26, 3:09 AM
labath requested changes to D89335: [lldb] Generic support for caching register set reads/writes.

I guess this slipped off my radar.... What's the state of this? Are you still interested in the patch? If yes, we can try again after rebasing.

Fri, Nov 26, 3:00 AM
labath accepted D90876: [lldb] [test] Improve comment on expr-after-step-after-crash tests.

Getting this off my queue. I think the change is fine, but I can also live with the original comment.

Fri, Nov 26, 2:56 AM
labath added a comment to D114509: [lldb] Introduce PlatformQemuUser.

Thanks for the review. I'm going to wait until the US folks get back before submitting this.

Fri, Nov 26, 2:08 AM · Restricted Project

Thu, Nov 25

labath committed rG165545c7a431: [lldb/gdb-remote] Ignore spurious ACK packets (authored by labath).
[lldb/gdb-remote] Ignore spurious ACK packets
Thu, Nov 25, 3:45 AM
labath committed rGa6fedbf20c8f: [lldb/gdb-remote] Remove initial pipe-draining workaround (authored by labath).
[lldb/gdb-remote] Remove initial pipe-draining workaround
Thu, Nov 25, 3:45 AM
labath closed D114520: [lldb/gdb-remote] Ignore spurious ACK packets.
Thu, Nov 25, 3:44 AM · Restricted Project
labath closed D114529: [lldb/gdb-remote] Remove initial pipe-draining workaround.
Thu, Nov 25, 3:44 AM · Restricted Project
labath added a comment to D114509: [lldb] Introduce PlatformQemuUser.

That said, I wonder if we should use some formatter (e.g. black) for Python files. While I can't really say I like black's coding style, it's quite popular and using some formatter would be consistent with using clang-format for C++ code.

Thu, Nov 25, 3:41 AM · Restricted Project
labath updated the diff for D114509: [lldb] Introduce PlatformQemuUser.

feedback David

Thu, Nov 25, 3:31 AM · Restricted Project
labath added a comment to D114509: [lldb] Introduce PlatformQemuUser.

qemu also does not support macOS

Do you mean qemu-user here?

Yes, definitely.

Thu, Nov 25, 3:30 AM · Restricted Project
labath added a comment to D114008: [formatters] Add a deque formatter for libstdcpp and fix the libcxx one.

As we discussed offline, I add the print tests, since SBValue.GetValue() doesn't work for complex structures

Thu, Nov 25, 2:35 AM · Restricted Project
labath added a comment to D114288: [NFC] Refactor symbol table parsing..

Do all .dwo files claim the main executable that references them is their owning module?

The short answer is "yes".

Thu, Nov 25, 12:58 AM · Restricted Project
labath added a comment to D114554: Fix TestFileHandle.py. Remove redundant skipIfReproducer annotation.

I've fixed this in rGc0e3bb4d4ba306.

Thu, Nov 25, 12:04 AM · Restricted Project

Wed, Nov 24

labath committed rGc0e3bb4d4ba3: [lldb] Fix TestFileHandle.py (authored by labath).
[lldb] Fix TestFileHandle.py
Wed, Nov 24, 11:58 PM
labath committed rG96beb30fbbce: [lldb] Move GetSupportedArchitectureAtIndex to PlatformDarwin (authored by labath).
[lldb] Move GetSupportedArchitectureAtIndex to PlatformDarwin
Wed, Nov 24, 6:48 AM
labath added a comment to D114529: [lldb/gdb-remote] Remove initial pipe-draining workaround.

Ed, I don't suppose you by any chance remember more details about that patch? I couldn't find a way to reproduce this with a modern-day qemu, but I also couldn't find any evidence that this ever was an issue, so it is possible (though not very likely, I hope) that I am missing something...

Wed, Nov 24, 5:25 AM · Restricted Project
labath requested review of D114529: [lldb/gdb-remote] Remove initial pipe-draining workaround.
Wed, Nov 24, 5:22 AM · Restricted Project
labath added inline comments to D114520: [lldb/gdb-remote] Ignore spurious ACK packets.
Wed, Nov 24, 4:36 AM · Restricted Project
labath updated the diff for D114509: [lldb] Introduce PlatformQemuUser.

Address Michał's feedback.

Wed, Nov 24, 2:54 AM · Restricted Project
labath added inline comments to D114509: [lldb] Introduce PlatformQemuUser.
Wed, Nov 24, 2:53 AM · Restricted Project
labath requested review of D114520: [lldb/gdb-remote] Ignore spurious ACK packets.
Wed, Nov 24, 2:47 AM · Restricted Project
labath committed rG6f82264dbb02: [lldb/gdb-remote] Remove more non-stop mode remnants (authored by labath).
[lldb/gdb-remote] Remove more non-stop mode remnants
Wed, Nov 24, 1:02 AM
labath requested review of D114509: [lldb] Introduce PlatformQemuUser.
Wed, Nov 24, 12:14 AM · Restricted Project

Tue, Nov 23

labath added a comment to D114450: Improve optional formatter.

thanks

Tue, Nov 23, 11:42 PM · Restricted Project
labath accepted D114465: [lldb] Move create_relative_symlink function up in CMake hierarchy.
Tue, Nov 23, 12:39 PM · Restricted Project
labath accepted D114467: [LLDB][NativePDB] Allow find functions by full names.
Tue, Nov 23, 12:37 PM · Restricted Project
labath added inline comments to D114458: Make some libstd++ formatters safer.
Tue, Nov 23, 10:41 AM · Restricted Project
labath added a comment to D114450: Improve optional formatter.

That said, I'm more worried about the strange interactions between libc++ and libstdc++ formatters I reported.

yes, I'm trying to install libc++ on my machine now

Tue, Nov 23, 10:32 AM · Restricted Project
labath accepted D114450: Improve optional formatter.

I didn't realize that _M_engaged has type bool. That means the max value this could have is 255, which isn't nearly as bad as 0xffffffff. Nonetheless, this is a good change.

Tue, Nov 23, 9:54 AM · Restricted Project
labath added a comment to D114288: [NFC] Refactor symbol table parsing..

I don't believe this solution is correct.

Tue, Nov 23, 9:44 AM · Restricted Project
labath added a comment to D114403: [formatters] Add a formatter for libstdc++ optional.

I'm seeing some very funky behavior after this patch. My machine has both libc++ and libstdc++, so both versions of this test run normally, and then libstdc++ fails. If I run just the libstdc++ version then it passes just fine. If I change the test order so that the libstdc++ version is run first, then the *libc++* version fails instead...

Tue, Nov 23, 2:24 AM · Restricted Project

Mon, Nov 22

labath added inline comments to D112758: [llvm] [Debuginfo] Debuginfod client library..
Mon, Nov 22, 11:44 PM · Restricted Project
labath updated the diff for D114369: [lldb] Remove 'extern "C"' from the lldb-swig-python interface.

Revert the PyInit_lldb changes. They fail on windows due to declspec(dllexport)
mistmatches, and this function is kinda special anyway.

Mon, Nov 22, 8:55 AM · Restricted Project
labath requested review of D114369: [lldb] Remove 'extern "C"' from the lldb-swig-python interface.
Mon, Nov 22, 7:45 AM · Restricted Project
labath committed rG7f09ab08de5a: [lldb] Fix [some] leaks in python bindings (authored by labath).
[lldb] Fix [some] leaks in python bindings
Mon, Nov 22, 6:27 AM
labath committed rG7c8ae65f2c3d: [lldb/test] Make it possible to run the mock gdb server on a single thread (authored by labath).
[lldb/test] Make it possible to run the mock gdb server on a single thread
Mon, Nov 22, 6:27 AM
labath closed D114259: [lldb] Fix [some] leaks in python bindings.
Mon, Nov 22, 6:27 AM · Restricted Project