Page MenuHomePhabricator

labath (Pavel Labath)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 4 2013, 6:02 AM (381 w, 21 h)

Recent Activity

Thu, Sep 17

labath added reviewers for D87765: [llvm][lldb] Add optimal ThreadPool concurrency: MaskRay, aganea.

This sounds like a good idea, but I'd like to get another set of eyes on the llvm part.

Thu, Sep 17, 1:22 AM · Restricted Project, Restricted Project

Wed, Sep 16

labath accepted D87694: [lldb] Don't send invalid region addresses to lldb server.
Wed, Sep 16, 5:57 AM · Restricted Project
labath added inline comments to D87694: [lldb] Don't send invalid region addresses to lldb server.
Wed, Sep 16, 3:17 AM · Restricted Project
labath added inline comments to D87694: [lldb] Don't send invalid region addresses to lldb server.
Wed, Sep 16, 1:25 AM · Restricted Project
labath accepted D87675: [lldb/DWARF] Refactor to prefer emplace_back() vs. push_back().

Ok, this is something we can talk about. dwarf parsing is a big bottle neck, so spending some time to optimize that (and even sacrifice some readability/mantainability/etc. if needed) is worthwhile.

Wed, Sep 16, 1:18 AM · Restricted Project

Tue, Sep 15

labath added a comment to D87172: Check if debug line sequences are starting after the first code segment.

This is fixing the same issue that I tried to fix with D84402, but then failed to get back to it when the discussion about the best approach started getting long. While I do have some reservations about this approach, it is definitive improvement than the status quo, so I won't start bikeshedding the design (but do see the inline comments)..

Tue, Sep 15, 6:34 AM · Restricted Project
labath added a comment to D87675: [lldb/DWARF] Refactor to prefer emplace_back() vs. push_back().

Use std::pair rather than an ad-hoc two-member trivial structures. Since std::pair provides the default constructor to initialize both members, no extra C++ glue is required for emplace_back()'ing the pairs into containers.

Tue, Sep 15, 5:37 AM · Restricted Project
labath added a comment to D87590: Backport D79219, D85820, D86134 to 10.0 branch.

Also note that the changes are not likely to be backported to 11 even: https://reviews.llvm.org/rG31e5f7120bdd2f76337686d9d169b1c00e6ee69c#942622.

Tue, Sep 15, 4:34 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
labath committed rGaf3789a18811: [lldb] Improve qemu interop for aarch64 (authored by labath).
[lldb] Improve qemu interop for aarch64
Tue, Sep 15, 4:32 AM
labath committed rG0a2213c6eb24: [lldb/cmake] Fix testing support library dependencies (authored by labath).
[lldb/cmake] Fix testing support library dependencies
Tue, Sep 15, 4:32 AM
labath added a reviewer for D87590: Backport D79219, D85820, D86134 to 10.0 branch: tstellar.

@tstellar, who is (was) the release manager for 10.0, can make a definitive call, but our usual modus operandi is to have one point release for each major version. I don't know if this is a sufficiently major issue to break from that (and it definitely is risky).

Tue, Sep 15, 4:03 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
labath added a comment to D87675: [lldb/DWARF] Refactor to prefer emplace_back() vs. push_back().

Using emplacement instead of construction is a good idea, where it makes sense. However, I am not convinced that adding constructors to otherwise-trivial structures is worth it. These structures are usually trivially copyable and/or movable and so an emplacement does not really save much.

Tue, Sep 15, 3:58 AM · Restricted Project
Herald added a reviewer for D87333: [lldb/ipv6] Support running lldb tests in an ipv6-only environment.: JDevlieghere.

(Sorry about the delay.) Given the current requirements, I think this patch is fine (excellent even).

Tue, Sep 15, 2:30 AM · Restricted Project
labath accepted D86996: [lldb] Add -l/--language option to script command.

