Page MenuHomePhabricator

clayborg (Greg Clayton)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Today

clayborg committed rG4181bfe6888f: Clarify the "env" launch configuration setting. (authored by clayborg).
Clarify the "env" launch configuration setting.
Mon, Jun 21, 4:11 PM
clayborg closed D104578: Clarify the "env" launch configuration setting..
Mon, Jun 21, 4:11 PM · Restricted Project
clayborg added a comment to D104488: Create synthetic symbol names on demand to improve memory consumption and startup times..

Are the Symbol ID's for unnamed symbols the same each time you read in a symbol file? While the unnamed_symbol symbol names are not significant, it would be good if you were crashing in __lldb_unnamed_symbol111 on one lldb run, you would also crash in the same unnamed symbol when you crashed again.

Mon, Jun 21, 4:08 PM · Restricted Project

Fri, Jun 18

clayborg requested changes to D104422: [trace] Add a TraceCursor class.

Just a few nits!

Fri, Jun 18, 4:28 PM · Restricted Project
clayborg requested review of D104578: Clarify the "env" launch configuration setting..
Fri, Jun 18, 4:21 PM · Restricted Project
clayborg updated the diff for D104488: Create synthetic symbol names on demand to improve memory consumption and startup times..

Fixed a case where the accessor was being used twice in the same function and fixed a typo in a comment.

Fri, Jun 18, 4:04 PM · Restricted Project
clayborg added a comment to D104395: [LLDB][GUI] Add initial forms support.

@clayborg I tried implementing scrolling mechanisms as suggested. My first trial essentially defined a "visible area" rectangle which gets updated with every time the selection changes, then when it comes to drawing, each field checks if it is completely contained in the visible area and draws itself with an offset that we get from from the visible area origin. This worked, but fields that spans multiple columns can completely disappear leaving mostly no fields on the window, so it worked in most cases, but not all. My second trial was about drawing to an ncurses pad that is large enough to fit all contents, then the window is refreshed from that pad. This used manual ncurses calls because we don't support pads at the moment, so I scratched that for now. I think support for pads would be good for those kind of applications in the future. I plan to work on a proposal that would include support for pads and lightweight subwindows, I will detail that later.

Fri, Jun 18, 2:20 PM · Restricted Project
clayborg accepted D104395: [LLDB][GUI] Add initial forms support.

Looks good!

Fri, Jun 18, 1:33 PM · Restricted Project
clayborg added a comment to D104054: [lldb] Enable Rust v0 symbol demangling.

Yes, good to go!

Fri, Jun 18, 1:27 PM · Restricted Project
clayborg requested changes to D104395: [LLDB][GUI] Add initial forms support.

Lets try the diamond character for the boolean stuff unless anyone has any objections. Maybe handle a few more keys for the boolean field as suggested in the comments. This will be good to go after these changes!

Fri, Jun 18, 12:45 PM · Restricted Project
clayborg requested changes to D104422: [trace] Add a TraceCursor class.

Just one issue regarding the stop ID for post mortem. Once this is resolved, we will be good to go!

Fri, Jun 18, 12:04 PM · Restricted Project

Thu, Jun 17

clayborg requested review of D104488: Create synthetic symbol names on demand to improve memory consumption and startup times..
Thu, Jun 17, 2:31 PM · Restricted Project
clayborg accepted D103500: [trace][intel-pt] Create basic SB API.

Looks good!

Thu, Jun 17, 1:57 PM · Restricted Project

Wed, Jun 16

clayborg added inline comments to D104422: [trace] Add a TraceCursor class.
Wed, Jun 16, 9:13 PM · Restricted Project
clayborg requested changes to D103500: [trace][intel-pt] Create basic SB API.

a few nits!

Wed, Jun 16, 6:21 PM · Restricted Project
clayborg accepted D101128: [lldb-vscode] only report long running progress events.
Wed, Jun 16, 6:12 PM · Restricted Project
clayborg added a comment to D104395: [LLDB][GUI] Add initial forms support.

Looks very nice.

