Page MenuHomePhabricator

clayborg (Greg Clayton)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 23 2014, 10:11 AM (251 w, 2 d)

Recent Activity

Today

clayborg added a comment to D62570: Use LLVM's debug line parser in LLDB.

We also need to test to ensure this doesn't regress parsing speed. Need a large C++ project, like LLDB with debug clang and debug llvm, and something that forces all line tables to be parsed (not just the prologues like when setting a breakpoint) and see how things compare between the two methods.

Thu, Jul 18, 1:41 PM · Restricted Project, Restricted Project
clayborg added a comment to D64917: Add offsetof support to expression evaluator..

nice!

Thu, Jul 18, 8:07 AM · Restricted Project

Tue, Jul 16

clayborg added a comment to D62570: Use LLVM's debug line parser in LLDB.

So in the actual line table parsing code it seems like we are creating a line table in LLVM format, complete with sequences and a bunch of LLVM data structures, and then copying the results over into LLDB and then throwing away the LLVM line table. It would be nicer to modify the LLVM line table code to be able to get a callback when a row was pushed (if it isn't already in that format) and expose that so clients, like LLDB, can just get a callback and only store the data structure we care about. This would allow us to share the llvm line table parsing code and re-use it in LLDB, but not require twice the memory and overhead. Symbolication tools could also take advantage of this by not storing any rows except the closest matching row.

Tue, Jul 16, 6:04 PM · Restricted Project, Restricted Project

Mon, Jul 15

clayborg accepted D64698: Handle EOF from `lldb-vscode`.
Mon, Jul 15, 5:53 PM · Restricted Project, Restricted Project

Fri, Jul 12

clayborg added a comment to D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.

I need some feedback here. Is the general consensus that we need to modify raw_ostream to support seeking or continue with the current FileWriter?

Fri, Jul 12, 1:19 PM · Restricted Project
clayborg accepted D64159: [Core] Generalize ValueObject::MaybeCalculateCompleteType.
Fri, Jul 12, 10:06 AM · Restricted Project

Thu, Jul 11

clayborg added reviewers for D64578: Make Python version setting actually effective: sgraenitz, JDevlieghere.
Thu, Jul 11, 1:17 PM · Restricted Project
clayborg added a comment to D64578: Make Python version setting actually effective.

Probably best to have someone else review cmake changes, as I am not very familiar with all the intricacies of all the settings. I will add some people with more cmake experience to the reviewers. Jonas and Stefan, feel free to add more cmake experts here.

Thu, Jul 11, 1:17 PM · Restricted Project
clayborg accepted D64535: Add convenience methods to convert LLDB to LLVM data structures..
Thu, Jul 11, 7:23 AM · Restricted Project, Restricted Project

Wed, Jul 10

clayborg added a comment to D64535: Add convenience methods to convert LLDB to LLVM data structures..

Just wondering if we want to cache the llvm::DWARFContext in the LLDB DWARFContext. See inline comments.

Wed, Jul 10, 3:42 PM · Restricted Project, Restricted Project
clayborg added a comment to D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.

Ping for comments

Wed, Jul 10, 1:23 PM · Restricted Project
clayborg added a comment to D64373: Don't use PyInt on Python 3.

never mind, I missed that you had!

Wed, Jul 10, 1:20 PM · Restricted Project
clayborg added a comment to D64373: Don't use PyInt on Python 3.

ok, so do you want to abandon this patch?

Wed, Jul 10, 11:49 AM · Restricted Project
clayborg accepted D64444: Add Python 3.6 and 3.7 to the version list.
Wed, Jul 10, 11:44 AM · Restricted Project, Restricted Project
clayborg added inline comments to D62931: [lldb-server] Add setting to force 'g' packet use.
Wed, Jul 10, 11:38 AM · Restricted Project, Restricted Project
clayborg added a comment to D64407: [DWARF] Simplify DWARFAttribute. NFC..

I agree with all comments. After those few things are fixed, this looks good.

Wed, Jul 10, 11:33 AM · Restricted Project

Mon, Jul 8

clayborg added a comment to D64194: [lldb] Fix crash due to unicode characters and dollars in variable names..

Do we really expect users to actually write code where variables start with '$'? We will need to make some clear rules of the lookup order if so. I would suggest the following order

  • look for real variables by exact name match first
  • look for expression global variables next
  • look for expression results next ($<digit>)
  • look for registers
Mon, Jul 8, 1:04 PM · Restricted Project
clayborg added a comment to D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.

I am happy to do what is needed. Anyone have any comments on my last comments? We just need a solid direction that everyone thinks we should follow.

Mon, Jul 8, 10:34 AM · Restricted Project

Wed, Jul 3

clayborg added a comment to D64159: [Core] Generalize ValueObject::MaybeCalculateCompleteType.

The main issue I have with this is the name of the function we are adding to LanguageRuntime. See inlined comments.

Wed, Jul 3, 4:31 PM · Restricted Project
clayborg added a comment to D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.

So what is the verdict on FileWriter? Do we want real error handling like is currently coded, or just be able to check sometime later and find out that something went wrong? If we just want to check that something is wrong, we don't need to maintain a llvm::Error, we can just check the stream at any point and return a generic error that means nothing. If this is what we do, I am not sure I see the point of adding the error handling. The current solution calls hasError() which checks if the llvm::Error contains a ErrorInfoBase pointer so the call is pretty efficient. And the first thing to cause the error will stay in the error object. Without the error checking, FileWriter seems to just forward IO operations to the underlying std::ostream. I wonder if it would be easier to review the other components of GSYM by inlining those FileWriter use sites.

Others may have different opinions but my feeling is that FileWriter does lots of duplicate work that has been done in raw_ostream. It tries to catch errors but I don't see how errors are handled. It may become clearer when other components of GSYM are checked in. Before that, without concrete use cases, it may be difficult to suggest what FileWriter should do.

Wed, Jul 3, 1:00 PM · Restricted Project

Tue, Jul 2

clayborg added a comment to D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.

So what is the verdict on FileWriter? Do we want real error handling like is currently coded, or just be able to check sometime later and find out that something went wrong? If we just want to check that something is wrong, we don't need to maintain a llvm::Error, we can just check the stream at any point and return a generic error that means nothing. If this is what we do, I am not sure I see the point of adding the error handling. The current solution calls hasError() which checks if the llvm::Error contains a ErrorInfoBase pointer so the call is pretty efficient. And the first thing to cause the error will stay in the error object.

Tue, Jul 2, 2:53 PM · Restricted Project

Mon, Jul 1

clayborg added a comment to D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.

Jim, you ok with doing the symbol context scope refactoring as a separate diff?

Mon, Jul 1, 11:09 AM · Restricted Project
clayborg accepted D61233: Refactor ObjectFile::GetSDKVersion.
Mon, Jul 1, 8:35 AM · Restricted Project, Restricted Project
clayborg updated the diff for D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.

Don't #include <iostream>

Mon, Jul 1, 7:30 AM · Restricted Project
clayborg added a comment to D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.

It currently uses std::ostream over llvm::raw_ostream because llvm::raw_ostream doesn't support seeking which is required when encoding and decoding GSYM data.

There is raw_fd_ostream::seek. You just need to make sure raw_fd_ostream::supportsSeeking() returns true when you open it.

Mon, Jul 1, 7:24 AM · Restricted Project
clayborg added inline comments to D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.
Mon, Jul 1, 7:15 AM · Restricted Project
clayborg accepted D63540: Fix lookup of symbols at the same address with no size vs. size.
Mon, Jul 1, 7:09 AM · Restricted Project, Restricted Project

Fri, Jun 28

clayborg accepted D63914: Make the expression parser work for missing weak symbols.
Fri, Jun 28, 2:37 PM · Restricted Project, Restricted Project
clayborg requested changes to D63914: Make the expression parser work for missing weak symbols.

A few nits, but nothing structural. Fix those and this will be good to go.

Fri, Jun 28, 1:45 PM · Restricted Project, Restricted Project
clayborg updated the diff for D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.

Update this patch to compile after r364634.

Fri, Jun 28, 10:58 AM · Restricted Project
clayborg added a comment to D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.

This looks good to me. I wonder if the SymbolContextScope -> Language calculation that you do in IsRuntimeSupportValue should really be done in SymbolContextScope? If that's going to be the policy for going from SymbolContextScope, maybe centralize it there?

Fri, Jun 28, 10:52 AM · Restricted Project

Thu, Jun 27

clayborg accepted D62887: Update the thread list before setting stop reasons with an OS plugin.
Thu, Jun 27, 9:32 PM · Restricted Project, Restricted Project
clayborg updated the diff for D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.

Added llvm::Error as a member variable of FileWriter. Each method that does anything with the stream will check if there is an error and return if the FileWriter has encountered an error. Added:

llvm::Error FileWriter::takeError();

To allow clients to check errors after a series of method calls on FileWriter objects. The error must be checked or taken before the FileWriter object destructs.

Thu, Jun 27, 3:23 PM · Restricted Project
clayborg updated the diff for D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.

Use reinterpret_cast instead of C casts.

Thu, Jun 27, 1:14 PM · Restricted Project
clayborg added a comment to D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.

JF is currently working on making BitStream error-safe over in https://reviews.llvm.org/D63518. Perhaps there are some patterns that are worth copying.

Thu, Jun 27, 12:48 PM · Restricted Project
clayborg updated the diff for D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.

Switch FileWriter::writeCStr to use llvm::StringRef. Addressed other review comments.

Thu, Jun 27, 10:03 AM · Restricted Project
clayborg added inline comments to D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.
Thu, Jun 27, 10:03 AM · Restricted Project

Wed, Jun 26

clayborg added a comment to D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.

How invasive would it be to change this to not use an ostream but to instead use an in memory buffer?

Wed, Jun 26, 4:06 PM · Restricted Project
clayborg added a comment to D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.

Jim: do you have any other solutions that could make this work?

Wed, Jun 26, 2:38 PM · Restricted Project
clayborg added inline comments to D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.
Wed, Jun 26, 2:32 PM · Restricted Project
clayborg added inline comments to D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.
Wed, Jun 26, 2:10 PM · Restricted Project
clayborg updated the diff for D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.

Switched FileWriter::writeData() to use a llvm::ArrayRef<uint8_t> instead of pointer and size.
Address other review comments.

Wed, Jun 26, 1:38 PM · Restricted Project
clayborg added inline comments to D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.
Wed, Jun 26, 1:38 PM · Restricted Project
clayborg added inline comments to D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.
Wed, Jun 26, 10:28 AM · Restricted Project
clayborg added inline comments to D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.
Wed, Jun 26, 9:39 AM · Restricted Project
clayborg committed rG208cce7500b3: Fix builbots after r364427. (authored by clayborg).
Fix builbots after r364427.
Wed, Jun 26, 9:25 AM
clayborg added a comment to D63104: Add GSYM utility files along with unit tests..

Fixed build bots as they showed an iterator was being used even if it was the end of a collection.

Wed, Jun 26, 9:25 AM · Restricted Project
clayborg created D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges.
Wed, Jun 26, 8:58 AM · Restricted Project
clayborg added a comment to D61233: Refactor ObjectFile::GetSDKVersion.

lgtm

Wed, Jun 26, 7:29 AM · Restricted Project, Restricted Project
clayborg committed rG044776bf5d92: Add GSYM utility files along with unit tests. (authored by clayborg).
Add GSYM utility files along with unit tests.
Wed, Jun 26, 7:12 AM
clayborg accepted D63802: Handle nested register definition xml files from the remote serial protocol stub.

few nits, but looks good.

Wed, Jun 26, 7:08 AM · Restricted Project, Restricted Project

Tue, Jun 25

clayborg added a comment to D63540: Fix lookup of symbols at the same address with no size vs. size.

So I am fine with symbols having zero size being in the symbol table. I would be fine not changing anything in the sorting and leaving some symbols with zero size, we just need to fix:

Symbol *Symtab::FindSymbolAtFileAddress(addr_t file_addr);

To ignore zero sized symbols when it finds them _if_ there is another symbol that has a size for that address. Wouldn't that fix the issue here?

You are assuming here that the symbols have size zero at the time we are performing the lookup. If I understand correctly what is going on, the problem here is that the code munging the symbol table (InitAddressIndexes), will set these symbols to have non-zero size. This is what this patch is trying to avoid.

Tue, Jun 25, 5:06 PM · Restricted Project, Restricted Project

Mon, Jun 24

clayborg accepted D63730: Remove core loading timeout.
Mon, Jun 24, 11:05 AM · Restricted Project
clayborg added inline comments to D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.
Mon, Jun 24, 10:25 AM · Restricted Project
clayborg added a comment to D63104: Add GSYM utility files along with unit tests..

It would be great to get this moving. I have figured out how the rest of the patches will be broken up to make them really simple to test and get in, but need this one to go in first.

Mon, Jun 24, 9:08 AM · Restricted Project
clayborg added inline comments to D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.
Mon, Jun 24, 8:57 AM · Restricted Project
clayborg accepted D63643: DWARF: Add support for type units+split dwarf combo.
Mon, Jun 24, 8:33 AM · Restricted Project
clayborg added a comment to D63591: DWARFDebugLoc: Make parsing and error reporting more robust.

DataExtractor is a copy of the one from LLDB from a while back and changes have been made to adapt it to llvm. DataExtractor was designed so that you can have one of them (like for .debug_info or any other DWARF section) and use this same extractor from multiple threads. This is why it is currently stateless.

Mon, Jun 24, 8:27 AM · Restricted Project

Thu, Jun 20

clayborg added a comment to D63104: Add GSYM utility files along with unit tests..

friendly ping

Thu, Jun 20, 11:20 AM · Restricted Project
clayborg accepted D63428: DWARF: Add "dwo_num" field to the DIERef class.
Thu, Jun 20, 7:04 AM · Restricted Project

Wed, Jun 19

clayborg requested changes to D63428: DWARF: Add "dwo_num" field to the DIERef class.
Wed, Jun 19, 3:11 PM · Restricted Project
clayborg added a comment to D63540: Fix lookup of symbols at the same address with no size vs. size.

my fix assumes we would have zero sized symbols first in the m_file_addr_to_index map, and the symbols with the same size would be ordered after the ones with zero size. Also, I am assuming when we do:

const FileRangeToIndexMap::Entry *entry = m_file_addr_to_index.FindEntryStartsAt(file_addr);

that it finds the first symbol that starts with "file_addr". If this isn't the case we might need to be able to back up a few symbols to ensure we get consistent results

Wed, Jun 19, 12:27 PM · Restricted Project, Restricted Project
clayborg added a comment to D63540: Fix lookup of symbols at the same address with no size vs. size.

So I am fine with symbols having zero size being in the symbol table. I would be fine not changing anything in the sorting and leaving some symbols with zero size, we just need to fix:

Symbol *Symtab::FindSymbolAtFileAddress(addr_t file_addr);
Wed, Jun 19, 12:23 PM · Restricted Project, Restricted Project

Jun 18 2019

clayborg added a comment to D63104: Add GSYM utility files along with unit tests..

Marked things as done.

Jun 18 2019, 8:40 AM · Restricted Project
clayborg updated the diff for D63104: Add GSYM utility files along with unit tests..

Converted AddressRange objects into a class so we can ensure consistent and correct results when using accessors.

Jun 18 2019, 8:40 AM · Restricted Project
clayborg added a comment to D63491: DWARF: Use a more compact internal representation in the manual dwarf index.

Ok, sounds good. In that case, I think I can squeeze that change into the previous patch in this series.

Jun 18 2019, 8:16 AM
clayborg added a comment to D63491: DWARF: Use a more compact internal representation in the manual dwarf index.

I made this depend on the DIERef patch instead of the other way around because the cleaner separation between dwo identifiers and compile unit offsets implemented in that patch makes it easier to implement this. It would still be possible to implement it the other way around, but the logic would be more complex, and it would have to be redone anyway once the DIERef patch lands.

Another possibility would be to just remove the unit_offset field from the DIERef class, and have the manual index keep using DIERefs. Once we introduce the dwo identifier, the unit offset is strictly optional, as the unit is uniquely identified by the other coordinates.

Jun 18 2019, 7:14 AM

Jun 17 2019

clayborg added a comment to D63104: Add GSYM utility files along with unit tests..

Marking inline comments as done and mentioning that I did add tests for FunctionInfo operator < as well for cases where things have inline info and line entries

Jun 17 2019, 10:56 AM · Restricted Project
clayborg updated the diff for D63104: Add GSYM utility files along with unit tests..
Jun 17 2019, 10:52 AM · Restricted Project
clayborg added a comment to D63399: DWARF: Make DIERefs always valid.

I am concerned that our mapping from DIERef to lldb::user_id_t won't work for all cases now that we are/have expanded the DIERef class (including as we add the DWO field). I voiced this concern in https://reviews.llvm.org/D63428. Let me know what you think.

Jun 17 2019, 9:10 AM · Restricted Project
clayborg added a comment to D63428: DWARF: Add "dwo_num" field to the DIERef class.

I am concerned with increasing the size of DIERef objects. If we go to 12 bytes per DIERef, and we mostly store these in NameToDIE in "lldb_private::UniqueCStringMap<DIERef> m_map;" maps, which contain a std::vector of UniqueCStringMap::Entry objects which are:

Jun 17 2019, 8:45 AM · Restricted Project
clayborg accepted D63400: DWARF: Provide accessors to DIERef fields.
Jun 17 2019, 8:01 AM · Restricted Project
clayborg added a comment to D63400: DWARF: Provide accessors to DIERef fields.

Just questioning if we want to increase the size of the DIERef struct.

Jun 17 2019, 7:26 AM · Restricted Project

Jun 14 2019

clayborg added a comment to D63357: [Process] Remove unused field from HistoryThread.

Added Jason and Jim. Not sure if all libdispatch stuff is in llvm.org? Either Jim or Jason should ok this just in case they still use this in Apple specific repos.

Jun 14 2019, 2:52 PM · Restricted Project
clayborg added reviewers for D63357: [Process] Remove unused field from HistoryThread: jasonmolenda, jingham.
Jun 14 2019, 2:52 PM · Restricted Project
clayborg committed rG6df47ef22b26: Don't try to parse ObjC method if CU isn't ObjC (authored by clayborg).
Don't try to parse ObjC method if CU isn't ObjC
Jun 14 2019, 12:17 PM
clayborg accepted D62501: Implement GetSharedLibraryInfoAddress.
Jun 14 2019, 10:58 AM · Restricted Project, Restricted Project
clayborg added a comment to D63166: Move common functionality from processwindows into processdebugger.

I would rather see this stuff just moved into NativeProcessWindows (or some class in the windows lldb-server plug-in) and have all code in ProcessWindow.cpp and ProcessDebugger.cpp go away once lldb-server works on windows? The goal is to get rid of the ProcessWindows.cpp once lldb-server is running. Better to not maintain two different plug-ins (one for native and one for remote). The remote version will always work less well in that case.

Jun 14 2019, 10:43 AM · Restricted Project
clayborg accepted D63322: DWARF: Avoid storing DIERefs in long-lived containers.
Jun 14 2019, 10:34 AM · Restricted Project
clayborg added inline comments to D63104: Add GSYM utility files along with unit tests..
Jun 14 2019, 10:31 AM · Restricted Project
clayborg updated the diff for D63104: Add GSYM utility files along with unit tests..

Address review comments:

  • make all comment block doxygen blocks
Jun 14 2019, 10:28 AM · Restricted Project

Jun 13 2019

clayborg added a comment to D63165: Initial support for native debugging of x86/x64 Windows processes.
In D63165#1540606, @Hui wrote:

Sorry for the stupid question, but ...

What exactly is meant here by "Native"? How is a NativeProcessWindows different from ProcessWindows?

The Native*** classes are meant to be used from lldb-server. They look somewhat similar to their non-native counterpart because they still do debugging, but they're a lot dumber, because they only deal with basic process control, and none of the fancy symbolic stuff that you'd need debug info for.

They differ in APIs but most of them have common implementations. The APIs from native process classes are more easy to apply process/thread control.
Hope the native and non-native ones can be merged. The similar thing to the RegisterContext and NativeRegisterContext classes.

The other thing is that using "native" classes can avoid linking a lot of unnecessary lldb libs (LLDB plugins or whatever comes with the plugins) to lldb-server.
The nativeprocesswindows could just be a pass-through to processwindows plugin, but the usage is a sort of strange since the
lldb-server needs to initialize the plugin, create a target, and create a instance just like what lldb does. This means literally
there will be two lldb debuggers, one on host and the other one on remote. It is doable, but not that applicable.

Jun 13 2019, 8:59 AM · Restricted Project
clayborg accepted D63268: Make UniqueCStringMap work with non-default-constructible types and other improvements/cleanups.
Jun 13 2019, 8:49 AM · Restricted Project
clayborg updated the diff for D63104: Add GSYM utility files along with unit tests..

Change:

uint32_t llvm::gsym::StringTable::find(StringRef Str) const;

to return optional:

llvm::Optional<uint32_t> llvm::gsym::StringTable::find(StringRef Str) const;

as suggested.

Jun 13 2019, 8:42 AM · Restricted Project
clayborg updated the diff for D63171: Don't try to parse ObjC method if CU isn't ObjC.

Let compiler hoist "cu_language == eLanguageTypeObjC || cu_language == eLanguageTypeObjC_plus_plus" by inlining it into if statement so it is more readable.

Jun 13 2019, 8:32 AM · Restricted Project
clayborg requested changes to D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue.
Jun 13 2019, 8:24 AM · Restricted Project
clayborg added inline comments to D63268: Make UniqueCStringMap work with non-default-constructible types and other improvements/cleanups.
Jun 13 2019, 8:07 AM · Restricted Project

Jun 11 2019

clayborg added a comment to D63171: Don't try to parse ObjC method if CU isn't ObjC.

I suppose one could compile Objective-C code on Linux using GCC.

Jun 11 2019, 4:12 PM · Restricted Project
clayborg added inline comments to D63171: Don't try to parse ObjC method if CU isn't ObjC.
Jun 11 2019, 4:06 PM · Restricted Project
clayborg created D63171: Don't try to parse ObjC method if CU isn't ObjC.
Jun 11 2019, 2:56 PM · Restricted Project
clayborg added a comment to D63165: Initial support for native debugging of x86/x64 Windows processes.

I don't see much I would change here as long as this works and gets tested by the generic GDB remote protocol testing? Any others have comments?

Jun 11 2019, 1:59 PM · Restricted Project
clayborg updated the diff for D62756: Be consistent when adding names and mangled names to the index.

Remove commented out code.

Jun 11 2019, 11:08 AM
clayborg updated the diff for D62756: Be consistent when adding names and mangled names to the index.

Changed to use ConstString in AddMangled which simplifies the logic quite a bit. I verified performance didn't regress.

Jun 11 2019, 10:35 AM
clayborg added a comment to D62756: Be consistent when adding names and mangled names to the index.

Being consistent definitely sounds like a good idea. Since this does change behavior somewhat, I'm wondering whether it would make sense to add a test here. The thing that's not clear to me is whether this change in behavior is only theoretical, or if it can also happen in a real-world scenario. We can always hand-construct an DIE which will have a linkage name, but no "normal" name, but I don't see how would this ever happen in practice. How did you initially discover this inconsistency?

Jun 11 2019, 10:35 AM
clayborg added a comment to D63110: Fix a crash in option parsing..

nice

Jun 11 2019, 7:09 AM · Restricted Project, Restricted Project
clayborg accepted D62894: DWARF: Share line tables of type units.
Jun 11 2019, 7:06 AM · Restricted Project

Jun 10 2019

clayborg added a comment to D53379: GSYM symbolication format.

Started the split up with: https://reviews.llvm.org/D63104

Jun 10 2019, 4:02 PM
clayborg created D63104: Add GSYM utility files along with unit tests..
Jun 10 2019, 3:59 PM · Restricted Project
clayborg requested changes to D62894: DWARF: Share line tables of type units.

Still want to resolve getting files from a DWARFUnit a bit better. See inlined comment.

Jun 10 2019, 10:09 AM · Restricted Project

Jun 7 2019

clayborg added a comment to D58678: Improve step over performance by not stopping at branches that are function calls and stepping into and them out of each one.

@clayborg Seems like this still steps into the call if the call is the last instruction in the range. ThreadPlanStepRange::SetNextBranchBreakpoint checks if (last_index - pc_index > 1) before setting the breakpoint. So if last_index == pc_index and pc points to call then the thread plan will resort to single stepping and thus go through all the same machinery. Obviously, this isn't a problem as this just leads to using the same functionality that it used prior to this patch, but you miss out on the optimization you're aiming for.

Jun 7 2019, 2:06 PM · Restricted Project