This looks fine (sorry about the delay, I've been OOO).

Tue, Sep 15, 1:53 AM · Restricted Project, Restricted Project

Fri, Sep 4

labath added a comment to D86996: [lldb] Add -l/--language option to script command.

I'm not married to the current way we process commands, but I do value their consistency (both between different commands, and between a command and their documentation). This would make script behave differently than expr even though they have identical modes of operation (raw input, special handling of empty argument, etc.), and differently from its documentation. The help script command contains this:

Invoke the script interpreter with provided code and display any results.  Start the interactive
interpreter if no code is supplied.  Expects 'raw' input (see 'help raw-input'.)

I'm not sure how this is patch coded, but probably after the argument are added, this additional snippet will appear:

Important Note: Because this command takes 'raw' input, if you use any command options you must use
 ' -- ' between the end of the command options and the beginning of the raw input.

help raw-input says this:

<raw-input> -- Free-form text passed to a command without prior interpretation, allowing spaces
               without requiring quotes.  To pass arguments and free form text put two dashes ' -- '
               between the last argument and any raw input.

If we do want to change that, we should at least make sure all of these things reflect that. And yeah, that should be a separate patch (maybe a small RFC even).

Fri, Sep 4, 5:45 AM · Restricted Project, Restricted Project

Thu, Sep 3

labath added a comment to D86670: [intel-pt] Add a basic implementation of the dump command.

I think it would be appropriate to discuss the design of this feature on lldb-dev before going over the individual patches. One of the fundamental aspects of this patchset that I think should not be overlooked is that it essentially adds a new level of structure ( a "Target Group") lldb. One of the questions I'd have is whether this concept shouldn't be formalized somewhere instead of it existing just virtually as the group of threads that happen to share the same trace object.

The idea was to iterate on the design of the trace feature using these patches and have the discussion here. The idea is one trace file can create N individual targets. Each target can be independent and the threads contained in each belong to each target. "trace dump" when no args are supplied it could maybe only dump the currently selected target to start with? I am not sure what the right semantics are, but that shouldn't stop us with proceeding with these patches IMHO. It will be easy to modify as we evolve if we do want to have target groups, but I don't think it is required for these patches. The target group is a much higher level design that can affect many things and could be used in this case, but isn't required. If a trace plug-in needs to iterate over all of the targets this can be achieved by using the debugger to grab all targets and iterate over them, so no real group is required.

So it would simplify things right now if we say that "trace dump" dumps the trace data for the currently selected target right now. That will map well with the stepping commands that will soon be added to allow forward and reverse traversal of the trace data.

Thu, Sep 3, 6:39 AM · Restricted Project
labath updated subscribers of D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..

I'm pretty sure the json changes should get a separate review. The json owner (let's call him @sammccall) might have a different idea on how to do this thing...

Thu, Sep 3, 5:58 AM · Restricted Project, Restricted Project
labath added a reviewer for D82863: [LLDB] Add support to resize SVE registers at run-time: jasonmolenda.

I'd like to also pull in Jason for this, since this is really the trickiest part of the whole patchset.

Thu, Sep 3, 5:45 AM
labath added a comment to D82857: [LLDB] Add per-thread register infos shared pointer in gdb-remote.

There are things I'd want to change here, but I think this is patch is fundamentally ok. Before going into that, I think we should first sort out the dependant patch.

Thu, Sep 3, 5:02 AM
labath accepted D86792: [lldb] Improve test failure reporting for expect().
Thu, Sep 3, 4:42 AM · Restricted Project
labath accepted D87036: [lldb] Make symbol list output from `image dump symtab` not depend on internal ordering of DenseMap.

Looks good.

Thu, Sep 3, 1:22 AM · Restricted Project
labath requested changes to D86996: [lldb] Add -l/--language option to script command.

I agree that silly, but maybe the same fix should then be applied to the expr command (and any other command with similar behavior).

Sure, we can use the same trick in other places that use OptionsWithRaw. I'll do that in a separate patch.

Actually, after discussion this with @teemperor offline, for the expo command we can't use that trick because a valid option might also be a valid expression. expr --flag would parse correctly if flag was an option, but it might also be an expression that decrements --flag.

Thu, Sep 3, 12:21 AM · Restricted Project, Restricted Project
labath added inline comments to D86821: [lldb] Make the majority of the lit configuration values optional for the API tests.
Thu, Sep 3, 12:06 AM · Restricted Project

