Page MenuHomePhabricator

clayborg (Greg Clayton)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 23 2014, 10:11 AM (405 w, 1 d)

Recent Activity

Mon, Jun 27

clayborg added inline comments to D126359: [LLDB] Add basic floating point ops to IR interpreter.
Mon, Jun 27, 10:12 AM · Restricted Project, Restricted Project
clayborg added a comment to D126359: [LLDB] Add basic floating point ops to IR interpreter.

Will the floating point emulation be perfect compared to actually JITing the code? If we are going to enable this feature I would like to see tests that actually JIT code do the same operations somehow to verify that things match up with the IR interpreted results produce. The main reason we didn't include floating point code in the IR interpreter before is it is harder to make things match up with exactly how things are going to be run if it were JIT'ed into the process. On x86_64 or x64 there are floating point settings that can be changed and affect how operations happen in the current thread. The main example I always think of is how older debuggers would create recursive descent parsers and evaluate floating point operations using native floating point types. I believe that our APFloat classes can do everything correctly, but I would like to see tests that verify that we get the same results as if we were running actual code in the process. For this you can add an extra function to the lldb/test/API/lang/c/fpeval/main.c source like:

typedef enum FloatOperation { add, subtract, multiply, ... };
double eval(double a, double b, FloatOperation op) {
  switch (op) {
  case add: return a+b;
  case subtract: return a-b;
  ...
  }
}
Mon, Jun 27, 10:02 AM · Restricted Project, Restricted Project
clayborg accepted D128557: [lldb] Add a log dump command to dump the circular log buffer.
Mon, Jun 27, 9:49 AM · Restricted Project, Restricted Project
clayborg added a comment to D128557: [lldb] Add a log dump command to dump the circular log buffer.

Thanks for pointing that out. I blindly copied the categories logic from Log::Disable which uses it when computing the flags. I've omitted it for now because I think it would be weird to set the circular buffer size to 5 and then have 0 messages printed for the process category because 5 log messages from the commands category pushed them out. One solution would be to keep a circular buffer per category. I'll think about it and if I come up with a good solution I'll put up another patch.

Mon, Jun 27, 9:46 AM · Restricted Project, Restricted Project

Fri, Jun 24

clayborg requested review of D128560: An upcoming patch to LLDB will require the ability to decode base64. This patch adds support for decoding base64 and adds tests..
Fri, Jun 24, 3:11 PM · Restricted Project, Restricted Project
clayborg accepted D128453: Automate checking for "command that takes no arguments" being passed arguments in CommandObjectParsed.
Fri, Jun 24, 2:52 PM · Restricted Project, Restricted Project
clayborg added a comment to D128504: debugserver: spawn process in its own process group.

Seems to make sense to me, but I will let Jason Molenda give the final OK.

Fri, Jun 24, 2:50 PM · Restricted Project, Restricted Project
clayborg requested changes to D128557: [lldb] Add a log dump command to dump the circular log buffer.

So we either need to track categories for each buffered log message in the circular buffer so that if the user specifies a category within a channel to dump with "log dump lldb <category>" we can filter out the messages we dump if they are not in the one or more categories that were supplied, or remove categories from the "log dump" command and only specify the channel. Easy to fix the code to work either way.

Fri, Jun 24, 2:40 PM · Restricted Project, Restricted Project
clayborg accepted D128323: [lldb] Add support for specifying a log handler.
Fri, Jun 24, 2:20 PM · Restricted Project, Restricted Project

Thu, Jun 23

clayborg added a comment to D128480: [lldb] Replace Host::SystemLog with Debugger::Report{Error,Warning}.

I don't have anything against this, though I still think the LogHandlers should use the Host layer by making the LogHandler objects into real LLDB plug-ins. But if everyone else feels otherwise, I won't object.

Thu, Jun 23, 5:19 PM · Restricted Project, Restricted Project
clayborg added inline comments to D126254: Add support for decoding base64..
Thu, Jun 23, 4:14 PM · Restricted Project, Restricted Project
clayborg committed rG8b987ca5e37e: Add support for decoding base64. (authored by clayborg).
Add support for decoding base64.
Thu, Jun 23, 4:13 PM · Restricted Project, Restricted Project
clayborg closed D126254: Add support for decoding base64..
Thu, Jun 23, 4:13 PM · Restricted Project, Restricted Project
clayborg added inline comments to D128323: [lldb] Add support for specifying a log handler.
Thu, Jun 23, 3:55 PM · Restricted Project, Restricted Project
clayborg added a comment to D128386: [lldb] Make thread safety the responsibility of the log handlers.