Wed, Jun 16, 6:08 PM · Restricted Project
clayborg added inline comments to D104422: [trace] Add a TraceCursor class.
Wed, Jun 16, 5:17 PM · Restricted Project

Fri, Jun 11

clayborg accepted D76877: Implement DW_CFA_LLVM_* for Heterogeneous Debugging.

Accepting since no one else had comments. Sorry for the delay.

Fri, Jun 11, 3:41 PM · debug-info, Restricted Project
clayborg added a comment to D103588: [trace] Create a top level ThreadTrace class.

I mostly commented on ThreadTrace.h and we should resolve the questions in there before reviewing the rest of this patch.

Fri, Jun 11, 2:31 PM · Restricted Project
clayborg accepted D100243: [LLDB][GUI] Expand selected thread tree item by default.

Looks good!

Fri, Jun 11, 2:08 PM · Restricted Project
clayborg requested changes to D100243: [LLDB][GUI] Expand selected thread tree item by default.

Many minor things, but looking good! I look forward to seeing this get checked in

Fri, Jun 11, 12:02 PM · Restricted Project
clayborg accepted D104054: [lldb] Enable Rust v0 symbol demangling.
Fri, Jun 11, 11:22 AM · Restricted Project

Thu, Jun 10

clayborg added a comment to D104054: [lldb] Enable Rust v0 symbol demangling.

Once teemperor's issues are resolved this looks good to me!

Thu, Jun 10, 11:14 PM · Restricted Project

Mon, Jun 7

clayborg requested changes to D103588: [trace] Create a top level ThreadTrace class.

One quick either documentation update for the error string, or switch to ConstString... Probably best to the the error string docs as if we use fancy C++ union stuff it might not compile for everyone on all C++ compilers..

Mon, Jun 7, 10:56 PM · Restricted Project
clayborg requested changes to D103500: [trace][intel-pt] Create basic SB API.
Mon, Jun 7, 10:42 PM · Restricted Project
clayborg requested changes to D103588: [trace] Create a top level ThreadTrace class.

So it would be nice to try and not encode errors into the TraceInstruction class and deal with any errors at decode time.

Mon, Jun 7, 2:56 PM · Restricted Project
clayborg requested changes to D103500: [trace][intel-pt] Create basic SB API.
Mon, Jun 7, 2:48 PM · Restricted Project

Thu, Jun 3

clayborg accepted D103609: [lldb-vscode] Synchronize calls to SendTerminatedEvent.
Thu, Jun 3, 11:18 AM · Restricted Project

Wed, Jun 2

clayborg added a comment to D76877: Implement DW_CFA_LLVM_* for Heterogeneous Debugging.

If no one else comments in the next 2 days, I will accept this patch. So speak up soon if you have an opinion.

Wed, Jun 2, 9:23 PM · debug-info, Restricted Project
clayborg committed rGb0572abf72fd: Improve performance when parsing symbol tables in mach-o files. (authored by clayborg).
Improve performance when parsing symbol tables in mach-o files.
Wed, Jun 2, 10:31 AM
clayborg closed D103504: Improve performance when parsing symbol tables in mach-o files..
Wed, Jun 2, 10:31 AM · Restricted Project

Tue, Jun 1

clayborg requested review of D103504: Improve performance when parsing symbol tables in mach-o files..
Tue, Jun 1, 9:54 PM · Restricted Project
clayborg requested changes to D103376: [lldb] Add SBDebugger APIs for querying logging channels and categories.

Fix the test variable names and this is good to go

Tue, Jun 1, 4:03 PM · Restricted Project
clayborg accepted D103375: [lldb/API] Expose triple for SBProcessInfo..

LGTM

Tue, Jun 1, 3:56 PM · Restricted Project

Thu, May 27

clayborg accepted D102866: [lldb][intel-pt] Remove old plugin.
Thu, May 27, 11:57 AM · Restricted Project

Wed, May 26

clayborg accepted D103209: [lldb] Fix gnu_libstdcpp's update methods.
Wed, May 26, 2:46 PM · Restricted Project
clayborg added a comment to D102829: Add --quiet option to llvm-gsymutil to suppress output of warnings..

