clayborg (Greg Clayton)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 23 2014, 10:11 AM (195 w, 5 d)

Recent Activity

Fri, Jun 22

clayborg added a comment to D48479: Represent invalid UUIDs as UUIDs with length zero.

Would love to remove the "accept_zeroes" argument everywhere. Too much matching happens in LLDB and we can't have multiple shared libraries claiming zeros as their UUID

Fri, Jun 22, 4:05 PM
clayborg added inline comments to D48463: Prevent dead locking when calling PrintAsync.
Fri, Jun 22, 11:23 AM

Thu, Jun 21

clayborg added a comment to D47992: [lldb-mi] Clean up and update a few MI commands..

fine with leaving it as is if needed as well

Thu, Jun 21, 1:33 PM
clayborg added a comment to D47992: [lldb-mi] Clean up and update a few MI commands..

I didn't committed patch https://reviews.llvm.org/D48295 yet, so I think if we change HandleSBError handlers from
std::function<bool/void()> to std::function<bool/boid(CMICmdBase *)> we'll be able to create anonymous namespace and declare there a function that takes a pointer to current command class(this pointer) instead if defining

auto successHandler = [this] {
    // CODETAG_DEBUG_SESSION_RUNNING_PROG_RECEIVED_SIGINT_PAUSE_PROGRAM
    if (!CMIDriver::Instance().SetDriverStateRunningDebugging()) {
      const CMIUtilString &rErrMsg(CMIDriver::Instance().GetErrorDescription());
      this->SetError(CMIUtilString::Format(
          MIRSRC(IDS_CMD_ERR_SET_NEW_DRIVER_STATE),
          this->m_cmdData.strMiCmd.c_str(),
          rErrMsg.c_str()));
      return MIstatus::failure;
    }
    return MIstatus::success;

3 times in one file. It will reduce duplicating of code but I still think, is it worth it?

Thu, Jun 21, 11:44 AM
clayborg added a comment to D48450: [DataFormatter] Add CFDictionary data formatter.

It might be nice to split up the tests a bit. The data formatter tests have a history of testing 1000 things at once and saying "something failed in the this test with 1000 things". It would be nice if we knew that only dictionaries were failing by testing each type individually. Not required for this test, but try this out by reverting the changes in NSDictionary.cpp so you can see the kind of issue I am talking about.

Thu, Jun 21, 11:41 AM
clayborg added a comment to D48384: WIP: Let OptionArgParser fix up address arguments depending on intended use..

So bit zero is used by ARM folks sometime to indicate thumb when disassembling. We will need to check what happens for ARM and make sure we aren't breaking anything. If we have a thumb function at 0x2000, people sometimes might say "disassemble --start-address 0x2001" and expect thumb mode to be forced on. So this patch is fine, but it might lose the extra hint since it would sanitize the address to 0x2000. So verify the ARM case to make sure it didn't used to work.

Thu, Jun 21, 9:17 AM
clayborg added a comment to D48393: Make DWARFParsing more thread-safe.

I am not sure this will actually solve the problems you are seeing. This may avoid corrupting the internal DenseMap data structures, but it will not make the algorithm using them actually correct.
For example the pattern in ParseTypeFromDWARF is:

  1. check the "already parsed map". If the DIE is already parsed then you're done.
  2. if the map contains the magic "DIE_IS_BEING_PARSED" key, abort (recursive dwarf references)
  3. otherwise, insert the "DIE_IS_BEING_PARSED" key into the map
  4. do the parsing, which potentially involves recursive ParseTypeFromDWARF calls
  5. insert the parsed type into the map

    What you do is make each of the steps (1), (3), (5) atomic individually. However, the whole algorithm is not correct unless the whole sequence is atomic as a whole. Otherwise, if you have two threads trying to parse the same DIE (directly or indirectly), one of them could see the intermediate DIE_IS_BEING_PARSED and incorrectly assume that it encountered recursive types.
Thu, Jun 21, 7:36 AM
clayborg added a comment to D47991: Improve SBThread's stepping API using SBError parameter..

This patch is adding new overloads to SBAPI calls that don't return an SBError, such as:

// Old:
void StepOutOfFrame(SBFrame &frame);
// New:
void StepOutOfFrame(SBFrame &frame, SBError &error);

I wonder if it would be easier to use and more consistent with the rest of the API if we instead added an overload that returns an SBError, like this:

// New:
SBError StepOutOfFrameWithError(SBFrame &frame);
// Alternative names that are just as ugly.
SBError StepOutOfFrameE(SBFrame &frame);
SBError StepOutOfFrame2(SBFrame &frame);

@clayborg, @jingham: What do you think?

Thu, Jun 21, 7:13 AM

Tue, Jun 19

clayborg accepted D48215: Remove dependency from Host to python.
Tue, Jun 19, 8:50 AM

Mon, Jun 18

clayborg accepted D48295: Implement new methods for handling an error in MI commands..
Mon, Jun 18, 5:44 PM
clayborg accepted D47792: Fix up Info.plist when building LLDB.framework with CMake.

Looks good to me as long as Xcode still produces the same Info.plist in its LLDB.framework after your changes.

Mon, Jun 18, 4:40 PM
clayborg accepted D48272: Replace HostInfo::GetLLDBPath with specific functions.

Looks good. My only question is do we not require people to only fill in the directory portion of the FileSpec anymore for these functions? I am fine with way since hopefully FileSpec::AppendPathComponent handles things correctly.

Mon, Jun 18, 9:31 AM
clayborg added a comment to D48060: Introduce lldb-framework CMake target and centralize its logic.

Indeed!!! So happy to see this work getting done. Great stuff. Will be great to not have to maintain the Xcode project as some point in the near future.

Mon, Jun 18, 9:26 AM
clayborg added a comment to D47992: [lldb-mi] Clean up and update a few MI commands..

If you look at bool CMICmdCmdExecContinue::Execute(), you'll see that there are cases in which we need a flexible way to finish MI command(specific action in error case for example). We have a few options: not to add such an utility function, add and use it in simple situations where we just want to check on SBError status or we may create utility function with lambda functions in which user could specify actions he needs. What are your thoughts about this?

Mon, Jun 18, 9:19 AM

Fri, Jun 15

clayborg added a comment to D48215: Remove dependency from Host to python.

(pressed return too soon)

Would that address your reservations about this idea?

Fri, Jun 15, 9:11 AM
clayborg added a comment to D48215: Remove dependency from Host to python.

It might be nice to start with the removal of HostInfo::GetLLDBPath(...) first and fixing up all call sites that use that API to use the new internally sanctioned versions. Then submit this patch?

Fri, Jun 15, 9:08 AM
clayborg added a comment to D48215: Remove dependency from Host to python.

The internal api has no guarantees as to its stability.

Fri, Jun 15, 8:29 AM
clayborg added inline comments to D48215: Remove dependency from Host to python.
Fri, Jun 15, 7:48 AM
clayborg accepted D48212: Revert "[lldb-mi] Add overload method for setting an error".

I didn't remember we already did this in SBError... Sorry about that.

Fri, Jun 15, 7:17 AM

Thu, Jun 14

clayborg created D48177: Suppress SIGSEGV on Android when the program will recover.
Thu, Jun 14, 9:52 AM
clayborg added a comment to D24960: Remove unreachable from apis in posix terminal compat windows and return 0.

shouldn't these be returning -1 for some of these? Zero usually means no error. You would need to check each unix function and return the right error code for failure.

Thu, Jun 14, 8:14 AM · Restricted Project

Wed, Jun 13

clayborg added a comment to D47991: Improve SBThread's stepping API using SBError parameter..

Looks good. Just a question about including the commented out default arguments

Don't you think it increases readability of this code?

Wed, Jun 13, 8:59 AM
clayborg added inline comments to D48084: [FileSpec] Delegate common operations to llvm::sys::path.
Wed, Jun 13, 7:34 AM
clayborg added a comment to D47991: Improve SBThread's stepping API using SBError parameter..

Looks good. Just a question about including the commented out default arguments

Wed, Jun 13, 7:25 AM

Tue, Jun 12

clayborg resigned from D48060: Introduce lldb-framework CMake target and centralize its logic.

I'll defer to the cmake experts. I hope we can produce a LLDB.framework that is identical to the Xcode produced version, or at least as close.

Tue, Jun 12, 6:00 PM

Mon, Jun 11

clayborg accepted D48049: Add a new SBTarget::LoadCore() overload which surfaces errors if the load fails.

Remove extra spaces before ( and good to go.

Mon, Jun 11, 1:47 PM
clayborg added a comment to D47991: Improve SBThread's stepping API using SBError parameter..

It is unfortunate that we can't overload with the return value and return an SBError...

Mon, Jun 11, 8:38 AM
clayborg accepted D47991: Improve SBThread's stepping API using SBError parameter..

This is fine. Make sure all other steps have error overloads.

Mon, Jun 11, 8:37 AM
clayborg added inline comments to D47838: [lldb-mi] Re-implement MI -exec-step command..
Mon, Jun 11, 8:28 AM
clayborg added inline comments to D47992: [lldb-mi] Clean up and update a few MI commands..
Mon, Jun 11, 8:20 AM

Fri, Jun 8

clayborg added a comment to D47879: Implement vAttachWait in lldb-server.

This is a good start, We probably need a test. Pavel: can you outline what Ryan would need to do to add a test for this?

Fri, Jun 8, 9:16 AM
clayborg accepted D47415: [lldb, lldb-mi] Re-implement MI -exec-continue command..
Fri, Jun 8, 9:14 AM

Thu, Jun 7

clayborg accepted D47914: [lldb-mi] Add overloaded method for setting an error..
Thu, Jun 7, 3:59 PM
clayborg requested changes to D47415: [lldb, lldb-mi] Re-implement MI -exec-continue command..

Also need to remove m_lldbResult from the CMICmdCmdExecContinue class.

Thu, Jun 7, 2:00 PM
clayborg added inline comments to D47415: [lldb, lldb-mi] Re-implement MI -exec-continue command..
Thu, Jun 7, 1:58 PM
clayborg added inline comments to D47415: [lldb, lldb-mi] Re-implement MI -exec-continue command..
Thu, Jun 7, 1:47 PM
clayborg added inline comments to D47415: [lldb, lldb-mi] Re-implement MI -exec-continue command..
Thu, Jun 7, 1:25 PM
clayborg accepted D47881: DebugNamesDWARFIndex: Implement GetFunctions method.
Thu, Jun 7, 1:20 PM
clayborg accepted D47728: Fix DynamicRegisterInfo copying/moving issue.
Thu, Jun 7, 10:54 AM
clayborg added inline comments to D47879: Implement vAttachWait in lldb-server.
Thu, Jun 7, 9:00 AM

Wed, Jun 6

clayborg accepted D47832: DebugNamesDWARFIndex: Add support for partial indexes.

Just one nit you can choose to fix or not.

Wed, Jun 6, 9:57 AM
clayborg accepted D47781: DebugNamesDWARFIndex: Add ability to lookup variables.

Much better. Anything we can do to clearly define and simplify the indexers is great stuff.

Wed, Jun 6, 9:48 AM

Tue, Jun 5

clayborg added inline comments to D47792: Fix up Info.plist when building LLDB.framework with CMake.
Tue, Jun 5, 4:41 PM
clayborg accepted D47797: [lldb-mi] Re-implement MI -exec-next command..
Tue, Jun 5, 4:38 PM
clayborg added a comment to D47781: DebugNamesDWARFIndex: Add ability to lookup variables.

Feel free to check into it and modify the contract for GetGlobalVariables and possibly rename "name" to "basename" to be clear what the argument is. Then modify the headerdoc to clearly state what we are looking for if the filtering is going on higher up

Tue, Jun 5, 12:42 PM
clayborg requested changes to D47781: DebugNamesDWARFIndex: Add ability to lookup variables.

Check the context as noted in inline comments.

Tue, Jun 5, 10:21 AM
clayborg accepted D47629: [DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use.

I like this new setting much better. Looks good.

Tue, Jun 5, 9:38 AM

Mon, Jun 4

clayborg added inline comments to D47629: [DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use.
Mon, Jun 4, 4:10 PM
clayborg added a comment to D47728: Fix DynamicRegisterInfo copying/moving issue.

Only question is do we need the extra "struct Data"?

Mon, Jun 4, 4:07 PM
clayborg accepted D47147: DWARFIndex: Reduce duplication in the GetFunctions methods.

Some if statements simplifications if you want to, but looks good.

Mon, Jun 4, 3:59 PM
clayborg accepted D40470: Protect DWARFCompileUnit::m_die_array by a new mutex.
Mon, Jun 4, 9:16 AM

Fri, Jun 1

clayborg accepted D47660: Fix support for distinguishing archive members by timestamp on Darwin..
Fri, Jun 1, 4:02 PM
clayborg accepted D47470: AppleDWARFIndex: Get function method-ness directly from debug info.

I can't think of a better name, looks good. Thanks for taking the time, this will be great.

Fri, Jun 1, 9:33 AM
clayborg accepted D47612: Add dependency on clang-headers when building LLDB.framework using CMake.
Fri, Jun 1, 9:30 AM

Thu, May 31

clayborg added a comment to D47470: AppleDWARFIndex: Get function method-ness directly from debug info.

After seeing this, it might be nice to put the recursion that follows the DW_AT_specification and DW_AT_abstract_origin and avoids infinite recursion into a DWARFDIE function that takes a callback:

Thu, May 31, 10:44 AM
clayborg accepted D47481: Initialize FunctionCaller::m_struct_valid.

We need a clang warning that tells us when POD types are not initialized! I am sure we have other similar issues...

Thu, May 31, 10:38 AM
clayborg accepted D47470: AppleDWARFIndex: Get function method-ness directly from debug info.

Very nice

Thu, May 31, 10:34 AM

Tue, May 29

clayborg added a comment to D40470: Protect DWARFCompileUnit::m_die_array by a new mutex.

I think we should expose the llvm::sys::RWMutex and lets clients like BuildAddressRangeTable and SymbolFileDWARF::Index use it and get rid of m_die_array_usecount all together.

Tue, May 29, 11:40 PM
clayborg accepted D47495: [FileSpec] Re-implmenet removeLastPathComponent.

LGTM

Tue, May 29, 2:48 PM
clayborg added inline comments to D47495: [FileSpec] Re-implmenet removeLastPathComponent.
Tue, May 29, 2:41 PM
clayborg requested changes to D47495: [FileSpec] Re-implmenet removeLastPathComponent.

Remove the argument since we don't need it. Just assume we keep ".".

Tue, May 29, 2:38 PM
clayborg added a comment to D47470: AppleDWARFIndex: Get function method-ness directly from debug info.

So my previous code suggestion might not work as we would want to recurse through the specification or abstract origin chain.

Tue, May 29, 10:56 AM
clayborg added inline comments to D47470: AppleDWARFIndex: Get function method-ness directly from debug info.
Tue, May 29, 10:54 AM
clayborg accepted D46810: 3/3: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer.

Only question is if we remove the extra empty scope as noted in inline comments. Looks good.

Tue, May 29, 9:26 AM
clayborg accepted D47415: [lldb, lldb-mi] Re-implement MI -exec-continue command..

So nice to get rid of these HandleCommand hacks!

Tue, May 29, 9:18 AM
clayborg added inline comments to D47470: AppleDWARFIndex: Get function method-ness directly from debug info.
Tue, May 29, 9:16 AM

May 25 2018

clayborg accepted D47368: ManualDWARFIndex: Treat DW_TAG_subprogram and DW_TAG_inlined_subroutine the same way.
May 25 2018, 4:19 PM

May 24 2018

clayborg added a comment to D47302: [WIP] New class SBTargetSettings to store and manipulate all target's properties..

Be sure to not pass through any experimental settings.

May 24 2018, 2:37 PM
clayborg added a comment to D46810: 3/3: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer.

So this problem exists both in the LLDB and LLVM DWARF parsers. I am not sure this fix is safe. I would rather fix this by fixing DWARFDIE class to "do the right thing". We should be able to teach the DWARFDIE class to replace its "m_die" with the updated "m_die" if a method ever causes DWARFDIE to need to expand all DIEs in a DWARFUnit. That seems like a much safer fix. Having m_first_die is not safe because it if you call DWARFDIE::GetFirstChild() it will just add 1 to the "m_die" and we will crash. All parent, sibling and child code just do pointer arithmetic to find their counterparts. So since DWARFDIE has the "DWARFUnit *m_cu;" and "DWARFDebugInfoEntry *m_die;" we should use DWARFDIE to abstract this from users. Anyone playing directly with DWARFDebugInfoEntry must know the rules and do the right thing or just use DWARFDIE.

Is this statement still valid now with DWARFBaseDIE?

May 24 2018, 2:37 PM
clayborg added a comment to D47302: [WIP] New class SBTargetSettings to store and manipulate all target's properties..

no. Create a new SBTargetSettings class. SBTarget will hand one out for global and for target instance settings, Add all settings accessors to SBTargetSettings class

What is a difference between SBTargetSettings and TargetProperties classes except API level?

May 24 2018, 2:33 PM
clayborg added a comment to D46810: 3/3: Fix DWARFUnit::GetUnitDIEPtrOnly stale pointer.

A better solution here would be to have two functions: one for parsing the Unit DIE only and one for parsing all DIEs:

May 24 2018, 2:23 PM
clayborg added a comment to D47276: 2/3: Use DWARFBaseDIE as compile-time protection.

Rename to DWARFBaseDIE and this is good to go.

May 24 2018, 1:50 PM
clayborg accepted D47342: Move SystemInitializerFull header to source/API.
May 24 2018, 1:30 PM
clayborg added a comment to D47302: [WIP] New class SBTargetSettings to store and manipulate all target's properties..

no. Create a new SBTargetSettings class. SBTarget will hand one out for global and for target instance settings, Add all settings accessors to SBTargetSettings class

May 24 2018, 12:42 PM
clayborg added a comment to D47342: Move SystemInitializerFull header to source/API.

Looks good to me. Pavel, you ok with the file location?

May 24 2018, 12:40 PM
clayborg requested changes to D47275: 1/3: DWARFDIE split out to DWARFBaseDIE.

ok, just rename DWARFFirstDIE to DWARFBaseDIE and this is good to go.

May 24 2018, 11:46 AM
clayborg added a comment to D47302: [WIP] New class SBTargetSettings to store and manipulate all target's properties..

It might make sense to create a new SBTargetSettings class that has accessors. Then we can have to accessors on SBTarget:

class SBTarget {
  static SBTargetSettings GetGlobalSettings();
  SBTargetSettings GetSettings();
};

What global settings should be in your opinion? I guess that they should be stored in the SBDebugger, and a typical use case for them would be:

SBTarget target;
target.HookUpGlobalSettings;

I would code it exactly as I specified above. Ask the SBTarget for its settings. We would only expose the "target.*" settings through the SBTargetSettings class. SBDebugger could have its own settings for everything that is stored in the Debugger.cpp g_properties variable, those would be in SBDebuggerSettings if we need access to those:

class SBDebugger {
  static SBDebuggerSettings GetGlobalSettings();
  SBDebuggerSettings GetSettings();
};

This allows us to expose settings in a way that would allow us to serialize the settings and then load them again later.

class SBTargetSettings {
  // Accessors for "target...." setting
  void AppendImageSearchPath(const char *from, const char *to);
  size_t GetNumImageSearchPaths();
  const char *GetImageSearchPathAtIndex(size_t i);
  // Save and load all settings
  void Load(SBStream &s);
  void Save(SBStream &s);
};

Serialization sounds good, but, to accurately understand you, do you mean "classic" serialization with saving data into the file or serialization just for the time when debugger is run?

SBStream can be a string buffer or a file. We should probably save to JSON and load from JSON. But that doesn't need to be done in this patch.

So, for now we'll just save settings to a string buffer?

May 24 2018, 11:02 AM
clayborg added a comment to D47275: 1/3: DWARFDIE split out to DWARFBaseDIE.

I don't think a name like DWARFUnitDIE is a good one bacause it would make a weird is-a relationship (a DWARFDIE represetning a DW_TAG_variable is certainly not a "unit DIE" yet you could assign it to a DWARFUnitDIE&). We could have a DWARFUnitDIE type if we wanted to, but that would have to be a special type in addition to DWARFBasicDIE. However, I think that would be overkill.

Yeah, we just need to be able to tell the difference between the top level DIE we hand out with no children and the one that has all the abilities.

Yes, but this is not what this patch is doing. The class inheritance makes is such that any "die with all abilities" we hand out, also is-a "top level die with no children". This is where my problem comes from.

May 24 2018, 10:55 AM
clayborg added a comment to D47302: [WIP] New class SBTargetSettings to store and manipulate all target's properties..

It might make sense to create a new SBTargetSettings class that has accessors. Then we can have to accessors on SBTarget:

class SBTarget {
  static SBTargetSettings GetGlobalSettings();
  SBTargetSettings GetSettings();
};

What global settings should be in your opinion? I guess that they should be stored in the SBDebugger, and a typical use case for them would be:

SBTarget target;
target.HookUpGlobalSettings;
May 24 2018, 10:51 AM
clayborg added a comment to D47275: 1/3: DWARFDIE split out to DWARFBaseDIE.

I don't think a name like DWARFUnitDIE is a good one bacause it would make a weird is-a relationship (a DWARFDIE represetning a DW_TAG_variable is certainly not a "unit DIE" yet you could assign it to a DWARFUnitDIE&). We could have a DWARFUnitDIE type if we wanted to, but that would have to be a special type in addition to DWARFBasicDIE. However, I think that would be overkill.

May 24 2018, 10:16 AM
clayborg added a comment to D47302: [WIP] New class SBTargetSettings to store and manipulate all target's properties..

The main reason for the split up in SBTargetSettings is we have both global settings and the target specific settings. If you modify the global settings, you are modifying the target settings for all future targets. As they get created, they will copy the global settings. LLDB has global settings in a global variable, and then each new lldb_private::Target objects that are created have their own. So this new interface would mirror that setup. It also keeps all accessors from having a "bool apply_to_global" for each setting accessor.

May 24 2018, 10:12 AM
clayborg accepted D47275: 1/3: DWARFDIE split out to DWARFBaseDIE.

I like DWARFFirstDIE. Pavel should ok this too.

May 24 2018, 9:25 AM
clayborg added a comment to D47302: [WIP] New class SBTargetSettings to store and manipulate all target's properties..

It might make sense to create a new SBTargetSettings class that has accessors. Then we can have to accessors on SBTarget:

class SBTarget {
  static SBTargetSettings GetGlobalSettings();
  SBTargetSettings GetSettings();
};
May 24 2018, 7:40 AM

May 23 2018

clayborg added a comment to D47278: Remove lldb-private headers when building LLDB.framework with CMake.

The issue is actually that SystemInitializerFull.h and SystemInitializerFull.cpp are in the wrong directories. They belong in the "lldb/Initialization" and "Source//Initialization". We should fix this and then some/all of your changes won't be needed?

May 23 2018, 3:40 PM
clayborg accepted D47285: Add PPC64le support information.
May 23 2018, 3:15 PM
clayborg added a comment to D47285: Add PPC64le support information.

Is PPC64le only supported? no PPC64?

May 23 2018, 3:01 PM
clayborg added a comment to D47278: Remove lldb-private headers when building LLDB.framework with CMake.

sorry, not as a test, but just as a way to figure out if we are getting all the needed header files when we modify this framework header file copying code.

Ah, yeah. I'm in the process of trying to build LLDB.framework with cmake+ninja and get the same thing as when you build with xcodebuild.
As far as headers go, we roughly have parity after this change. CMake copies SystemInitializerFull.h into LLDB.Framework/Headers while Xcode doesn't, but I'm not sure that actually matters that much.

May 23 2018, 2:36 PM
clayborg added a comment to D47278: Remove lldb-private headers when building LLDB.framework with CMake.

sorry, not as a test, but just as a way to figure out if we are getting all the needed header files when we modify this framework header file copying code.

May 23 2018, 2:06 PM
clayborg added a comment to D47278: Remove lldb-private headers when building LLDB.framework with CMake.

I also think that'd be a great idea. The only way I can think to do that would be to build LLDB.framework using both build systems and compare the two. Is there a faster way?

May 23 2018, 1:31 PM
clayborg added a comment to D47276: 2/3: Use DWARFBaseDIE as compile-time protection.

Waiting for answers on renaming from patch 1/3 before commenting.

May 23 2018, 1:19 PM
clayborg added a comment to D47278: Remove lldb-private headers when building LLDB.framework with CMake.

It would be good to verify all headers that are in the LLDB.framework produced by Xcode builds are also in the LLDB.framework from cmake/ninja

May 23 2018, 1:18 PM
clayborg added a comment to D47275: 1/3: DWARFDIE split out to DWARFBaseDIE.

Marked things that don't belong in DWARFBasicDIE.

May 23 2018, 1:17 PM
clayborg accepted D47253: DWARF: Move indexing code from DWARFUnit to ManualDWARFIndex.

LGTM

May 23 2018, 7:06 AM
clayborg accepted D47250: Move ObjectFile initialization out of SystemInitializerCommon.

LGTM

May 23 2018, 7:05 AM

May 21 2018

clayborg added a comment to D47147: DWARFIndex: Reduce duplication in the GetFunctions methods.

Before I look too closely, we can probably add a DWARFDIE accessor function like:

May 21 2018, 2:24 PM
clayborg accepted D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target..
May 21 2018, 7:15 AM · Restricted Project
clayborg accepted D47110: [LLDB, lldb-mi] Add option --synchronous..

Looks fine, just move the option parsing code down by the others instead of having it before the checking for file args.

May 21 2018, 7:12 AM

May 18 2018

clayborg accepted D47064: Add some apple-tables lookup tests.
May 18 2018, 2:35 PM
clayborg updated the diff for D47021: Fix PathMappingList for relative and empty paths after recent FileSpec normalization changes.
  • Fixed Pavel's issues
  • If user specifies "" as the first directory in PathMappingList, it will match "."
  • User can specify "/" as the first directory to remap all absolute paths
  • Fixed FileSpec::IsAbsolute() and added tests for issues I discovered when adding tests for relative paths in PathMappingListTests
  • Modified tests in PathMappingListTests to use a help function which simplifies code
  • Added more tests to things that shouldn't get remapped
May 18 2018, 2:30 PM
clayborg added a comment to D47021: Fix PathMappingList for relative and empty paths after recent FileSpec normalization changes.

I will make the fixes and also test out mapping "" to "." as suggested.

May 18 2018, 10:50 AM