Maybe we should leave the option in the command for history purposes??? Taking it out will now make anyone's log enable lines fail when they didn't use to...

Thu, Jun 23, 2:35 PM · Restricted Project
clayborg added a comment to D128321: [lldb] Add OSLog log handler.

If we do add plug-ins, you could make an Apple specific plug-in named "darwin-os-log" or something that would only be compiled in for Apple builds, or we can do a generic "os-log" plug-in that uses the host layer.

Thu, Jun 23, 1:47 PM · Restricted Project, Restricted Project
clayborg added a comment to D128321: [lldb] Add OSLog log handler.

So it seems like we are catering to our code organization here instead of doing the right thing and relying on the host layer. I would rather move the log code into the host layer or make log handlers actual plug-ins so that we can do the right thing here.

Thu, Jun 23, 1:44 PM · Restricted Project, Restricted Project

Wed, Jun 22

clayborg added inline comments to D128323: [lldb] Add support for specifying a log handler.
Wed, Jun 22, 9:54 AM · Restricted Project, Restricted Project
clayborg added a comment to D128321: [lldb] Add OSLog log handler.

I think we should allow this class to always exist and not conditionally compile it in or out. Then we add a new Host.h method to emit a log message in "lldb/Host/Host.h" and allow each host OS to emit a message. Maybe there is a default implementation that emits the message to stderr, and then we override this for in HostInfoMacOSX.h? Then each OS can use their OS logging API of choice.

Wed, Jun 22, 9:47 AM · Restricted Project, Restricted Project

Tue, Jun 21

clayborg added a comment to D128026: [lldb] Add a BufferedLogHandler.

This is an interesting take that makes more sense than specifying a size in bytes and a "m_circular" since that kind of thing could cut the first message. Saving entire messages as they are logged might make more sense for our circular log messages as we would ensure we have full message lines, though it does come with a "copy each message" cost

Tue, Jun 21, 3:19 PM · Restricted Project
clayborg accepted D127986: [lldb] Support a buffered logging mode.

Looks good!

Tue, Jun 21, 3:11 PM · Restricted Project, Restricted Project
clayborg added a comment to D128069: [lldb] add SBSection.alignment to python bindings.

Sent this via email, but doesn't look like it showed up here:

I'm less concerned about the getter and the property and more about the underlying functionality. It doesn't look like it's being tested, and adding it to the SB API allows to change that.

Just to clarify, are you asking me to add a test for the underlying
functionality, Section.GetLog2Align()? Or is the goal to add the
test at the SBSection level to ensure we detect if someone ever
changes the underlying api?

Tue, Jun 21, 11:32 AM · Restricted Project, Restricted Project

Mon, Jun 20

clayborg accepted D128234: Fix build break introduced by https://reviews.llvm.org/D127702.
Mon, Jun 20, 5:28 PM · Restricted Project, Restricted Project
clayborg accepted D127702: Support logpoints in lldb-vscode.
Mon, Jun 20, 3:32 PM · Restricted Project, Restricted Project

Thu, Jun 16

clayborg added a comment to D127999: [lldb] fix stepping through POSIX trampolines.

LGTM, just a question if we want to restrict this test to linux only.

Thu, Jun 16, 4:20 PM · Restricted Project, Restricted Project
clayborg added a comment to D127986: [lldb] Support a buffered logging mode.

The delegate idea is fine too! No need to overload the base class with extra stuff if not needed. I would just like there to be no predispositions on the kinds of logs that can be used when buffering is enabled.

Thu, Jun 16, 4:18 PM · Restricted Project, Restricted Project
clayborg added a comment to D127986: [lldb] Support a buffered logging mode.

For this idea to work we would need to change the Emit() function to be DoEmit(...), then add logic to LogHandler:

void LogHandler::Emit(StringRef message) {
  if (m_circular) {
    // Fill buffer in circular fashion
    return;
  }
  if (m_buffer_size > 0) {
    if (m_buffer.size() + message.size() > m_buffer_size) {
      // If we exceed the buffer size, flush.
      DoEmit(m_buffer);
      DoEmit(message);
      m_buffer.clear();
    } else {
      // Buffer size not exceeded yet.
      m_buffer += message.str();
    }
  } else {
    DoEmit(message); 
  }
}
Thu, Jun 16, 1:37 PM · Restricted Project, Restricted Project
clayborg added a comment to D127986: [lldb] Support a buffered logging mode.

