Page MenuHomePhabricator

clayborg (Greg Clayton)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 23 2014, 10:11 AM (242 w, 6 d)

Recent Activity

Yesterday

clayborg accepted D62159: LLGS: support 32-bit on 64-bit hosts.
Mon, May 20, 5:36 PM · Restricted Project
clayborg accepted D62073: DWARF: Introduce DWARFUnitHeader class.

Fine if we get things in quick!

Mon, May 20, 9:46 AM · Restricted Project
clayborg accepted D62008: DWARF: Introduce DWARFTypeUnit class.

Ok, as long as we get things in soon, some functionality is better than none.

Mon, May 20, 9:45 AM

Fri, May 17

clayborg added a comment to D62073: DWARF: Introduce DWARFUnitHeader class.

Are type units still disabled with the kill switch we had in? Or is this on top of the .debug_types patch? We have an internal .debug_types patch we have been using on an older LLVM/Clang/LLDB and we had some major performance issues regarding line tables so I am not sure where we would need to put these fixes (in this patch or in the .debug_types specific patch. This patch seems to enable parsing .debug_types already. The main issue we ran into were:

  • only search actual compile unit line tables for breakpoints as many .debug_type units will point to existing line tables from real compile units
  • don't add address ranges to type unit DWARFUnits from the line tables
  • re-work how support files are parsed by sharing the line tables within a SymbolFileDWARF. .debug_types has type units that reuse line tables from real compile units (thousands of times) and we ended up seeing the same line table being parsed over and over and over. Another way to fix this is to no vend a lldb_private::CompileUnit for any units that aren't really compile units (DW_UT_type and DW_UT_split_type).
Fri, May 17, 1:31 PM · Restricted Project

Wed, May 15

clayborg accepted D61502: Permit cross-CU references.

Just a few simplifications to GetName() and AppendTypeName() are in question and can optionally be done if needed.

Wed, May 15, 9:45 AM · Restricted Project, Restricted Project
clayborg added inline comments to D61687: Update Python tests for lldb-server on Windows.
Wed, May 15, 9:31 AM · Restricted Project
clayborg accepted D61908: DWARF: Add ability to reference debug info coming from multiple sections.
Wed, May 15, 9:07 AM · Restricted Project
clayborg added inline comments to D61853: [FuncUnwinders] Use "symbol file" unwind plans for unwinding.
Wed, May 15, 9:07 AM
clayborg accepted D61942: DWARFContext: Return empty data extractors instead of null pointers.
Wed, May 15, 9:02 AM · Restricted Project

Tue, May 14

clayborg added a comment to D61908: DWARF: Add ability to reference debug info coming from multiple sections.

See inline comments. Nice patch overall though, real close.

Tue, May 14, 11:08 AM · Restricted Project

Fri, May 10

clayborg added inline comments to D61779: FuncUnwinders: General clean up and optimization.
Fri, May 10, 7:48 AM
clayborg added inline comments to D61779: FuncUnwinders: General clean up and optimization.
Fri, May 10, 7:48 AM
clayborg added a comment to D61783: [DWARF] Use sequential integers for the IDs of the SymbolFileDWOs.

Might want the person that did DWO support to chime in here if that wasn't you?

Fri, May 10, 7:30 AM · Restricted Project
clayborg accepted D61783: [DWARF] Use sequential integers for the IDs of the SymbolFileDWOs.
Fri, May 10, 7:30 AM · Restricted Project
clayborg added inline comments to D61779: FuncUnwinders: General clean up and optimization.
Fri, May 10, 7:20 AM

Thu, May 9

clayborg committed rG23a7971ddff4: Disable the step over skipping calls feature since buildbots are not happy. (authored by clayborg).
Disable the step over skipping calls feature since buildbots are not happy.
Thu, May 9, 5:12 PM
clayborg committed rGdf225764b7d5: Improve step over performance by not stopping at branches that are function… (authored by clayborg).
Improve step over performance by not stopping at branches that are function…
Thu, May 9, 1:38 PM
clayborg accepted D61733: Breakpad: Generate unwind plans from STACK CFI records.

Thanks for the clarification! LGTM

Thu, May 9, 10:07 AM · Restricted Project
clayborg requested changes to D61687: Update Python tests for lldb-server on Windows.
Thu, May 9, 8:24 AM · Restricted Project
clayborg accepted D61611: [JITLoaderGDB] Set eTypeJIT for objects read from JIT descriptors.

lgtm.

Thu, May 9, 7:48 AM · Restricted Project, Restricted Project
clayborg added a comment to D61732: FuncUnwinders: Add a new "SymbolFile" unwind plan.

Looks good except the inline comment about all the unwind plans we have now. Would love to simplify this so that EH frame, compact unwind, ARM unwind, breakpad unwind all come from the symbol file or object files. Also be nice to have a register resolver that works without having a process so we can dump unwind info without needing a process.

Thu, May 9, 7:41 AM · Restricted Project
clayborg added inline comments to D61733: Breakpad: Generate unwind plans from STACK CFI records.
Thu, May 9, 7:33 AM · Restricted Project

Wed, May 8

clayborg committed rG5f8e88cd6941: Fix bug in ArchSpec::MergeFrom (authored by clayborg).
Fix bug in ArchSpec::MergeFrom
Wed, May 8, 3:01 PM

Tue, May 7

clayborg created D61659: Fix bug in ArchSpec::MergeFrom.
Tue, May 7, 2:58 PM · Restricted Project
clayborg added inline comments to D61648: [DWARF] Centralize user_id <-> DWARFDIE conversions.
Tue, May 7, 1:54 PM · Restricted Project
clayborg committed rGeeed7ee2cc7c: Added missing files from 360071. (authored by clayborg).
Added missing files from 360071.
Tue, May 7, 8:36 AM
clayborg requested changes to D61587: [lldb] Added support for dwarf expressions DW_OP_call2/DW_OP_call4.

A few nits and waiting for the test case.

Tue, May 7, 7:28 AM · Restricted Project

Mon, May 6

clayborg committed rGdab6189a591b: Revert xcode scheme changes that I didn't mean to check in. (authored by clayborg).
Revert xcode scheme changes that I didn't mean to check in.
Mon, May 6, 1:03 PM
clayborg committed rG8a7779209d93: Include inlined functions when figuring out a contiguous address range (authored by clayborg).
Include inlined functions when figuring out a contiguous address range
Mon, May 6, 1:00 PM
clayborg added a comment to D61476: [WIP] Add command interpreter to lldb-vscode.

actually lldb-vscode is always running locally so my previous concern is no longer valid.

Mon, May 6, 10:08 AM
clayborg added a comment to D61476: [WIP] Add command interpreter to lldb-vscode.

It is very dangerous to play with any real STDIN, STDOUT, or STDERR in the lldb-vscode world since this is what is usually the file descriptors used for all the packets.

Mon, May 6, 10:03 AM
clayborg accepted D61018: RegisterContextLLDB: Push CFA value on DWARF stack when evaluating register expressions.
Mon, May 6, 9:50 AM · Restricted Project
clayborg accepted D61577: [Driver] Change the way we deal with local lldbinit files..
Mon, May 6, 9:27 AM · Restricted Project, Restricted Project
clayborg added a comment to D61578: [Driver] Add command line option to allow loading local lldbinit file.

Just wanted to verify that we can put a:

settings set target.load-cwd-lldbinit true

in our ~/.lldbinit file and it will then load the local init file in the current working directory?

Mon, May 6, 9:19 AM · Restricted Project, Restricted Project
clayborg added a comment to D61587: [lldb] Added support for dwarf expressions DW_OP_call2/DW_OP_call4.

Need to add a test for this as well. And to cover all cases, you will need to test in a multi-compile unit example so we can ensure the relative CU offset of the call2 or call4 works in compile units that aren't the first one.

Mon, May 6, 9:15 AM · Restricted Project
clayborg requested changes to D61587: [lldb] Added support for dwarf expressions DW_OP_call2/DW_OP_call4.
Mon, May 6, 9:14 AM · Restricted Project

Fri, May 3

clayborg accepted D61543: Initialization: move InstructionEmulation to full initialization.

lgtm

Fri, May 3, 5:07 PM · Restricted Project
clayborg accepted D61473: ExpressionParser: only force link MCJIT when needed.
Fri, May 3, 4:18 PM · Restricted Project

Thu, May 2

clayborg added inline comments to D61292: Include inlined functions when figuring out a contiguous address range.
Thu, May 2, 1:43 PM · Restricted Project, Restricted Project
clayborg added a comment to D61292: Include inlined functions when figuring out a contiguous address range.

Jim: this is a modified version of the patch we had talked about a while ago with needed fixes for making sure the inline function call site is the same as the starting file and line. We have iterated on this locally already, so I am good with this patch. Let us know what you think.

Thu, May 2, 7:38 AM · Restricted Project, Restricted Project
clayborg added a reviewer for D61292: Include inlined functions when figuring out a contiguous address range: jingham.
Thu, May 2, 7:34 AM · Restricted Project, Restricted Project
clayborg added a comment to D61423: MinidumpYAML: add support for the ThreadList stream.

lgtm

Thu, May 2, 7:12 AM · Restricted Project

Tue, Apr 30

clayborg added inline comments to D61311: PostfixExpression: Use signed integers in IntegerNode.
Tue, Apr 30, 2:31 PM · Restricted Project

Mon, Apr 29

clayborg accepted D59015: [lldb-mi] Include full path in the -data-disassemble response.

LGTM. If there are any other " std::unique_ptr<char[]>" instances you would like to fix after this patch, feel free to submit more fixes without need for review.

Mon, Apr 29, 9:50 AM · Restricted Project, Restricted Project
clayborg accepted D61183: PostfixExpression: Introduce CFANode.
Mon, Apr 29, 7:14 AM · Restricted Project

Fri, Apr 26

clayborg added inline comments to D59015: [lldb-mi] Include full path in the -data-disassemble response.
Fri, Apr 26, 2:03 PM · Restricted Project, Restricted Project
clayborg added a comment to D61183: PostfixExpression: Introduce CFANode.

FYI: for all DWARF expressions found in EH frame stuff, they expect the CFA address to be pushed onto the stack prior to executing the expression. Do we just want to follow suit with the these expressions and avoid the extra needed node?

Fri, Apr 26, 2:01 PM · Restricted Project
clayborg accepted D61182: DWARFExpression: Fix implementation of DW_OP_pick.
Fri, Apr 26, 10:43 AM · Restricted Project

Thu, Apr 25

clayborg added a comment to D61090: [SBHostOS} Remove getting the python script interpreter path.

I agree with all that was said above.

Thu, Apr 25, 10:50 AM · Restricted Project
clayborg added a comment to D61090: [SBHostOS} Remove getting the python script interpreter path.

I agree with Pavel: no new SB APIs needed as this call is where it is supposed to be on SBHostOS.

Thu, Apr 25, 7:06 AM · Restricted Project

Wed, Apr 24

clayborg added a comment to D61101: Hide stderr output from lldb-argdumper.

lgtm

Wed, Apr 24, 4:26 PM · Restricted Project
clayborg added a comment to D61090: [SBHostOS} Remove getting the python script interpreter path.

@clayborg I would also like to point out that some of us fair amount of time cleaning LLDB_DISABLE_PYTHON leaking all over the codebase, and this is basically the last occurrence, so we're motivated to have this going away/fixed in some way.

Wed, Apr 24, 3:34 PM · Restricted Project
clayborg requested changes to D61090: [SBHostOS} Remove getting the python script interpreter path.
Wed, Apr 24, 2:45 PM · Restricted Project
clayborg added a reviewer for D61090: [SBHostOS} Remove getting the python script interpreter path: clayborg.
Wed, Apr 24, 2:45 PM · Restricted Project
clayborg added a comment to D61090: [SBHostOS} Remove getting the python script interpreter path.

People use this to populate the extra system path in python scripts when "import lldb" fails and throws an exception as you can run "lldb -P" to get the python path for the lldb module for the current lldb in your path.

Wed, Apr 24, 2:45 PM · Restricted Project
clayborg added a comment to D61044: Fix infinite recursion when calling C++ template functions.

Love having the AST output available so we can compare real to DWARF converted stuff!

Wed, Apr 24, 10:52 AM · Restricted Project
clayborg added a comment to D61064: Object/Minidump: Add support for the ThreadList stream.

LGTM, but since in llvm someone else should ok too.

Wed, Apr 24, 10:04 AM · Restricted Project

Tue, Apr 23

clayborg accepted D60468: Lock accesses to OptionValueFileSpecList objects.

Fine to fix the current issue. We should think about building this into the base class eventually. We can always have multi-threaded access, so we will always need to protect modifying operations.

Tue, Apr 23, 11:50 AM · Restricted Project
clayborg accepted D61000: Kill modify-python-lldb.py.

Thanks for the diff! LGTM

Tue, Apr 23, 11:40 AM · Restricted Project
clayborg added a comment to D61000: Kill modify-python-lldb.py.

Can you post a diff of things before and after? Hard to tell what we are opting out of just by seeing this patch

Tue, Apr 23, 8:06 AM · Restricted Project
clayborg added a comment to D61018: RegisterContextLLDB: Push CFA value on DWARF stack when evaluating register expressions.

Got errors trying to compile this .s file on mac:

$ ~/Documents/src/lldb/svn/lldb/llvm-build/Release+Asserts/x86_64/bin/clang foo.s -o foo.o
foo.s:3:9: error: unknown directive
        .type bar, @function
        ^
foo.s:11:9: error: unknown directive
        .size bar, .-bar
        ^
foo.s:13:9: error: unknown directive
        .type foo, @function
        ^
foo.s:25:9: error: unknown directive
        .size foo, .-foo
        ^
foo.s:27:9: error: unknown directive
        .type main, @function
        ^
foo.s:49:9: error: unknown directive
        .size main, .-main
Tue, Apr 23, 7:43 AM · Restricted Project

Mon, Apr 22

clayborg added a comment to D60968: WIP, RFC: Add an example of how LLDB python plug-in could look like.

Not sure what functionality this really adds? It seems to just wrap the OS plug-in python class in a different way?

Mon, Apr 22, 11:23 AM · Restricted Project
clayborg accepted D60948: yamlify TestMiniDumpUUID binaries.
Mon, Apr 22, 7:18 AM · Restricted Project
clayborg added inline comments to D59015: [lldb-mi] Include full path in the -data-disassemble response.
Mon, Apr 22, 7:05 AM · Restricted Project, Restricted Project

Apr 16 2019

clayborg added inline comments to D59015: [lldb-mi] Include full path in the -data-disassemble response.
Apr 16 2019, 7:57 AM · Restricted Project, Restricted Project

Apr 15 2019

clayborg accepted D60653: Correctly check if a warning message lacks a trailing new line.
Apr 15 2019, 3:15 PM · Restricted Project, Restricted Project
clayborg added a comment to D60667: Allow direct comparison of ConstString against StringRef.

LGTM.

Apr 15 2019, 10:45 AM · Restricted Project, Restricted Project

Apr 12 2019

clayborg added a comment to D59960: Fix for ambiguous lookup in expressions between local variable and namespace.

Jim Ingham said:

Apr 12 2019, 11:03 AM · Restricted Project
clayborg added a comment to D59960: Fix for ambiguous lookup in expressions between local variable and namespace.

Just thought of 1 additional way to allow us to pull in fewer var declarations: get a list of all of the member variable names in the current class when stopped in a class method and only add ones that match local variables. If we are in a static member variable then skip of course. Comments? Thoughts?

Apr 12 2019, 7:21 AM · Restricted Project

Apr 11 2019

clayborg added inline comments to D60496: [lldb-server] Update tests to use std::thread/mutex for all platforms.
Apr 11 2019, 2:14 PM · Restricted Project
clayborg added a comment to D60501: Minidump: extend UUID byte-swapping to windows platform.

Nice!

Apr 11 2019, 11:29 AM · Restricted Project
clayborg requested changes to D59960: Fix for ambiguous lookup in expressions between local variable and namespace.

I would really like to see a better solution than just adding all var decls to an expression as this is already costly for C++ and we shouldn't propagate this. I know it used to be really really expensive. Not sure if it still is. But it would be worth measuring by running expressions in C++ and adding and removing this feature to see how much it costs us when we have many variables all with unique and complex types.

Apr 11 2019, 11:23 AM · Restricted Project

Apr 9 2019

clayborg added inline comments to D59015: [lldb-mi] Include full path in the -data-disassemble response.
Apr 9 2019, 1:42 PM · Restricted Project, Restricted Project
clayborg added a comment to D60468: Lock accesses to OptionValueFileSpecList objects.

Almost seems like we can build the mutex into the base class OptionValue as we need general threaded protection for every setting. They any function that gets or sets the value should be able to protect itself using the base mutex

Apr 9 2019, 9:27 AM · Restricted Project
clayborg added inline comments to D60405: MinidumpYAML: Add support for ModuleList stream.
Apr 9 2019, 7:08 AM · Restricted Project

Apr 8 2019

clayborg added a comment to D60405: MinidumpYAML: Add support for ModuleList stream.

LGTM. Probably should get one more ok

Apr 8 2019, 8:07 AM · Restricted Project
clayborg accepted D60268: Breakpad: Parse Stack CFI records.
Apr 8 2019, 7:57 AM · Restricted Project

Apr 6 2019

clayborg added a comment to D60340: Unify random timeouts throughout LLDB and make them configurable..

The motivation for this test was the observation that an asanified LLDB would often exhibit seemingly random test failures that could be traced back to debugserver packets getting out of sync. With this path applied I can no longer reproduce the one particular failure mode that I was investigating.

Apr 6 2019, 11:53 AM · Restricted Project

Apr 5 2019

clayborg added a comment to D60300: [testsuite] Split Objective-C data formatter.

I think this is good regardless for readability, but I would really appreciate if we can collect some numbers to see how effective this change actually is.

I see 120 -> 12 seconds on my iMac Pro.

Apr 5 2019, 10:58 AM · Restricted Project, Restricted Project
clayborg added inline comments to D60268: Breakpad: Parse Stack CFI records.
Apr 5 2019, 7:24 AM · Restricted Project
clayborg added inline comments to D60268: Breakpad: Parse Stack CFI records.
Apr 5 2019, 7:21 AM · Restricted Project
clayborg requested changes to D59015: [lldb-mi] Include full path in the -data-disassemble response.
Apr 5 2019, 7:06 AM · Restricted Project, Restricted Project

Apr 4 2019

clayborg accepted D60121: Object/Minidump: Add support for reading the ModuleList stream.
Apr 4 2019, 5:13 PM · Restricted Project

Apr 3 2019

clayborg added a comment to D60153: Re-enable most lldb-vscode tests on Linux..

Thanks for getting this stuff reliably working. I debug using VS Code every day using lldb-vscode and it is my favorite LLDB based debugger! I look forward to seeing support for Windows and linux being tested and available.

Apr 3 2019, 10:35 AM · Restricted Project, Restricted Project
clayborg added a comment to D59826: [expression] Explicitly build the lookup table of any TagDecl's context.

Any way to dump the AST in a test to verify we don't create multiple?

I think I might be able to use the log object to dump the AST and then from python it would be possible to check for duplicates. Is that a good direction?

Apr 3 2019, 9:56 AM · Restricted Project
clayborg committed rGbbc428e93a7d: Attempt #2 to get this patch working. I will watch the build bots carefully… (authored by clayborg).
Attempt #2 to get this patch working. I will watch the build bots carefully…
Apr 3 2019, 9:30 AM
clayborg added a comment to D60001: Allow partial UUID matching in Minidump core file plug-in.

ok, so I think I figured out what was going on: I had the .so files still in my build packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new directory. They didn't show up in the "svn stat" command because they are ignored!!! arg! That is what was causing problems when testing on my machine.

Apr 3 2019, 9:30 AM · Restricted Project
clayborg added inline comments to D60001: Allow partial UUID matching in Minidump core file plug-in.
Apr 3 2019, 8:58 AM · Restricted Project

Apr 2 2019

clayborg added inline comments to D60172: Renamed Target::GetSharedModule to AddModule, allow for ModulesDidLoad to be delayed when batch adding Modules.
Apr 2 2019, 8:55 PM · Restricted Project
clayborg committed rG380c2420ecb0: Clean up windows build bot. (authored by clayborg).
Clean up windows build bot.
Apr 2 2019, 10:50 AM
clayborg committed rG505058686014: Fix buildbot where paths were not matching up. (authored by clayborg).
Fix buildbot where paths were not matching up.
Apr 2 2019, 9:38 AM
clayborg added inline comments to D60001: Allow partial UUID matching in Minidump core file plug-in.
Apr 2 2019, 9:01 AM · Restricted Project
clayborg committed rG838bba9c34bf: Allow partial UUID matching in Minidump core file plug-in (authored by clayborg).
Allow partial UUID matching in Minidump core file plug-in
Apr 2 2019, 8:41 AM
clayborg added a comment to D59826: [expression] Explicitly build the lookup table of any TagDecl's context.

Any way to dump the AST in a test to verify we don't create multiple?

Apr 2 2019, 8:35 AM · Restricted Project
clayborg accepted D60119: modify-python-lldb.py: clean up __iter__ and __len__ support.
Apr 2 2019, 8:33 AM · Restricted Project

Apr 1 2019

clayborg added inline comments to D60001: Allow partial UUID matching in Minidump core file plug-in.
Apr 1 2019, 4:30 PM · Restricted Project
clayborg updated the diff for D60001: Allow partial UUID matching in Minidump core file plug-in.

Fix comments and address review issues.

Apr 1 2019, 4:30 PM · Restricted Project
clayborg added a comment to D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected.

In general any call that is doing anything through the public API needs to take the target API mutex. It is ok for quick accessors or other things to not do so, but anything that would need to keep the process in its current state, like asking a frame for a register value, should take the API mutex to ensure one thread doesn't say "run" and another says "what is the PC from frame 0 of thread 1".

Apr 1 2019, 11:10 AM · Restricted Project
clayborg added a comment to D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected.

Added Jim Ingham in case he wants to chime in here.

Apr 1 2019, 11:06 AM · Restricted Project
clayborg added a reviewer for D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected: jingham.
Apr 1 2019, 11:06 AM · Restricted Project