@clayborg I am not sure what it means you now accepted the revision? I am still not sure what to do about the tests.

Wed, May 26, 11:44 AM · Restricted Project
clayborg added a comment to D76877: Implement DW_CFA_LLVM_* for Heterogeneous Debugging.

I am ok with all of the DWARF unwind changes. Someone else should give the ok for the MC stuff.

Wed, May 26, 11:17 AM · debug-info, Restricted Project

Mon, May 24

clayborg added a comment to D102993: [lldb] Disable minimal import mode for RecordDecls that back FieldDecls.

Do we want any safeguard on StoredDeclsList::prependDeclNoReplace(...)? Right now it can still cause duplicate NamedDecl pointers. I fix that tested helps prevent this:

Mon, May 24, 2:22 PM · Restricted Project
clayborg added inline comments to D102993: [lldb] Disable minimal import mode for RecordDecls that back FieldDecls.
Mon, May 24, 2:16 PM · Restricted Project

May 21 2021

clayborg accepted D102829: Add --quiet option to llvm-gsymutil to suppress output of warnings..
May 21 2021, 3:36 PM · Restricted Project
clayborg requested changes to D76877: Implement DW_CFA_LLVM_* for Heterogeneous Debugging.
May 21 2021, 3:29 PM · debug-info, Restricted Project
clayborg requested changes to D101128: [lldb-vscode] only report long running progress events.

The only last nit is we are passing the report progress callback around all over the place. I think it would be nicer to just make a static function that we can call and remove the passing of the instance variable for the report callback all over to the event classes.

May 21 2021, 12:11 PM · Restricted Project

May 20 2021

clayborg added a comment to D102866: [lldb][intel-pt] Remove old plugin.

I would be ok with this. Can you track down the original author and add them to the review of this to be sure they aren't using it anymore, and if they need tracing that they have everything they need with your new functionality?

May 20 2021, 2:21 PM · Restricted Project
clayborg requested changes to D102829: Add --quiet option to llvm-gsymutil to suppress output of warnings..

Contents looks good, just need a test to ensure that warnings are quieted. There should be a test for each kind of warning, so it should be easy to run that same command with the --quiet option and verify that the warning does _not_ appear.

May 20 2021, 11:44 AM · Restricted Project

May 19 2021

clayborg added a comment to D102219: Optimize GSymCreator::finalize..

! In D102219#2755080, @simon.giesecke wrote:

May 19 2021, 3:09 PM · debug-info, Restricted Project
clayborg accepted D102224: Add option to llvm-gsymutil to read addresses from stdin..

Looks good to me. Thanks for the changes.

May 19 2021, 3:05 PM · Restricted Project, debug-info
clayborg added inline comments to D76877: Implement DW_CFA_LLVM_* for Heterogeneous Debugging.
May 19 2021, 3:03 PM · debug-info, Restricted Project

May 18 2021

clayborg added a comment to D102634: Calculate indexes of last child of each DWARF entry once during tryExtractDIEsIfNeeded..

What's your use case such that this performance concern has come up for you?

May 18 2021, 4:25 PM · Restricted Project, debug-info
clayborg added inline comments to D76877: Implement DW_CFA_LLVM_* for Heterogeneous Debugging.
May 18 2021, 11:04 AM · debug-info, Restricted Project

May 17 2021

clayborg accepted D102484: Avoid calculating the string hash twice in GsymCreator::insertString..
May 17 2021, 10:59 PM · Restricted Project, debug-info
clayborg added a comment to D102486: Use a non-recursive mutex in GsymCreator..

I would go ahead and request commit access, but at least there is the Phabricator link in the commit that shows that you authored the diff. This is the main reason you want to get commit access, so you everyone can easily see who did the patches.

May 17 2021, 10:58 PM · debug-info, Restricted Project
clayborg added inline comments to D27683: Prepare PrettyStackTrace for LLDB adoption.
May 17 2021, 7:48 PM · Restricted Project
clayborg requested changes to D102634: Calculate indexes of last child of each DWARF entry once during tryExtractDIEsIfNeeded..

