clayborg (Greg Clayton)
User

Projects

User does not belong to any projects.
User Since
Sep 23 2014, 10:11 AM (126 w, 4 d)

Recent Activity

Thu, Feb 23

clayborg accepted D30287: Introduce support for Debug Registers in RegisterContextNetBSD_x86_64.

That is fine, just wanted to check.

Thu, Feb 23, 2:08 PM · Restricted Project
clayborg added a comment to D30054: Delete DataBufferMemoryMap.

I would still vote to check Buffer for NULL. GetByteSize() and GetBytes() are usually accessed one time so there won't be a performance issue. If anyone wanted to actually use a DataBufferLLVM as a member variable, they would need to use a std::unique_ptr to one with the current designed because it can't be default constructed.

Thu, Feb 23, 1:01 PM
clayborg requested changes to D30054: Delete DataBufferMemoryMap.

In general code around crashes, we don't want to introduce any crashes in LLDB (no llvm_unreachable, asserts ok if code still functions without them). See inlined comments.

Thu, Feb 23, 12:17 PM
clayborg accepted D30288: Switch NetBSD from paccept(2) to accept4(2).
Thu, Feb 23, 11:51 AM · Restricted Project
clayborg added inline comments to D30287: Introduce support for Debug Registers in RegisterContextNetBSD_x86_64.
Thu, Feb 23, 11:50 AM · Restricted Project

Wed, Feb 22

clayborg accepted D30255: Ensure lldb-server waits for child debug servers to start up when passing them a port number to listen on..

Looks good.

Wed, Feb 22, 11:15 AM
clayborg accepted D30251: Map ELF files as writable so we can update the relocations in them.
Wed, Feb 22, 11:12 AM

Mon, Feb 20

clayborg added a comment to D29669: Hardware breakpoints implementation for Arm/AArch64 targets.

LGTM, but Pavel should give the ok as well.

Mon, Feb 20, 11:25 AM
clayborg accepted D30168: Log: Fix race in accessing the stream variable.
Mon, Feb 20, 9:55 AM

Fri, Feb 17

clayborg accepted D30094: Fix a couple of corner cases in NameMatches.
Fri, Feb 17, 9:02 AM

Thu, Feb 16

clayborg added inline comments to D30024: [lldb] Add support for "external" reports in ThreadSanitizer LLDB plugin.
Thu, Feb 16, 9:22 AM
clayborg requested changes to D29964: Finish breaking the dependency from lldbUtility -> Host.

Please add Xcode changes I sent you offline and this will be good to go.

Thu, Feb 16, 8:53 AM
clayborg accepted D30024: [lldb] Add support for "external" reports in ThreadSanitizer LLDB plugin.

Looks good.

Thu, Feb 16, 8:52 AM

Wed, Feb 15

clayborg added a comment to D30007: [lldb] Provide API to know which sanitizer generated an eStopReasonInstrumentation.

I am fine as long is Jim Ingham is fine. Jim, if you are good with this, just put it into Accept Revision.

Wed, Feb 15, 3:06 PM · Restricted Project

Tue, Feb 14

clayborg accepted D29932: Fix TestNameLookup for GCC.
Tue, Feb 14, 9:12 AM

Mon, Feb 13

clayborg added a comment to D29909: Break some more dependencies in lldbUtility.

I don't know enough to say for sure. Go ahead and try it with this patch as the files are pretty simple and losing the history wouldn't be too bad on these files. If it fails to have history, then you should use an SVN workflow for any future moves.

Mon, Feb 13, 3:11 PM
clayborg added a comment to D19603: Fix entry point lookup for ObjectFilePECOFF..

Anyone still care about this? It would be nice to move it along or abandon it.

Mon, Feb 13, 3:01 PM
clayborg added a comment to D29909: Break some more dependencies in lldbUtility.

Hopefully we are maintaining the SVN history for these files? Hard to tell from the patch.

Mon, Feb 13, 3:00 PM
clayborg accepted D29909: Break some more dependencies in lldbUtility.

Looks fine.

Mon, Feb 13, 2:59 PM
clayborg accepted D29895: Refactor log channel registration mechanism.

Looks good!

Mon, Feb 13, 10:07 AM
clayborg requested changes to D29581: Initial implementation of SB APIs for Tracing support..