Wed, Sep 2

labath accepted D82855: [LLDB] Send SVE vg register in custom expedited registerset .
Wed, Sep 2, 8:39 AM
labath resigned from D86866: Add Option to sphinx-build for custom conf file location.

I don't know anything about Sphinx. It might be useful to mention in the patch description the motivation for this change.

Wed, Sep 2, 8:28 AM · Restricted Project
labath accepted D86872: [LLDB] Move targetHasSVE function to lldbtest.py.
Wed, Sep 2, 8:25 AM · Restricted Project
labath added a comment to D86792: [lldb] Improve test failure reporting for expect().

Combined two functions into one single run command
and check message function.

With some odd kwargs, but I think it helps to be able
to put the expected lines on the end.

Wed, Sep 2, 8:24 AM · Restricted Project
labath added a comment to D86996: [lldb] Add -l/--language option to script command.

because I thought it would be silly to require you to type script --language lua -- to launch the interactive Lua interpreter.

Wed, Sep 2, 8:15 AM · Restricted Project, Restricted Project
labath accepted D86962: [LLDB] Move NativeRegisterContextLinux/RegisterContextPOSIX*_arm to RegisterInfoAndSetInterface.

This is great, thanks for doing this. Just a couple of minor comments inline.

Wed, Sep 2, 8:00 AM · Restricted Project, Restricted Project
labath added inline comments to D86821: [lldb] Make the majority of the lit configuration values optional for the API tests.
Wed, Sep 2, 7:46 AM · Restricted Project
labath added a reviewer for D86825: [lldb/Gui] zero-initialize children_stop_id: clayborg.
Wed, Sep 2, 7:38 AM · Restricted Project
labath added a comment to D86817: [lldb] Get rid of LLDB_LIB_DIR and LLDB_IMPLIB_DIR in dotest.

Yay

Wed, Sep 2, 7:37 AM · Restricted Project
labath accepted D86818: [lldb/Host] Add missing proc states.
Wed, Sep 2, 7:35 AM · Restricted Project
labath accepted D86667: [lldb/Target] Add custom interpreter option to `platform shell`.

Thanks, this looks good to me now.

Wed, Sep 2, 7:31 AM · Restricted Project
labath added a comment to D86792: [lldb] Improve test failure reporting for expect().

I like this. And a big thank-you for writing the tests. We don't usually bother to write self-tests for the test infrastructure, but we definitely should be doing that. The place is slightly weird, but I think it will do for now -- it's easy to move this around if we find a better place for it.

Wed, Sep 2, 7:17 AM · Restricted Project

Fri, Aug 28

labath added inline comments to D86667: [lldb/Target] Add custom interpreter option to `platform shell`.
Fri, Aug 28, 6:46 AM · Restricted Project
labath added a comment to D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl.

I'm going to respond to the rest of your (very insightful) comment later. So far, I'm just responding to this:

This isn't exactly layout related, but there is the question of covariant methods. If a method is covariant, then its return type must be complete.

We already import all the base classes of a derived class (at least in full import). But, we do not import all the derived classes during the import of the base. Is this what you do in LLDB to support covariant return types?

Fri, Aug 28, 6:04 AM · Restricted Project, Restricted Project
labath committed rG9b50546b0b40: [lldb/Utility] Polish the Scalar class (authored by labath).
[lldb/Utility] Polish the Scalar class
Fri, Aug 28, 2:58 AM
labath committed rG1f9595ede48d: [lldb] Reduce intentation in SymbolFileDWARF::ParseVariableDIE (authored by labath).
[lldb] Reduce intentation in SymbolFileDWARF::ParseVariableDIE
Fri, Aug 28, 2:58 AM
labath added a comment to D86670: [intel-pt] Add a basic implementation of the dump command.

I think it would be appropriate to discuss the design of this feature on lldb-dev before going over the individual patches. One of the fundamental aspects of this patchset that I think should not be overlooked is that it essentially adds a new level of structure ( a "Target Group") lldb. One of the questions I'd have is whether this concept shouldn't be formalized somewhere instead of it existing just virtually as the group of threads that happen to share the same trace object.