The main questions is if we want --buffered for any log channel (file, callback, or circular). If we add a --circular flag, then we just let things accumulate in the LogHandler class.

Thu, Jun 16, 1:27 PM · Restricted Project, Restricted Project
clayborg added a comment to D127986: [lldb] Support a buffered logging mode.

I could also imagine having multiple options. For example

Thu, Jun 16, 1:04 PM · Restricted Project, Restricted Project
clayborg added a comment to D127999: [lldb] fix stepping through POSIX trampolines.

Is there a test that was failing with this? If not can we add one?

Thu, Jun 16, 12:59 PM · Restricted Project, Restricted Project
clayborg accepted D127922: [lldb] Introduce the concept of a log handler (NFC).

LGTM!

Thu, Jun 16, 12:57 PM · Restricted Project, Restricted Project
clayborg added inline comments to D127922: [lldb] Introduce the concept of a log handler (NFC).
Thu, Jun 16, 11:23 AM · Restricted Project, Restricted Project
clayborg added inline comments to D127922: [lldb] Introduce the concept of a log handler (NFC).
Thu, Jun 16, 11:05 AM · Restricted Project, Restricted Project
clayborg accepted D127889: [LLDB] Handle DIE with DW_AT_low_pc and empty ranges.
Thu, Jun 16, 10:52 AM · Restricted Project, Restricted Project
clayborg committed rG838a57e1a563: Fix a bug introduced by the move of AddressRanges.h into ADT. (authored by clayborg).
Fix a bug introduced by the move of AddressRanges.h into ADT.
Thu, Jun 16, 10:51 AM · Restricted Project, Restricted Project
clayborg closed D127811: Fix a bug introduced by the move of AddressRanges.h into ADT..
Thu, Jun 16, 10:51 AM · Restricted Project, Restricted Project
clayborg added a comment to D127986: [lldb] Support a buffered logging mode.

Something like:

(lldb) log enable --buffered 100000 lldb process state thread step

If we do this, then we need to be able to specify which buffers to dump somehow. In case someone did:

(lldb) log enable --buffered 100000 lldb ...
(lldb) log enable --buffered 100000 dwarf ...

I the user types "log buffer dump" then all streams would be dumped, maybe with a prefix like:

"lldb" log messages:
...
"dwarf" log messages:
...

Or we might be able to specify one or more channels to get just the ones we want:

(lldb) log buffered dump lldb
(lldb) log buffered dump dwarf
(lldb) log buffered dump lldb dwarf
Thu, Jun 16, 10:45 AM · Restricted Project, Restricted Project
clayborg added a comment to D127986: [lldb] Support a buffered logging mode.

I would vote to add new options to "log enable" and enable any such features on a per "log enable" invocation if possible? Otherwise each time you want to enable this you need to do two commands, and if someone already enables buffered mode, then you wouldn't be able to log anything else? Lets say Xcode always enables logging with something like:

(lldb) log buffered enable 1000000
(lldb) log enable lldb process state thread step

Then the user doesn't know this and they try to enable logging with:

(lldb) log enable lldb expr

Then they currently wouldn't get any logging to the screen? It would be buffered until we dump it with "log buffered dump"?

Thu, Jun 16, 10:41 AM · Restricted Project, Restricted Project
clayborg added inline comments to D127922: [lldb] Introduce the concept of a log handler (NFC).
Thu, Jun 16, 10:34 AM · Restricted Project, Restricted Project

Wed, Jun 15

clayborg updated the diff for D127811: Fix a bug introduced by the move of AddressRanges.h into ADT..

Removed accessors from AddressRange class to keep it immutable.

Wed, Jun 15, 11:23 AM · Restricted Project, Restricted Project
clayborg accepted D126464: [lldb] Add support to load object files from thin archives.

Sorry for the delay, looks good with the test

Wed, Jun 15, 1:01 AM · Restricted Project, Restricted Project

Tue, Jun 14

clayborg requested review of D127811: Fix a bug introduced by the move of AddressRanges.h into ADT..
Tue, Jun 14, 4:48 PM · Restricted Project, Restricted Project
clayborg added a comment to D126254: Add support for decoding base64..