You should probably get Phabricator working: https://llvm.org/docs/Phabricator.html

May 17 2021, 7:44 PM · Restricted Project, debug-info
clayborg added reviewers for D102634: Calculate indexes of last child of each DWARF entry once during tryExtractDIEsIfNeeded.: aprantl, JDevlieghere.
May 17 2021, 7:42 PM · Restricted Project, debug-info
clayborg added a comment to D98289: [lldb] 2/2: Fix DW_AT_ranges DW_FORM_sec_offset not using DW_AT_rnglists_base (used by GCC).

No objections from me. Anyone else?

May 17 2021, 5:03 PM · Restricted Project

May 15 2021

clayborg accepted D102562: Fix to the interrupt timeout handling: reset the timeout on re-entry.
May 15 2021, 3:10 PM · Restricted Project

May 14 2021

clayborg accepted D102483: Reformat GSYMCreator.cpp.

Was this all fixed by the auto linter?

May 14 2021, 2:58 PM · debug-info, Restricted Project
clayborg accepted D102484: Avoid calculating the string hash twice in GsymCreator::insertString..
May 14 2021, 2:56 PM · Restricted Project, debug-info
clayborg accepted D102485: Move FunctionInfo in addFunctionInfo rather than copying..
May 14 2021, 2:54 PM · Restricted Project, debug-info
clayborg added a comment to D102487: Avoid underestimating the number of DIEs for a given debug info size..

I had run statistics on a few hundred DWARF files way back when and came up with the original 14-20 byte number, but this was a long long time ago (at least over 10 years). With newer DWARF versions, and with optimizations this can easily change. So it would be great to know what the minimum value should be set to by default. I agree that adding a statistic would be nice so we can track this. I will test some recent DWARF files out and see what my numbers show and report back in this patch.

May 14 2021, 2:51 PM · Restricted Project, debug-info
clayborg accepted D102486: Use a non-recursive mutex in GsymCreator..

GSYM conversion still spends a significant time for locking in GsymCreator::insertString even with only 8 threads, so lock contention seems to be generally an issue here. I was wondering if we could avoid locking the mutex here, but using thread-local tables until finalisation does not seem to be feasible since any thread might insert the same string IIUC and we want to have unique IDs at this point. A lock-free map might be an alternative here, though?

May 14 2021, 2:43 PM · debug-info, Restricted Project
clayborg added a comment to D102486: Use a non-recursive mutex in GsymCreator..

GSYM conversion still spends a significant time for locking in GsymCreator::insertString even with only 8 threads, so lock contention seems to be generally an issue

Could perhaps start by moving the CachedHashStringRef construction outside the lock?

May 14 2021, 2:40 PM · debug-info, Restricted Project

May 12 2021

clayborg committed rGe5bdacba2e18: Optimize GSymCreator::finalize. (authored by clayborg).
Optimize GSymCreator::finalize.
May 12 2021, 3:18 PM
clayborg closed D102219: Optimize GSymCreator::finalize..
May 12 2021, 3:18 PM · debug-info, Restricted Project
clayborg added a comment to D102219: Optimize GSymCreator::finalize..

FWIW, and unrelated to this patch: processing other, more complex gcc-built binaries, e.g. clang, produces a lot of the other warnings (with and without the patch). Not sure if that's concerning.

May 12 2021, 1:19 PM · debug-info, Restricted Project
clayborg added a comment to D102219: Optimize GSymCreator::finalize..

There are unit tests that could be added if you would like to test a bad case, but existing unit tests should still pass with this.

I had a look at the test cases in llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp. The TestDWARF* test cases seem to provide a template for such a test, but unfortunately I am not familiar with the details of DWARF to come up with a representative test case. With some advice, I could do that in another patch?

May 12 2021, 10:56 AM · debug-info, Restricted Project
clayborg added a comment to D102219: Optimize GSymCreator::finalize..