Fri, Aug 28, 2:42 AM · Restricted Project
labath added a comment to D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..
  • I'm still using StructuredData for the parsing. There's a chance that once we release this feature users will want support for other formats besides JSON, so for now I prefer to ask for JSON input but parse in a format-agnostic way. If eventually no one needs any other format, we can switch to JSON-only parsing.
Fri, Aug 28, 2:25 AM · Restricted Project, Restricted Project
labath added a comment to D86652: [lldb] Fix ProcessEventData::DoOnRemoval to support multiple listeners.

This is going to derail this patch somewhat (sorry), but since this is an important long-standing open issue, I think we should discuss it in more detail.

Fri, Aug 28, 2:04 AM · Restricted Project
labath added inline comments to D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl.
Fri, Aug 28, 1:28 AM · Restricted Project, Restricted Project
labath accepted D86745: [lldb/test] Use @skipIfWindows for PExpectTest.

This looks like it could work, though only barely. @skipIfWindows checks for target system, but we would really need this to check the host OS (for remote testing, although I don't know if anyone does that on windows these days). However, in conjuction with @skipIfRemote it should do the right thing. In any case, this is small enough a change to try it out. Just be sure to check the windows bot afterwards.

Fri, Aug 28, 12:43 AM · Restricted Project
labath added a comment to D86667: [lldb/Target] Add custom interpreter option to `platform shell`.
In D86667#2243781, @mib wrote:

Do people really call command-line shells interpreters? I would have thought --shell would be a better name. lldb already has its own command interpreter which is orthogonal to the shells, so it seems confusing to use the same term for both.

I think platform shell --shell sounds/looks repetitive so I opted for -i|--interpreter instead, as in Command-line Interpreter.

Fri, Aug 28, 12:26 AM · Restricted Project

Thu, Aug 27

labath committed rGdd635062d867: [lldb/cmake] Fix linking of lldbSymbolHelpers for 9cb222e7 (authored by labath).
[lldb/cmake] Fix linking of lldbSymbolHelpers for 9cb222e7
Thu, Aug 27, 7:40 AM
labath committed rG5b2b75456560: [lldb/cmake] Fix linking of lldbUtilityHelpers for 9cb222e74 (authored by labath).
[lldb/cmake] Fix linking of lldbUtilityHelpers for 9cb222e74
Thu, Aug 27, 7:08 AM
labath committed rG0de146337391: [lldb] Fix Type::GetByteSize for pointer types (authored by labath).
[lldb] Fix Type::GetByteSize for pointer types
Thu, Aug 27, 6:41 AM
labath closed D86436: [lldb] Fix Type::GetByteSize for pointer types.
Thu, Aug 27, 6:41 AM · Restricted Project
labath committed rG9cb222e749e8: [cmake] Make gtest include directories a part of the library interface (authored by labath).
[cmake] Make gtest include directories a part of the library interface
Thu, Aug 27, 6:36 AM
labath closed D86616: [cmake] Make gtest include directories a part of the library interface.
Thu, Aug 27, 6:36 AM · Restricted Project, Restricted Project, Restricted Project
labath added a comment to D86616: [cmake] Make gtest include directories a part of the library interface.

LGTM, but I'm not an owner for any of the projects touched by this change.

Thu, Aug 27, 6:35 AM · Restricted Project, Restricted Project, Restricted Project
labath committed rG9f5927e42bf4: [lldb/DWARF] Fix handling of variables with both location and const_value… (authored by labath).
[lldb/DWARF] Fix handling of variables with both location and const_value…
Thu, Aug 27, 6:06 AM
labath committed rG219ccdfddecb: [lldb/Utility] Use APSInt in the Scalar class (authored by labath).
[lldb/Utility] Use APSInt in the Scalar class
Thu, Aug 27, 6:06 AM
labath closed D86615: [lldb/DWARF] Fix handling of variables with both location and const_value attributes.
Thu, Aug 27, 6:06 AM · Restricted Project
labath added inline comments to D86615: [lldb/DWARF] Fix handling of variables with both location and const_value attributes.
Thu, Aug 27, 6:02 AM · Restricted Project
labath accepted D86690: [lldb] Fix gcc 5.4.0 compile error.

Looks good.

Thu, Aug 27, 2:06 AM · Restricted Project
labath added a comment to D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..

If the format is going to be json, why not use llvm::json from llvm/Support/JSON.h? That's what we been migrating most of the existing stuff to already, so going using that for new code makes perfect sense.

Thu, Aug 27, 1:10 AM · Restricted Project, Restricted Project
labath added a comment to D86667: [lldb/Target] Add custom interpreter option to `platform shell`.

I am wondering what should the platform classes which do not implement this feature do (although most of them could implemented I don't think you need to implement all of them -- PlatformGdbRemote in particular can be a bit tricky). It seems to me like it would be better for them to bail out if a non-standard shell is requested instead of silently executing a command the user did not intend.

Thu, Aug 27, 1:03 AM · Restricted Project
labath added inline comments to D86660: Modifying ImportDeclContext(...) to ensure that we also handle the case when the FieldDecl is an ArrayType whose ElementType is a RecordDecl.
Thu, Aug 27, 12:33 AM · Restricted Project, Restricted Project

Wed, Aug 26

labath added a comment to D86436: [lldb] Fix Type::GetByteSize for pointer types.

@labath something I noticed when finding this (and related bugs) is that frame var carries a decent diagnostic

(int *) l_125 = <empty constant data>

and the expression parser returns something not particularly useful:

(lldb) p l_125 
error: <lldb wrapper prefix>:43:31: no member named 'l_125' in namespace '$__lldb_local_vars'
    using $__lldb_local_vars::l_125;
          ~~~~~~~~~~~~~~~~~~~~^
error: <user expression 0>:1:1: use of undeclared identifier 'l_125'
l_125

From my testing infrastructure/fuzzing perspective the two are indistinguishable, as the script I've written chokes on both, but it would be better from an ergonomics point of view if p would return something meaningful, if possible (even if there's a bug in lldb). Do you think it's worth filing a PR? (also, cc: @teemperor for ideas as he spent a fair amount of time working on the expression parser)

Wed, Aug 26, 6:08 AM · Restricted Project
labath requested review of D86616: [cmake] Make gtest include directories a part of the library interface.
Wed, Aug 26, 5:39 AM · Restricted Project, Restricted Project, Restricted Project
labath requested review of D86615: [lldb/DWARF] Fix handling of variables with both location and const_value attributes.
Wed, Aug 26, 5:33 AM · Restricted Project
labath committed rG82982304d709: [lldb/DWARF] More DW_AT_const_value fixes (authored by labath).
[lldb/DWARF] More DW_AT_const_value fixes
Wed, Aug 26, 4:24 AM
labath closed D86348: [lldb/DWARF] More DW_AT_const_value fixes.
Wed, Aug 26, 4:24 AM · Restricted Project
labath added a comment to D80659: [lldb-vscode] Redirect stderr and stdout to DAPs console message.

Sorry, I missed this patch. Modulo the inline comments and the missing windows support and tests, I think this patch is ok-ish. However, I am still not convinced of its usefulness. Why would we be doing something (particularly a thing which will be hard to do in a cross-platform manner, and will very likely border on, or downright cross into, undefined behavior territory), if we get that from vscode for free?

Wed, Aug 26, 4:16 AM · Restricted Project
labath accepted D86375: Load correct module for linux and android when duplicates exist in minidump..

Thanks for the clarification. The new version looks great.

Wed, Aug 26, 2:46 AM · Restricted Project
labath added a comment to D86603: [lldb] Correct wording of EXP_MSG.

Though these extra messages are often superfluous in general, I don't think that's the case for this particular message. This macro is only used in the expect and match checks. These are the most common way of asserting something right now, so I think it's reasonable to spend some effort to make the errors it produces clear and actionable. The current state is pretty far from ideal.

Wed, Aug 26, 2:33 AM · Restricted Project
labath added a comment to D82865: [LLDB] Add GetByteOffset to SBValue interface for reading register offset.

Sounds good. FWIW, I wouldn't be opposed to some CLI command which would dump some low-level details of the register context, similar to how we have the "target modules dump" command tree for Module objects...

Wed, Aug 26, 1:59 AM
labath added a comment to D82855: [LLDB] Send SVE vg register in custom expedited registerset .

This looks good, though it would be nice to add a test for it. It looks like TestGdbRemoteExpeditedRegisters.py has a lot of functionality that could be reused for this purpose.

Wed, Aug 26, 1:53 AM
labath accepted D82853: [LLDB] Support custom expedited register set in gdb-remote.

I like this a lot. LGTM with some small fixes inline.

Wed, Aug 26, 1:44 AM
labath accepted D86406: Speedup llvm-dwarfdump 3.9x.
Wed, Aug 26, 1:25 AM · Restricted Project

Mon, Aug 24

labath added inline comments to D86436: [lldb] Fix Type::GetByteSize for pointer types.
Mon, Aug 24, 10:15 AM · Restricted Project

Aug 24 2020

labath accepted D85284: [lldb] Remote disk file/directory completion for platform commands.

Sorry about the delay. This looks good to me now.

Aug 24 2020, 2:51 AM · Restricted Project
labath committed rG3d1b0000f9da: [lld] s/dyn_cast/isa in InputSection.cpp (authored by labath).
[lld] s/dyn_cast/isa in InputSection.cpp
Aug 24 2020, 2:46 AM
labath committed rG0e301fd02386: [lldb/Utility] Remove some Scalar type accessors (authored by labath).
[lldb/Utility] Remove some Scalar type accessors
Aug 24 2020, 2:46 AM
labath added a comment to D86348: [lldb/DWARF] More DW_AT_const_value fixes.

This looks nice! I'm somewhat suspicious that the new test doesn't specifically test the union case of the old test, but I'm assuming that would still work and your simpler tests covers the same code?

Aug 24 2020, 2:40 AM · Restricted Project
labath requested review of D86436: [lldb] Fix Type::GetByteSize for pointer types.
Aug 24 2020, 2:36 AM · Restricted Project
labath requested changes to D86375: Load correct module for linux and android when duplicates exist in minidump..

If I correctly understand what this is doing, then I don't think it's a good idea. The base of an (elf) shared library does not have to be mapped executable. These are the mappings I get for a trivial hello world program (no mmapping of libraries or anything) on my linux machine:

00400000-00401000 r--p 00000000 fd:01 2838574                            /tmp/l/a.out
00401000-00402000 r-xp 00001000 fd:01 2838574                            /tmp/l/a.out
00402000-00403000 r--p 00002000 fd:01 2838574                            /tmp/l/a.out
00403000-00404000 r--p 00002000 fd:01 2838574                            /tmp/l/a.out
00404000-00405000 rw-p 00003000 fd:01 2838574                            /tmp/l/a.out
020fb000-0211c000 rw-p 00000000 00:00 0                                  [heap]
7fe4f5d87000-7fe4f5da9000 r--p 00000000 fd:01 2738932                    /lib64/libc-2.31.so
7fe4f5da9000-7fe4f5ef3000 r-xp 00022000 fd:01 2738932                    /lib64/libc-2.31.so
7fe4f5ef3000-7fe4f5f3d000 r--p 0016c000 fd:01 2738932                    /lib64/libc-2.31.so
7fe4f5f3d000-7fe4f5f41000 r--p 001b5000 fd:01 2738932                    /lib64/libc-2.31.so
7fe4f5f41000-7fe4f5f43000 rw-p 001b9000 fd:01 2738932                    /lib64/libc-2.31.so
...

Here, the correct base of a.out is 0x00400000 and the libc base is 0x7fe4f5d87000. But this patch would make them be detected as 0x00401000 and 0x7fe4f5da9000, respectively.

Aug 24 2020, 1:46 AM · Restricted Project
labath added a comment to D86416: [lldb] -stdlib=libc++ for linking with lldb lib also if LLVM_ENABLE_LIBCXX.

I actually think tests relying on this should be converted to c++ unit tests. The logic to compile and link against liblldb takes up a considerable chunk of our test buildsystem. Why recreate that logic, when cmake knows how to do that already? TestPluginCommands might be more involved, but converting api/command-return-object should be trivial using the api unit test framework we already have (unittests/API)...

Aug 24 2020, 1:07 AM · Restricted Project
labath accepted D86261: Add hashing of the .text section to ProcessMinidump..

thanks

Aug 24 2020, 12:35 AM · Restricted Project

Aug 21 2020

labath added inline comments to D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value.
Aug 21 2020, 9:49 AM · Restricted Project
labath updated the diff for D86348: [lldb/DWARF] More DW_AT_const_value fixes.

Actually, keep the old const_value test, but give it a bitfield-specific name. Testing bitfields+const_values is still interesting.

Aug 21 2020, 6:27 AM · Restricted Project
labath requested review of D86348: [lldb/DWARF] More DW_AT_const_value fixes.
Aug 21 2020, 6:22 AM · Restricted Project
labath accepted D86261: Add hashing of the .text section to ProcessMinidump..

Hopefully this should be good to go, let me know if anyone has any issues.

Aug 21 2020, 1:43 AM · Restricted Project
labath added a comment to D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value.

LGTM, modulo the defensive check. Incidentally, I was just looking at a DW_AT_const_value bug, but I think it's a different issue than this one.

Aug 21 2020, 1:32 AM · Restricted Project

Aug 20 2020

labath committed rG8a8a2dd3165e: [lldb/Utility] Simplify Scalar handling of float types (authored by labath).
[lldb/Utility] Simplify Scalar handling of float types
Aug 20 2020, 7:26 AM
labath closed D86220: [lldb/Utility] Simplify Scalar handling of float types.
Aug 20 2020, 7:26 AM · Restricted Project
labath added inline comments to D86220: [lldb/Utility] Simplify Scalar handling of float types.
Aug 20 2020, 7:18 AM · Restricted Project
labath committed rG9109311356cc: [lldb] Forcefully complete a type when adding typedefs (authored by labath).
[lldb] Forcefully complete a type when adding typedefs
Aug 20 2020, 6:24 AM
labath closed D86216: [lldb] Forcefully complete a type when adding typedefs.
Aug 20 2020, 6:24 AM · Restricted Project
labath accepted D86090: Make DWARFExpression::GetLocationExpression public.

If you start to need to make more changes for your project, we'll need to have a bigger discussion, but right now, this seems ok.

Aug 20 2020, 6:11 AM · Restricted Project
labath updated subscribers of D86261: Add hashing of the .text section to ProcessMinidump..

Also, +@markmentovai, in case he has any thoughts on this.

Aug 20 2020, 5:57 AM · Restricted Project
labath added a comment to D86261: Add hashing of the .text section to ProcessMinidump..

This has been a feature we've been missing for a while. Thanks for implementing it.

Aug 20 2020, 5:57 AM · Restricted Project

Aug 19 2020

labath requested review of D86220: [lldb/Utility] Simplify Scalar handling of float types.
Aug 19 2020, 8:26 AM · Restricted Project
labath added a comment to D85284: [lldb] Remote disk file/directory completion for platform commands.

I looked at the gdb-remote docs, and I could find precedents for both returning error (or OK) for "empty" responses, and prefixing the responses with a random character. So I think adding the M character is fine too.

Aug 19 2020, 7:43 AM · Restricted Project
labath requested review of D86216: [lldb] Forcefully complete a type when adding typedefs.
Aug 19 2020, 6:59 AM · Restricted Project
labath committed rG9cc2f13deeb3: [lldb] Clean up DW_AT_declaration-with-children.s test (authored by labath).
[lldb] Clean up DW_AT_declaration-with-children.s test
Aug 19 2020, 6:02 AM
labath committed rGd7363397c669: [lldb] Add typedefs to the DeclContext they are created in (authored by labath).
[lldb] Add typedefs to the DeclContext they are created in
Aug 19 2020, 6:02 AM
labath closed D86140: [lldb] Add typedefs to the DeclContext they are created in.
Aug 19 2020, 6:02 AM · Restricted Project