ping

Tue, Jun 14, 2:59 PM · Restricted Project, Restricted Project
clayborg added a comment to D127702: Support logpoints in lldb-vscode.

I would like to get to one array solution of LogMessagePart entries and avoid having two arrays of strings that need to stay in sync (rawTextMessages and evalExpressions). Simpler to understand and no need to document the interdependence of rawTextMessages and evalExpressions.

Tue, Jun 14, 2:45 PM · Restricted Project, Restricted Project

Mon, Jun 13

clayborg added inline comments to D127702: Support logpoints in lldb-vscode.
Mon, Jun 13, 4:23 PM · Restricted Project, Restricted Project
clayborg requested changes to D127702: Support logpoints in lldb-vscode.
Mon, Jun 13, 4:11 PM · Restricted Project, Restricted Project

Tue, Jun 7

clayborg added a comment to D126254: Add support for decoding base64..

Sorry for the back and forth: I find the logic difficult to follow. What would you think of something along these lines (untested) https://godbolt.org/z/vh356fhaP

Tue, Jun 7, 2:04 PM · Restricted Project, Restricted Project

Mon, Jun 6

clayborg added a comment to D48865: [LLDB] CommandObjectThreadUntil::DoExecute() sets the wrong selected thread ID.

Jim Ingham should give any final OK since he is the code owner of the thread plans.

Mon, Jun 6, 1:02 PM · Restricted Project, Restricted Project

Wed, Jun 1

clayborg added a comment to D126059: [Debuginfo][DWARF][NFC] Add paired methods working with DWARFDebugInfoEntry..

Looks good to me, but someone that owns the DWARF parser in LLVM should give the final ok.

Wed, Jun 1, 9:56 PM · Restricted Project, Restricted Project
clayborg added inline comments to D126789: Stop regex commands from double entry into the history.
Wed, Jun 1, 9:52 PM · Restricted Project, Restricted Project

Tue, May 31

clayborg updated the diff for D126254: Add support for decoding base64..

Updated the code to not pass over the input twice. We now check for invalid '=' characters in the decoding loop. Also changed how we strip extra bytes from the end of the decoded input by looking at the back of the Input string since we already detect invalid '=' characters in the main string and would have returned an error already.

Tue, May 31, 10:39 PM · Restricted Project, Restricted Project
clayborg added a comment to D126464: [lldb] Add support to load object files from thin archives.