Also note this is my first contribution to LLVM and I don't have commit access, could you land the patch on my behalf?

May 12 2021, 10:52 AM · debug-info, Restricted Project
clayborg added a comment to D102224: Add option to llvm-gsymutil to read addresses from stdin..

I know this isn't covered by tests right now. I am not sure what the policy is for the command line utilities. Currently, llvm-gsymutil doesn't have any tests IIUC. If tests should be added, please direct me to a good example, and I will be happy to add them.

yes tests should be added and there are indeed llvm-gsymutil tests. See this directory:

llvm/test/tools/llvm-gsymutil

Ah, sorry for missing that. It seems so obvious now that I don't know how I missed it.

Hm, clang-tidy complains about two issues in code I copied from llvm-symbolizer. Should these be addressed?

if it is new code, then yes.

One thing to note is that there is library code that tools can directly link to, just like llvm-gsymutil links against the "DebugInfoGSYM" library. Then your lookups are very simple and you would call a function just like doLookup() that you added. Is there a reason you are wanting to run a command line tool instead of linking against the code?

In a first step, we just wanted to switch from one command line tool (addr2line) to another. For some use cases, linking against the library would be an option, and probably indeed one we will take in a future step. In other use cases, this might be less feasible, notably pprof, which is written in Go, which also calls other command line utilities such as nm, addr2line or llvm-symbolizer.

Also, I am not a fan of text scraping from the output of a tool unless it is purely for humans to read. If you are going to use this output from another tools, I would vote to add a new option: "--json" so that the output would be formatted using structured data like JSON. Something like:

$ llvm-gsymutil --json --addresses-from-stdin
0x1000 /tmp/a.gsym
0x2000 /tmp/b.gsym

And this would return output in a specific JSON format, something like:

[
  { "lookupAddress": 4096, "gsym": "/tmp/a.gsym", ... },
  { "lookupAddress": 8192, "gsym": "/tmp/b.gsym", ... }
]

The "..." above would be a JSON version of the llvm::gsym::LookupResult object, or an error message.

I agree that having a JSON output would be better for further processing. I just wanted to keep changes minimal for now, and stick with the output format the tool has. Just after putting up this review, I noticed that https://reviews.llvm.org/D96883 added a --json option to llvm-symbolizer.

We should probably use the same format here.

That makes me wonder whether the direction this takes is actually the right one. Another option might be to add support for GSYM to llvm-symbolizer. Do you think this would be a better direction?

May 12 2021, 10:47 AM · Restricted Project, debug-info

May 11 2021

clayborg accepted D102219: Optimize GSymCreator::finalize..

There are unit tests that could be added if you would like to test a bad case, but existing unit tests should still pass with this.

May 11 2021, 3:02 PM · debug-info, Restricted Project
clayborg added a comment to D102224: Add option to llvm-gsymutil to read addresses from stdin..

On optimization idea is that is one input file is specified, we could specify only addresses in the STDIN? Something like:

May 11 2021, 2:45 PM · Restricted Project, debug-info
clayborg added a comment to D102224: Add option to llvm-gsymutil to read addresses from stdin..

So looks like we just need tests:

  • check error when user specifies --addresses-from-stdin and also an address
  • check error when user specifies --addresses-from-stdin and also an input file
  • check for successful lookups on multiple address + file tuples

And if would be good to specify the input format for the --addresses-from-stdin option in the option description.

May 11 2021, 2:42 PM · Restricted Project, debug-info
clayborg added a comment to D102224: Add option to llvm-gsymutil to read addresses from stdin..

Sounds like, after I reread the description, that we have other tools in the llvm ecosystem that use this <addr> <file> format... Sorry for the noise. I will add inline comments to clarify any needed changes.

May 11 2021, 2:39 PM · Restricted Project, debug-info
clayborg requested changes to D102224: Add option to llvm-gsymutil to read addresses from stdin..

I know this isn't covered by tests right now. I am not sure what the policy is for the command line utilities. Currently, llvm-gsymutil doesn't have any tests IIUC. If tests should be added, please direct me to a good example, and I will be happy to add them.