Overall comments:

  • Check the SB API guidelines and submit new patch (https://lldb.llvm.org/SB-api-coding-rules.html)
  • It might be nice to make a lldb_private::Trace and a lldb::SBTrace object and have that be the object we used instead of adding many calls to SBProcess. You get a SBTrace object out of a SBProcess with:
class SBProcess
{
  SBTrace StartTrace(SBTraceOptions &opts, SBError &error);
  SBTrace GetTrace(); // Get current trace object, might return invalid/empty SBTrace object
};

Then all calls to start, stop, get config, get data, get metadata, are on the SBTrace object.

Mon, Feb 13, 9:50 AM

Fri, Feb 10

clayborg accepted D29823: Clean up debug logging.
Fri, Feb 10, 8:37 AM

Thu, Feb 9

clayborg added a comment to D29669: Hardware breakpoints implementation for Arm/AArch64 targets.

If it isn't making too much extra code I am fine with it being separate if you believe this is the right way forward. Give it some thought and if you still believe they should be separate, I won't object. It might be nice to send the HardwareBreakpoint structure down to the functions that are going to enable/disable the HW breakpoints instead of individual arguments as you may end up adding members to the HardwareBreakpoint in the future and this will keep you from having to update all of the signatures.

Thu, Feb 9, 9:20 AM

Wed, Feb 8

clayborg requested changes to D29669: Hardware breakpoints implementation for Arm/AArch64 targets.

I would prefer to see NativeBreakpoint struct expanded to have more member variables instead of adding a new hardware breakpoint list. Then you just ask any breakpoint to enable/disable/remove itself and the structure contains all of the info we need. Keeping two lists means we have to check two lists. Let me know if any of my inline comments weren't clear?

Wed, Feb 8, 10:11 AM
clayborg accepted D29615: Convert Log class to llvm streams.

Looks good as long as if we type two log enable commands like:

Wed, Feb 8, 9:38 AM

Mon, Feb 6

clayborg accepted D29288: Switch std::call_once to llvm::call_once.

Thanks for doing the right thing in LLVM first, looks great!

Mon, Feb 6, 9:07 AM · Restricted Project
clayborg added a comment to D29288: Switch std::call_once to llvm::call_once.

See inlined comment about initialization when llvm::once_flag is a member variable

Mon, Feb 6, 9:00 AM · Restricted Project

Fri, Feb 3

clayborg added a comment to D29514: Change Error::PutToLog to LLDB_LOG_ERROR.

Much better, just clean up the unnecessary includes and we are good to go.

Fri, Feb 3, 3:14 PM
clayborg added inline comments to D29510: Remove LIBLLDB_LOG_VERBOSE category.
Fri, Feb 3, 3:12 PM
clayborg accepted D29510: Remove LIBLLDB_LOG_VERBOSE category.

Looks good.

Fri, Feb 3, 2:29 PM
clayborg added a comment to D29514: Change Error::PutToLog to LLDB_LOG_ERROR.

So I agree with Pavel. Let us know what you think

Fri, Feb 3, 2:04 PM
clayborg added a comment to D29514: Change Error::PutToLog to LLDB_LOG_ERROR.

I realize the functionality would add a "error: " prefix if it wasn't there, but it seems like we could add this as a formatting option of the lldb_private::Error class? Then we can just switch the logging code to put the error in as a log item and add the extra flag to ensure we print "error:" it is isn't already prefixed with that?

Fri, Feb 3, 2:04 PM
clayborg added a comment to D29496: Push down more common code into PlatformPOSIX.

indeed very nice

Fri, Feb 3, 9:18 AM
clayborg accepted D29496: Push down more common code into PlatformPOSIX.
Fri, Feb 3, 9:06 AM

Wed, Feb 1

clayborg accepted D29406: Unify PlatformPOSIX::ResolveExecutable.
Wed, Feb 1, 2:22 PM
clayborg accepted D29403: Fix multi-process-driver.cpp build on NetBSD.
Wed, Feb 1, 10:28 AM · Restricted Project

Tue, Jan 31

clayborg accepted D29078: This patch implements a command to access and manipulate the Intel(R) MPX Boundary Tables..

If we can add a comment around "bndcfgu" where we use an SBData since it is a vector register and SBValue::GetValueAsUnsigned(...) won't work because it is a vector that would be nice for people to know. Other than that this is good to go!

Tue, Jan 31, 8:49 AM

Mon, Jan 30

clayborg added a comment to D29296: Make llvm::call_once more convenient to reuse out of LLVM.

LGTM, but I will let other LLVM folks give it the final approval.

Mon, Jan 30, 12:43 PM
clayborg requested changes to D29288: Switch std::call_once to llvm::call_once.

Be very careful when using this, you can't change member variables that used to be std::once to be statics. We also don't need the llvm namespace to be included with "using namespace llvm;" in many of the files.

Mon, Jan 30, 9:56 AM · Restricted Project
clayborg accepted D29215: [LLDB][MIPS] Fix TestMiniDumpNew.
Mon, Jan 30, 9:31 AM
clayborg accepted D29266: Synchronize PlatformNetBSD with Linux.
Mon, Jan 30, 9:27 AM · Restricted Project
clayborg accepted D29264: Add NetBSD support in Host::GetCurrentThreadID.
Mon, Jan 30, 9:26 AM · Restricted Project

Jan 25 2017

clayborg added a comment to D29078: This patch implements a command to access and manipulate the Intel(R) MPX Boundary Tables..

Let me know why your GetValueAsUnsigned isn't working on your register by stepping through it.

Jan 25 2017, 2:02 PM
clayborg added a comment to D29078: This patch implements a command to access and manipulate the Intel(R) MPX Boundary Tables..

I fixed SBData with:

Sending        packages/Python/lldbsuite/test/python_api/sbdata/TestSBData.py
Sending        source/API/SBData.cpp
Transmitting file data ..done
Committing transaction...
Committed revision 293102.
Jan 25 2017, 2:02 PM
clayborg added inline comments to D29078: This patch implements a command to access and manipulate the Intel(R) MPX Boundary Tables..
Jan 25 2017, 11:30 AM
clayborg added inline comments to D29078: This patch implements a command to access and manipulate the Intel(R) MPX Boundary Tables..
Jan 25 2017, 11:25 AM

Jan 24 2017

clayborg accepted D29091: Recognize Real-Time Signals on NetBSD.
Jan 24 2017, 10:57 AM · Restricted Project
clayborg accepted D29089: Switch HostInfoNetBSD::GetProgramFileSpec to sysctl(7).
Jan 24 2017, 10:14 AM · Restricted Project
clayborg added a comment to D29078: This patch implements a command to access and manipulate the Intel(R) MPX Boundary Tables..

Noticed another efficiency thing, see inlined comment.

Jan 24 2017, 9:47 AM
clayborg requested changes to D29078: This patch implements a command to access and manipulate the Intel(R) MPX Boundary Tables..

There are a few places where you are reading memory and then wanting to decode data from it. Right now, you first read memory into a local buffer and then we create an SBData and set the data to the bytes. It would be nice to have the ability to read memory and get an SBData back:

class SBProcess
{
  SBData ReadData(lldb::addr_t addr, size_t size, SBError &error);
};

Since the process can fill in the byte order and address byte size it can return an SBData that is ready to use. If you are interested in adding this in this patch, let me know.

Jan 24 2017, 9:41 AM

Jan 23 2017

clayborg accepted D29036: Add format_provider for lldb::StateType.

Looks good as long as the test suite is happy.

Jan 23 2017, 11:11 AM
clayborg added inline comments to D29036: Add format_provider for lldb::StateType.
Jan 23 2017, 9:45 AM
clayborg requested changes to D29036: Add format_provider for lldb::StateType.

Very close, just add a test for a bad value since someone can have code like:

Jan 23 2017, 9:43 AM
clayborg accepted D28808: Fix a bug where lldb does not respect the packet size..

Looks good.

Jan 23 2017, 8:49 AM
clayborg accepted D28944: Provide option to set pc of the file loaded in memory..

Looks good.

Jan 23 2017, 8:48 AM

Jan 20 2017

clayborg requested changes to D28808: Fix a bug where lldb does not respect the packet size..

See easy inline fix and this will be good to go

Jan 20 2017, 10:06 AM
clayborg requested changes to D28944: Provide option to set pc of the file loaded in memory..

Just set an error when "set_pc" is true and you get no entry point back from thee object file and this is good to go.

Jan 20 2017, 10:04 AM

Jan 19 2017

clayborg accepted D28804: Provide a substitute to load command of gdb.

Very nice!

Jan 19 2017, 9:14 AM

Jan 18 2017

clayborg requested changes to D28808: Fix a bug where lldb does not respect the packet size..

A few cleanups on the logging. See inlined comments.

Jan 18 2017, 8:50 AM

Jan 17 2017

clayborg added a comment to D28828: [DWARF] [ObjectYAML] Adding APIs for unittesting.

Looks fine to me. I'll let David chime in though just to be sure.

Jan 17 2017, 3:06 PM
clayborg accepted D27459: Add a more succinct logging syntax.

Looks good.

Jan 17 2017, 10:54 AM
clayborg requested changes to D28808: Fix a bug where lldb does not respect the packet size..

We need to check stub_max_size to make sure we don't get an integer underflow.

Jan 17 2017, 9:33 AM
clayborg requested changes to D28804: Provide a substitute to load command of gdb.

We need to agree on options names and move the object file loading code into ObjectFile.cpp. See inlined comments.

Jan 17 2017, 9:23 AM

Jan 13 2017

clayborg retitled D28704: Add a variant of DWARFDie::find() and DWARFDie::findRecursively() that takes a llvm::ArrayRef<dwarf::Attribute>. from to Add a variant of DWARFDie::find() and DWARFDie::findRecursively() that takes a llvm::ArrayRef<dwarf::Attribute>. .
Jan 13 2017, 2:30 PM
clayborg accepted D28677: FileSpec: Fix PrependPathComponent("/").
Jan 13 2017, 1:26 PM

Jan 12 2017

clayborg updated the diff for D28386: Add the ability to iterate across all attributes in a DIE..

Remove code after llvm_unreachable

Jan 12 2017, 4:22 PM
clayborg updated the diff for D28386: Add the ability to iterate across all attributes in a DIE..

Updated with suggestions. I can't make an invalid form test yet until we can use the yam2obj.

Jan 12 2017, 4:12 PM
clayborg abandoned D28613: Alternative proposal to https://reviews.llvm.org/D28581 where we return a DWARFormValue from DWARFDie::find*() functions.

After speaking with Adrian who had spoken with Dave Blaike, we settled on using the other proposal.

Jan 12 2017, 4:08 PM
clayborg updated the diff for D28581: Change DWARFDie::getAttributeValue() to DWARFDie::find(), add DWARFDie::findRecurse() and dwarf::toString() helper functions..

This is a proposed final solution with tests and all needed fixes.

Jan 12 2017, 3:14 PM
clayborg added inline comments to D28386: Add the ability to iterate across all attributes in a DIE..
Jan 12 2017, 10:55 AM
clayborg accepted D28616: Remove a couple of Stream flags.

Most of the DWARF stuff is about to go away anyway in favor of using the LLVM DWARF parser as I am currently modifying it to support all we need in LLDB so we can get rid of the entire DWARF parsers, so many of these changes are fine.

Jan 12 2017, 10:31 AM
clayborg added a comment to D28613: Alternative proposal to https://reviews.llvm.org/D28581 where we return a DWARFormValue from DWARFDie::find*() functions.

Whoops I accidentally added "aam" as a reviewer instead of Adrian...

Jan 12 2017, 9:27 AM
clayborg added a comment to D28581: Change DWARFDie::getAttributeValue() to DWARFDie::find(), add DWARFDie::findRecurse() and dwarf::toString() helper functions..

Alternate approach in https://reviews.llvm.org/D28613

Jan 12 2017, 9:21 AM
clayborg retitled D28613: Alternative proposal to https://reviews.llvm.org/D28581 where we return a DWARFormValue from DWARFDie::find*() functions from to Alternative proposal to https://reviews.llvm.org/D28581 where we return a DWARFormValue from DWARFDie::find*() functions.
Jan 12 2017, 9:20 AM

Jan 11 2017

clayborg updated the diff for D28581: Change DWARFDie::getAttributeValue() to DWARFDie::find(), add DWARFDie::findRecurse() and dwarf::toString() helper functions..

A complete version of switching over to use toFoo as Dave suggested.

Jan 11 2017, 4:57 PM
clayborg updated the diff for D28581: Change DWARFDie::getAttributeValue() to DWARFDie::find(), add DWARFDie::findRecurse() and dwarf::toString() helper functions..

Renamed findRecursive to findRecursively and don't recurse when looking through DW_AT_abstract_origin or DW_AT_specification DIEs.

Jan 11 2017, 3:46 PM
clayborg added a comment to D28581: Change DWARFDie::getAttributeValue() to DWARFDie::find(), add DWARFDie::findRecurse() and dwarf::toString() helper functions..

I do kind of agree with Adrian that this new code isn't as readable (see the "CallFile =" code...).

Jan 11 2017, 3:33 PM
clayborg added inline comments to D28581: Change DWARFDie::getAttributeValue() to DWARFDie::find(), add DWARFDie::findRecurse() and dwarf::toString() helper functions..
Jan 11 2017, 3:28 PM
clayborg retitled D28581: Change DWARFDie::getAttributeValue() to DWARFDie::find(), add DWARFDie::findRecurse() and dwarf::toString() helper functions. from to Change DWARFDie::getAttributeValue() to DWARFDie::find(), add DWARFDie::findRecurse() and dwarf::toString() helper functions..
Jan 11 2017, 3:03 PM
clayborg added a comment to D28569: Remove all variants of DWARFDie::getAttributeValueAs...() that had parameters that specified default values..

I will leave the tests using getValueOr() for now as I would like it to fail nicely so we can get a complete snapshot of what is failing instead of crashing on any failure and not knowing how many others tests would fail. I will read up on EXPECT_* and make a separate change for switching arg order if needed.

Jan 11 2017, 9:53 AM
clayborg accepted D28519: Add format_provider for the Error class.

Looks fine.

Jan 11 2017, 9:49 AM
clayborg requested changes to D27459: Add a more succinct logging syntax.

Looks fine except I would rather not have file + line on by default.

Jan 11 2017, 9:48 AM
clayborg retitled D28569: Remove all variants of DWARFDie::getAttributeValueAs...() that had parameters that specified default values. from to Remove all variants of DWARFDie::getAttributeValueAs...() that had parameters that specified default values..
Jan 11 2017, 9:41 AM
clayborg abandoned D28544: Remove all DWARFDie::getAttributeAsXXX() calls that returned default values and use new llvm::OptionalDefault template instead..

Looking in Optional,h I saw that is already has a getValueOr() function that does this. I will post a new patch that uses this since the title no longer makes sense.

Jan 11 2017, 9:13 AM

Jan 10 2017

clayborg added a comment to D28544: Remove all DWARFDie::getAttributeAsXXX() calls that returned default values and use new llvm::OptionalDefault template instead..

Do we want the case to be:

Jan 10 2017, 8:49 PM
clayborg retitled D28544: Remove all DWARFDie::getAttributeAsXXX() calls that returned default values and use new llvm::OptionalDefault template instead. from to Remove all DWARFDie::getAttributeAsXXX() calls that returned default values and use new llvm::OptionalDefault template instead..
Jan 10 2017, 5:50 PM
clayborg added a comment to D28519: Add format_provider for the Error class.

You can't add anything extra to the AsCString() since it returns a "const char *". You can't return a StringRef because it isn't backed by anything. You could return a std::string.

Jan 10 2017, 2:38 PM
clayborg added a comment to D28519: Add format_provider for the Error class.

Lets start with this. I would by default just emit the error string and then let users opt in via additional format modifiers to show the "error: " prefix and add the category.

Jan 10 2017, 2:35 PM
clayborg added a comment to D28519: Add format_provider for the Error class.

See inlined comment.

Jan 10 2017, 11:36 AM
clayborg added inline comments to D28386: Add the ability to iterate across all attributes in a DIE..
Jan 10 2017, 11:34 AM
clayborg updated the diff for D28386: Add the ability to iterate across all attributes in a DIE..

Fixed dblaikie's issues.

Jan 10 2017, 11:32 AM
clayborg added a comment to D27962: Get function start line number from DWARF info.

DWARFDie stuff looks good. I will let others comment on the rest.

Jan 10 2017, 11:20 AM

Jan 9 2017

clayborg added inline comments to D27962: Get function start line number from DWARF info.
Jan 9 2017, 5:21 PM
clayborg updated the diff for D28386: Add the ability to iterate across all attributes in a DIE..

Removed error handling as we consider this an invariant currently.
Updated test case to not use a switch statement with index.

Jan 9 2017, 4:28 PM
clayborg added a comment to D28386: Add the ability to iterate across all attributes in a DIE..

Marked things as done.

Jan 9 2017, 2:24 PM
clayborg updated the diff for D28386: Add the ability to iterate across all attributes in a DIE..

Added optional error handling to the attributes iterators. You can now pass an "llvm::Error *" to the attributes():

Jan 9 2017, 2:21 PM
clayborg accepted D28466: Improve Type::GetTypeScopeAndBasenameHelper and add unit tests.

Fix the consume_front mentioned in the inlined comment and this is good to go.

Jan 9 2017, 1:52 PM
clayborg requested changes to D28466: Improve Type::GetTypeScopeAndBasenameHelper and add unit tests.

Switch over to using llvm::StringRef in the functions where mentioned.

Jan 9 2017, 10:30 AM
clayborg accepted D27088: [LLDB][MIPS] Fix TestLldbGdbServer failure for MIPS.
Jan 9 2017, 10:17 AM
clayborg added a comment to D27962: Get function start line number from DWARF info.

If "StartLine" is actually DW_AT_decl_line, then I would suggest calling it "DeclLine". See other inlined comments.

Jan 9 2017, 10:04 AM