I have refactored my code so it should looks cleaner now, but I'm not sure how to add a test. It seems that adding a test for thin archive on macOS platforms can be not so straightforward. I see that in lldb/test/API/functionalities/archives/Makefile, the test suite is using libtool instead of ar to create archives on macOS platforms, and I don't know whether I can produce a thin archive with that. Also, ld on macOS platforms seems to be unable to identify thin archives (I'm using ld64.lld when testing my code locally).

I would really appreciate it if someone can provide some advice about how to implement such a test case here, thank you!

Tue, May 31, 9:05 AM · Restricted Project, Restricted Project

May 26 2022

clayborg requested changes to D126464: [lldb] Add support to load object files from thin archives.

A few things to fix but nothing serious. Nice patch! We also need a test for this. I would add a test to lldb/test/API/functionalities/archives/TestBSDArchives.py

May 26 2022, 11:33 AM · Restricted Project, Restricted Project

May 25 2022

clayborg added inline comments to D126254: Add support for decoding base64..
May 25 2022, 4:25 PM · Restricted Project, Restricted Project
clayborg added a comment to D123469: [DebugInfo][llvm-dwarfutil] Combine overlapped address ranges..

address ranges look good to me.

May 25 2022, 3:06 PM · Restricted Project, Restricted Project

May 24 2022

clayborg accepted D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created.

Don't allow setting signal actions by signal number before you have a process.

I understand

We don't know what signal 20 is going to end up being till we have a process, so allowing this by number doesn't make sense.

I am saying that it would be a good idea to make sure an error is returned when you do try and set a signal by number before a process exists and make sure there is a test that covers this if it isn't already in the current patch.

I think we crossed paths. I added that in the last update of the diff. The test is TestHandleProcess.py:20, and the code to enforce it starts around 1671 of CommandObjectProcess.cpp.

May 24 2022, 4:21 PM · Restricted Project, Restricted Project
clayborg added inline comments to D126254: Add support for decoding base64..
May 24 2022, 2:24 PM · Restricted Project, Restricted Project
clayborg updated the diff for D126254: Add support for decoding base64..

Address feedback:

  • Handle any bad '=' characters separately before the main loop.
  • Remember how many bytes to strip after decoding all bytes.
  • Use "Inv" constexpr instead of the magic 64 number to make things more readable
  • Update tests for changes
May 24 2022, 2:23 PM · Restricted Project, Restricted Project
clayborg added a comment to D123469: [DebugInfo][llvm-dwarfutil] Combine overlapped address ranges..

Ok, if we just protect the Collection::iterator stuff in AddessRanges, this will be good to go.

May 24 2022, 1:35 PM · Restricted Project, Restricted Project
clayborg added a comment to D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created.

Don't allow setting signal actions by signal number before you have a process.

May 24 2022, 12:09 PM · Restricted Project, Restricted Project
clayborg accepted D122974: prevent ConstString from calling djbHash() more than once.
May 24 2022, 12:05 PM · Restricted Project, Restricted Project, Restricted Project
clayborg added a comment to D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created.

So the "process handle" command allows us to set signals by signal number as well. Does this patch support this? It seems like it wouldn't be too hard to do if we wanted to handle this. Lemme know what you think, other than that LGTM.

Ah, I forgot about specifying the signal by number. Before you have a process I don't think we should allow signals by number. The mapping signal number -> "signal name for any platform" is not 1-1 so we couldn't guarantee we were doing the right thing here. I'll put in a check for "specified by number with no process" and error out.

May 24 2022, 11:53 AM · Restricted Project, Restricted Project
clayborg accepted D124704: [lldb] save ConstString hash value to cache files too.
May 24 2022, 10:56 AM · Restricted Project, Restricted Project
clayborg added a comment to D123469: [DebugInfo][llvm-dwarfutil] Combine overlapped address ranges..

@clayborg Greg, Could you check&accept this review directly, please?

After requested refactoring is done(https://reviews.llvm.org/D123469?id=423119#inline-1190512)
the AddressRanges class was changed. It combines the ranges like this:

[0x100,0x200) and [0x200, 0x300) -> [0x100, 0x300)

It can affect llvm-gsymutil. I think it should be OK to combine ranges like this. Please check that behavior.

May 24 2022, 10:53 AM · Restricted Project, Restricted Project
clayborg added a comment to D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created.

So the "process handle" command allows us to set signals by signal number as well. Does this patch support this? It seems like it wouldn't be too hard to do if we wanted to handle this. Lemme know what you think, other than that LGTM.

May 24 2022, 10:21 AM · Restricted Project, Restricted Project

May 23 2022

clayborg added inline comments to D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created.
May 23 2022, 6:04 PM · Restricted Project, Restricted Project
clayborg updated the diff for D126254: Add support for decoding base64..

Don't use -1 as the invalid table value, use 64 to avoid triggering narrowing conversion errors.

May 23 2022, 5:00 PM · Restricted Project, Restricted Project
clayborg added inline comments to D126254: Add support for decoding base64..
May 23 2022, 4:41 PM · Restricted Project, Restricted Project
clayborg added reviewers for D126254: Add support for decoding base64.: jvikstrom, hokein.
May 23 2022, 4:19 PM · Restricted Project, Restricted Project
clayborg added a comment to D126059: [Debuginfo][DWARF][NFC] Add paired methods working with DWARFDebugInfoEntry..

A 1 to 2 percent improvement is probably going to go away if the asserts are enabled when it has to do the check each time. If would still hesitate to add this kind of API as it will encourage others to use the APIs. I would be ok with it if the methods are protected or private and if we can add the DWARFLinker as a friend class to allow access to these performant but dangerous APIs only in certain situations that really require it. I really want to avoid most people from being able to use these APIs if possible to make sure we don't end up with bugs. We had many many bugs that stemmed from mismatching DWARFUnit + DWARFDebugIntoEntry before the DWARFDie class was created, mostly when doing cross CU references or in Fission cases where you have the skeleton CU vs the actual DWO CU + a DWARFDebugInfoEntry that comes from the actual DWO file. So there is a real danger to the safety and correctness of the code here.

if all of these bugs could be caught by assertions then the problem probably does not look too severe?
Running debug version of the application will show the problems.

Anyway, if we do not want these methods to be used by others - I am OK to make these methods protected and make DWARFLinker the only one using them.

May 23 2022, 4:14 PM · Restricted Project, Restricted Project
clayborg updated the diff for D126254: Add support for decoding base64..

Removed an extra string that was added to Base64Test::Base64 during testing.

May 23 2022, 4:05 PM · Restricted Project, Restricted Project
clayborg requested review of D126254: Add support for decoding base64..
May 23 2022, 4:03 PM · Restricted Project, Restricted Project
clayborg accepted D122974: prevent ConstString from calling djbHash() more than once.
May 23 2022, 1:13 PM · Restricted Project, Restricted Project, Restricted Project
clayborg accepted D124704: [lldb] save ConstString hash value to cache files too.

One small nit in the code you can choose to fix or not. LGTM. Thanks for the improvements!

May 23 2022, 1:10 PM · Restricted Project, Restricted Project
clayborg accepted D126225: Fix lldb-vscode frame test failure.

Since this "optimized" value isn't part of the actual JSON schema for the stack frames, and since we aren't using it in the IDE, best to remove this until it is either in the protocol definition or used by the IDE.

May 23 2022, 1:03 PM · Restricted Project, Restricted Project
clayborg added a comment to D126059: [Debuginfo][DWARF][NFC] Add paired methods working with DWARFDebugInfoEntry..

A 1 to 2 percent improvement is probably going to go away if the asserts are enabled when it has to do the check each time. If would still hesitate to add this kind of API as it will encourage others to use the APIs. I would be ok with it if the methods are protected or private and if we can add the DWARFLinker as a friend class to allow access to these performant but dangerous APIs only in certain situations that really require it. I really want to avoid most people from being able to use these APIs if possible to make sure we don't end up with bugs. We had many many bugs that stemmed from mismatching DWARFUnit + DWARFDebugIntoEntry before the DWARFDie class was created, mostly when doing cross CU references or in Fission cases where you have the skeleton CU vs the actual DWO CU + a DWARFDebugInfoEntry that comes from the actual DWO file. So there is a real danger to the safety and correctness of the code here.

May 23 2022, 1:00 PM · Restricted Project, Restricted Project

May 20 2022

clayborg added a comment to D126059: [Debuginfo][DWARF][NFC] Add paired methods working with DWARFDebugInfoEntry..

I would actually request that no one can play with DWARFDebugInfoEntry items at all except for the DWARFDie class. The main issue is you can ask the wrong DWARFUnit class to do something with a DWARFDebugInfoEntry object that it doesn't own. We ran into this problem in LLDB and this is why we created the DWARFDie class in LLDB. If you encapsulate everything into the DWARFDie class and control all entry points, then this mistake can not be made since everyone needs to use the DWARFDie class. If you do want it add it, I would suggest adding an assert that verifies that the DWARFDebugInfoEntry belongs to the current DWARFUnit in each new function that takes a "DWARFDebugInfoEntry *", but I would still rather see everyone use DWARFDie if possible for safety reason and to avoid having an assert in each call.

May 20 2022, 2:17 PM · Restricted Project, Restricted Project
clayborg accepted D126013: Add [opt] suffix to optimized stack frame in lldb-vscode.
May 20 2022, 1:58 PM · Restricted Project, Restricted Project

May 19 2022

clayborg added inline comments to D126013: Add [opt] suffix to optimized stack frame in lldb-vscode.
May 19 2022, 2:08 PM · Restricted Project, Restricted Project
clayborg requested changes to D126013: Add [opt] suffix to optimized stack frame in lldb-vscode.
May 19 2022, 2:07 PM · Restricted Project, Restricted Project
clayborg accepted D126014: Show error message for optimized variables.

A little background: we have people that end up debugging optimized code and when a variable's SBValue had an error, we wouldn't show anything in the variables window for the value.

May 19 2022, 2:01 PM · Restricted Project, Restricted Project

May 18 2022

clayborg accepted D125935: Fix if statement in DebugInfo/GSYM/LookupResult.cpp.
May 18 2022, 4:59 PM · Restricted Project, Restricted Project
clayborg accepted D125325: Pass plugin_name in SBProcess::SaveCore.

Thanks for the changes. LGTM

May 18 2022, 2:15 PM · Restricted Project, Restricted Project

May 13 2022

clayborg requested changes to D125325: Pass plugin_name in SBProcess::SaveCore.

Looking great, just a few more fixes. Thanks for making the changes.

May 13 2022, 1:36 PM · Restricted Project, Restricted Project

May 12 2022

clayborg added a comment to D125325: Pass plugin_name in SBProcess::SaveCore.

It might be a good idea to add a test for this. There seems to be a test already in this file:

lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py

In fact looking at this test, it seems they specify the "SaveCoreStyle" via an option. As long as we are adding new APIs to SBProcess, we should probably add the "SaveCoreStyle style" as a parameter to the new SaveCore, this enum is already in the public header file (lldb-enumerations.h).

May 12 2022, 4:00 PM · Restricted Project, Restricted Project
clayborg added a comment to D125322: [llvm][json] Fix UINT64 json parsing.

LGTM, but I don't own the LLVM JSON parser, so someone else should give the final ok!

May 12 2022, 3:54 PM · Restricted Project, Restricted Project

May 11 2022

clayborg committed rG91d5bfdb7996: Add "indexedVariables" to variables with lots of children. (authored by clayborg).
Add "indexedVariables" to variables with lots of children.
May 11 2022, 4:18 PM · Restricted Project
clayborg closed D125347: Add "indexedVariables" to variables with lots of children..
May 11 2022, 4:18 PM · Restricted Project, Restricted Project
clayborg added inline comments to D125347: Add "indexedVariables" to variables with lots of children..
May 11 2022, 3:29 PM · Restricted Project, Restricted Project
clayborg updated the diff for D125347: Add "indexedVariables" to variables with lots of children..

Remove logging change from VSCode.cpp that was accidentally left in.

May 11 2022, 3:26 PM · Restricted Project, Restricted Project
clayborg accepted D125073: [lldb-vscode] Fix data race in lldb-vscode when running with ThreadSanitizer.

Much easier and cleaner. LGTM

May 11 2022, 3:19 PM · Restricted Project, Restricted Project
clayborg added a comment to D96035: [dsymutil][DWARFlinker] implement separate multi-thread processing for compile units..

probably, we would like to use both dwarflinkers with dsymutil and dwarfutil. Thus, we do not want to name tools in the option. How about these variants:

--dwarf-linker={classic,mt}  (mt is multy-thread)

--dwarf-linker={classic,with-joined-types}

--dwarf-linker={classic,experimental}
May 11 2022, 3:18 PM · Restricted Project, Restricted Project
clayborg added inline comments to D125325: Pass plugin_name in SBProcess::SaveCore.
May 11 2022, 3:04 PM · Restricted Project, Restricted Project

May 10 2022

clayborg added reviewers for D125347: Add "indexedVariables" to variables with lots of children.: labath, jingham, yinghuitan, wallace, aadsm.
May 10 2022, 5:04 PM · Restricted Project, Restricted Project
clayborg requested review of D125347: Add "indexedVariables" to variables with lots of children..
May 10 2022, 5:02 PM · Restricted Project, Restricted Project
clayborg accepted D125107: [lldb] Parallelize fetching symbol files in crashlog.py.

lgtm!

May 10 2022, 1:36 PM · Restricted Project
clayborg added a comment to D96035: [dsymutil][DWARFlinker] implement separate multi-thread processing for compile units..

My main concern with creating one large unit for all types is the performance of the debugger when consuming this, but we don't need to fix this in the first iteration and can work on improving this later after we prove it is a problem. As long as the DWARF is in good shape and we are properly uniquing types in a much better way than before I am good.

May 10 2022, 1:25 PM · Restricted Project, Restricted Project
clayborg added a comment to D125073: [lldb-vscode] Fix data race in lldb-vscode when running with ThreadSanitizer.

Another solution would be to make m_thread_should_exit a std::atomic<bool> type instead of just a bool.

May 10 2022, 1:23 PM · Restricted Project, Restricted Project
clayborg requested changes to D125325: Pass plugin_name in SBProcess::SaveCore.

We can't change any public API calls since other tools might link against the existing API, but we can add new variants to the API. Inline fixes have been suggested.

May 10 2022, 1:16 PM · Restricted Project, Restricted Project

May 9 2022

clayborg committed rG879a47a55ffb: Add the ability to debug through an exec into ld (authored by clayborg).
Add the ability to debug through an exec into ld
May 9 2022, 4:08 PM · Restricted Project