May 11 2021, 2:37 PM · Restricted Project, debug-info

May 10 2021

clayborg accepted D102085: Add an "interrupt timeout" to Process, fix a bug in handling interrupt timeouts in .

Was that what you had in mind, Greg?

May 10 2021, 3:56 PM · Restricted Project
clayborg added a comment to D102085: Add an "interrupt timeout" to Process, fix a bug in handling interrupt timeouts in .

Thanks for combining the functions and fine not to use Optional is we have a good value to indicate no timeout.

May 10 2021, 11:07 AM · Restricted Project

May 7 2021

clayborg added a comment to D102085: Add an "interrupt timeout" to Process, fix a bug in handling interrupt timeouts in .

I don't see the change as "adding a bunch of timeouts", since what it mostly did was remove a bunch of send_async = false's. For all the clients that don't care about a timeout, I removed the send_async parameter, and for those that did I replaced it with a timeout. There were a few cases at the GDBRemoteClientBase layer where routines were calling the SendPacket routines with send_async = true w/o requesting that info from ProcessGDBRemote. To those I added a timeout parameter. That I consider a plus since it made explicit which packet request functions were interrupting and which were not.

Given the packet requests that called with send_async = false made up ~95% of all uses, it seemed like marking the few uses that did want to interrupt by the fact that they explicitly receive a timeout parameter was cleaner than switching everybody over to a Timeout, and then having 95% of the clients still have to make up an empty Timeout.

Instead, the rule is now "client functions that are going to interrupt the target get passed a timeout, and otherwise you don't have to worry about it". That removed a bunch of boiler-plate and seems clear to me.

We could go further, as you suggest, and add an m_interrupt_timeout to GDBRemoteClientBase (*). That would avoid ever having to pass a timeout into the SendPacket client functions in GDBRemoteClientBase. We already have ValueChanged callbacks for Properties. We'd have to add a virtual "InterruptTimeoutChanged" to Process to do the right thing for ProcessGDBRemote. None of that would be hard to do.

In fact, I actually thought about doing that, but I didn't because I like the fact that from ProcessGDBRemote, you are able to tell which methods in GDBRemoteClientBase interrupt a running process, and which don't by whether they take a timeout or not. If the timeout were in the GDBRemoteClientBase class you wouldn't see that. But that does seems a thing worth knowing. We could preserve that knowledge by adding some naming convention to distinguish between interrupting & non-interrupting methods. But I don't see what that really gains us.

May 7 2021, 3:08 PM · Restricted Project
clayborg added inline comments to D102085: Add an "interrupt timeout" to Process, fix a bug in handling interrupt timeouts in .
May 7 2021, 12:46 PM · Restricted Project
clayborg accepted D101892: [DebugInfo] UnwindTable::create() should not add empty rows to CFI unwind table.

Yes, lgtm!

May 7 2021, 11:08 AM · Restricted Project

May 5 2021

clayborg accepted D101933: If an interrupt fails, don't try to fetch any more packets from the server.
May 5 2021, 2:43 PM · Restricted Project
clayborg added inline comments to D76878: Implement DW_{OP,AT}_LLVM_* for Heterogeneous Debugging.
May 5 2021, 11:48 AM · debug-info, Restricted Project
clayborg added inline comments to D76877: Implement DW_CFA_LLVM_* for Heterogeneous Debugging.
May 5 2021, 11:43 AM · debug-info, Restricted Project
clayborg accepted D101892: [DebugInfo] UnwindTable::create() should not add empty rows to CFI unwind table.

LGTM! Just had one nit code edit suggestion.

May 5 2021, 11:25 AM · Restricted Project

May 4 2021

clayborg accepted D100740: [trace] Dedup different source lines when dumping instructions + refactor.

Just fix the one issue where we use the FileSpec operator== and this is good to go!

May 4 2021, 1:22 PM · Restricted Project
clayborg requested changes to D101128: [lldb-vscode] only report long running progress events.
May 4 2021, 1:11 PM · Restricted Project

May 3 2021

clayborg requested changes to D100740: [trace] Dedup different source lines when dumping instructions + refactor.
May 3 2021, 2:32 PM · Restricted Project
clayborg requested changes to D101128: [lldb-vscode] only report long running progress events.
May 3 2021, 10:59 AM · Restricted Project

Apr 28 2021

clayborg accepted D101131: [lldb-vscode] Follow up of D99989 - store some strings more safely.
Apr 28 2021, 4:35 PM · Restricted Project
clayborg accepted D101406: Rename human-readable name for DW_LANG_Mips_Assembler.

If other architectures are emitting this language for assembly files, then this LGTM.

Apr 28 2021, 3:03 PM · Restricted Project

Apr 26 2021

clayborg requested changes to D101131: [lldb-vscode] Follow up of D99989 - store some strings more safely.
Apr 26 2021, 3:28 PM · Restricted Project
clayborg added a comment to D101237: [lldb] Fix [[no_unique_address]] and libstdc++ 11's std::unique_ptr.

A bit of history on laying out classes: layout used to be a problem before we were able to give all of the field offsets directly to clang to assist in laying out. The main issues were:

  • #pragma pack directives are not in DWARF so we must use the DW_AT_data_member_location
  • Some DWARF optimizations cause class definitions to be omitted and we can have a class A that inherits from class B but we have no real definition for class B, just a forward declaration. In this case we will create a class A that inherits from a class B that has nothing in it, but the field offsets will ensure we show all other instance variables of class A correctly. Furthermore, we have code that can detect when we have such a case and it can grab the right definition for class B when it is imported into another AST, such as when evaluating an expression. This will only happen if class B is in another shared library from class A, and if we do have debug info for class B.
  • any other keywords or attributes such as [[no_unique_address]] that can affect layout that don't appear in DWARF.
Apr 26 2021, 2:59 PM · Restricted Project, Restricted Project, Restricted Project
clayborg added a comment to D101198: [lldb-vscode] Read requests asynchronously.

This will be tricky to do right and I really don't know how much extra performance we will get out of this for all of the possible issues we will can introduce by multi-threading things. That being said, I am happy to help see if this will actually improve performance.

Apr 26 2021, 12:26 PM · Restricted Project

Apr 23 2021

clayborg requested changes to D101128: [lldb-vscode] only report long running progress events.
Apr 23 2021, 12:35 PM · Restricted Project

Apr 22 2021

clayborg added a comment to D96236: [lldb] DWZ 1/9: Pass main DWARFUnit * along DWARFDIEs.

I would be fine with DWARFDie getting bigger to 24B. These objects are used temporarily and not used as part of the storage of all of a DWARFUnit's DIEs. DWARFUnit objects store an array of DWARFDebugInfoEntry objects and those are what we care about not getting bigger.

Apr 22 2021, 3:51 PM · Restricted Project
clayborg added inline comments to D99989: [lldb-vscode] Distinguish shadowed variables in the scopes request.
Apr 22 2021, 11:21 AM · Restricted Project

Apr 21 2021

clayborg added inline comments to D100243: [LLDB][GUI] Expand selected thread tree item by default.
Apr 21 2021, 11:21 AM · Restricted Project
clayborg added a comment to D96236: [lldb] DWZ 1/9: Pass main DWARFUnit * along DWARFDIEs.

Much of this patch requires all places that took a DWARFDie object now also must take an extra parameter. Is there really no way to have DWARFDie be able to discover the main unit via its contained DWARFUnit? DWARFDie objects are transient, so they can contain more information that just what they currently contain:

Apr 21 2021, 10:58 AM · Restricted Project

Apr 20 2021

clayborg added a comment to D100740: [trace] Dedup different source lines when dumping instructions + refactor.

The title should be updated on this diff as well. Set the "json" right now

Apr 20 2021, 5:07 PM · Restricted Project
clayborg requested changes to D100740: [trace] Dedup different source lines when dumping instructions + refactor.
Apr 20 2021, 5:06 PM · Restricted Project