Page MenuHomePhabricator

wallace (walter erquinigo)
User

Projects

User does not belong to any projects.

User Details

User Since
May 10 2016, 10:57 AM (227 w, 5 d)

Recent Activity

Yesterday

wallace updated the summary of D87730: [intel-pt] Pretty print the instruction list.
Sat, Sep 19, 10:10 PM · Restricted Project

Fri, Sep 18

wallace updated the summary of D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..
Fri, Sep 18, 4:17 PM · Restricted Project, Restricted Project
wallace updated the diff for D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..

Based on the discussion from D87335, I'm moving all the JSON utilities as non-member functions to the settings parser file. Eventually this can be refactored if that patch moves forward.

Fri, Sep 18, 4:17 PM · Restricted Project, Restricted Project
wallace added a comment to D87335: [json] Create some llvm::Expected-based accessors.

Thanks for the review. I'll implement this as helper functions in my patch, however it'd be interesting to figure out a way to create this API so that callers don't recreate this functionality. My general understanding is that showing some error message, even if incomplete, is better than showing almost nothing. But I can understand your point.

Fri, Sep 18, 12:44 PM · Restricted Project

Tue, Sep 15

wallace updated the diff for D87730: [intel-pt] Pretty print the instruction list.

remove whitespace changes

Tue, Sep 15, 4:59 PM · Restricted Project
wallace requested review of D87730: [intel-pt] Pretty print the instruction list.
Tue, Sep 15, 4:50 PM · Restricted Project
wallace added a comment to D87335: [json] Create some llvm::Expected-based accessors.

ping

Tue, Sep 15, 10:46 AM · Restricted Project

Sun, Sep 13

wallace requested review of D87589: [intel-pt] Add the instruction decoding functionality.
Sun, Sep 13, 12:48 PM · Restricted Project

Tue, Sep 8

wallace abandoned D86899: [trace][RFC] create skeleton for thread decoder.
Tue, Sep 8, 4:51 PM · Restricted Project
wallace updated the diff for D86670: [intel-pt] Add a basic implementation of the dump command.
  • format
Tue, Sep 8, 4:38 PM · Restricted Project
wallace updated the diff for D86670: [intel-pt] Add a basic implementation of the dump command.

Addresses Greg's comments.

Tue, Sep 8, 4:34 PM · Restricted Project
wallace updated the diff for D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..
  • Moved the JSON changes to another diff.
  • Addressed all the comments so far.
Tue, Sep 8, 4:11 PM · Restricted Project, Restricted Project
wallace updated the summary of D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..
Tue, Sep 8, 4:04 PM · Restricted Project, Restricted Project
wallace requested review of D87335: [json] Create some llvm::Expected-based accessors.
Tue, Sep 8, 4:03 PM · Restricted Project

Fri, Sep 4

wallace added a comment to D86670: [intel-pt] Add a basic implementation of the dump command.

Fair enough. I'll work on a document. Hopefully the feedback is going to be good :)

Fri, Sep 4, 9:13 AM · Restricted Project

Wed, Sep 2

wallace closed D84974: Enable Launching the Debugee in VSCode Terminal.
Wed, Sep 2, 4:55 PM · Restricted Project
wallace updated the diff for D84974: Enable Launching the Debugee in VSCode Terminal.

Now this feature is fully functional. Besiding adding a test, I was able to debug LLDB from an integrated terminal in VSCode. Stdin and stdout work correctly.

Wed, Sep 2, 12:33 PM · Restricted Project

Tue, Sep 1

wallace commandeered D86662: Simplify Symbol Status Message to Only Debug Info Size.
Tue, Sep 1, 3:58 PM · Restricted Project
wallace updated the diff for D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..

fix header c++ declaration

Tue, Sep 1, 3:55 PM · Restricted Project, Restricted Project
wallace updated the diff for D86670: [intel-pt] Add a basic implementation of the dump command.
  • Now using the TraceDumpOptions struct
  • Added support for multiple thread-ids in the dump command. Included a test for this
Tue, Sep 1, 3:51 PM · Restricted Project
wallace updated the diff for D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..

fix nit. Waiting for an additional approval

Tue, Sep 1, 2:33 PM · Restricted Project, Restricted Project

Mon, Aug 31

wallace requested review of D86899: [trace][RFC] create skeleton for thread decoder.
Mon, Aug 31, 4:43 PM · Restricted Project
wallace updated the diff for D86670: [intel-pt] Add a basic implementation of the dump command.

Rebase. Had to add several const qualifiers to match the const of Dump, which is a good thing.

Mon, Aug 31, 12:50 PM · Restricted Project

Sat, Aug 29

wallace updated the diff for D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..
  • Removed whitespace only changes.
  • Simplified the way a SettingsParser is created, now there's no need to passinga a pointer around.
  • Fixed the forward declaration in lldb-private-interfaces.
Sat, Aug 29, 10:25 AM · Restricted Project, Restricted Project

Fri, Aug 28

wallace updated the diff for D86670: [intel-pt] Add a basic implementation of the dump command.

Addressed comments

Fri, Aug 28, 7:24 PM · Restricted Project
wallace updated the diff for D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..

fix typo

Fri, Aug 28, 6:44 PM · Restricted Project, Restricted Project
wallace updated the diff for D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..

Addressed all comments.

  • Wwitched to using llvm::json instead of StructuredData. Fortunately, the logic is pretty much the same.
  • Moved the "triple" field to the process level
Fri, Aug 28, 5:53 PM · Restricted Project, Restricted Project
wallace added a comment to D86670: [intel-pt] Add a basic implementation of the dump command.

I agree with that Greg said.

Fri, Aug 28, 11:26 AM · Restricted Project
wallace added a comment to D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..

You changed my mind. I'll move to Support/JSON.h

Fri, Aug 28, 9:13 AM · Restricted Project, Restricted Project

Thu, Aug 27

wallace updated the diff for D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..

Addressed comments

  • Now using StringRef correctly whenever possible.
  • Revert back to using "traceFile" only inside the thread section. That's how intel-pt works at the moment and, as Greg suggested, we can change the schema in the future if needed.
  • Now the global schema construction inlines the plug-in schema.
  • Simplified the CMake logic
  • Separated the parser from the plugin. Now new plugins need to derive their own Trace implementation along with their own Parser.
  • I'm still using StructuredData for the parsing. There's a chance that once we release this feature users will want support for other formats besides JSON, so for now I prefer to ask for JSON input but parse in a format-agnostic way. If eventually no one needs any other format, we can switch to JSON-only parsing.
  • Also, now the Targets created during parsing are preserved only if the parsing succeeded. Otherwise, the Debugger object is left unchanged.
Thu, Aug 27, 5:53 PM · Restricted Project, Restricted Project
wallace added a comment to D86662: Simplify Symbol Status Message to Only Debug Info Size.

All module information is only displayed in our custom VSCode. Vanilla VSCode doesn't have support for displaying modules.

Thu, Aug 27, 3:46 PM · Restricted Project
wallace accepted D86662: Simplify Symbol Status Message to Only Debug Info Size.
Thu, Aug 27, 2:21 PM · Restricted Project

Wed, Aug 26

wallace added inline comments to D86670: [intel-pt] Add a basic implementation of the dump command.
Wed, Aug 26, 10:52 PM · Restricted Project
wallace added a comment to D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..

Thanks for the review! Those are very useful comments. So, I'll split the parsing out from the Trace object. Regarding a possible inconsistent state, you are right. That could happen. The targets, modules and processes used in the parsing are all created there, and it should be enough to only "commit" those targets if there has been no error. I'll make the necessary updates.

Wed, Aug 26, 10:49 PM · Restricted Project, Restricted Project
wallace added inline comments to D86670: [intel-pt] Add a basic implementation of the dump command.
Wed, Aug 26, 10:40 PM · Restricted Project
wallace requested review of D86670: [intel-pt] Add a basic implementation of the dump command.
Wed, Aug 26, 6:54 PM · Restricted Project
wallace updated the diff for D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..
  • As error messaging is important when parsing, I create a new set of functions in StructuredData for parsing that return an llvm::Expected. This allowed me to move the helper functions for parsing from the Trace file to StructuredData, so that in the future other contributors can

use those methods when needed.

  • I modified the schema to allow for the "traceFile" entry at several levels.
  • "systemPath" was also added for the module entry
  • I didn't use the PlaceholderObject yet, as Greg suggested. That might involve a few other changes and I need to see if that class is sufficient for decoding intel-pt traces. However, as soon as I'm working on the decoding part, I'll move to PlaceholderObject, as it will be nice to avoid failing hard if a module is not available.
  • I simplified the code here and there as well.
  • I'm still using strings for addresses, like "0xFFFFFFFF" or "12312412". After checking some recommendations on stack overflow, using strings seems to be a good practice for having compatibility with JS, which has 54-bit sized integers. I imagine that these trace files might be send over the network and JS could potentially be used then.
Wed, Aug 26, 6:01 PM · Restricted Project, Restricted Project
wallace added inline comments to D86662: Simplify Symbol Status Message to Only Debug Info Size.
Wed, Aug 26, 2:42 PM · Restricted Project
wallace requested changes to D86662: Simplify Symbol Status Message to Only Debug Info Size.

For the record, this came out of discussion in which the consensus was to display less characters and maximize the relevant information, which in this case is just the debug info size.

Wed, Aug 26, 2:16 PM · Restricted Project
wallace added a comment to D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..

Given that this feature is intended to be used by non debugger developers, I want to stick to using JSON as a default input format, as it's more ubiquitous than YAML and I prefer to make this decision based on the user experience than on the technical solution. I like the YAML parser though, but converting JSON to YAML for parsing might throw away relevant error messaging.

Wed, Aug 26, 1:54 PM · Restricted Project, Restricted Project

Tue, Aug 25

wallace added a comment to D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..

That's a very good idea! I'll revisit this patch then. I was not fond of doing the manual parsing but I hadn't found a more automated way.

Tue, Aug 25, 3:48 PM · Restricted Project, Restricted Project

Mon, Aug 24

wallace updated the diff for D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..
  • Added a command that shows the schema of a given trace plugin.
  • Added a test for such command
Mon, Aug 24, 5:18 PM · Restricted Project, Restricted Project
wallace updated the diff for D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..
  • Addressed comments.
  • Added schemas for both the base class and the implementation plugins.
  • Added two tests checking the error messages along with the schema when parsing failed.
  • Made some general clean up of the code
Mon, Aug 24, 4:19 PM · Restricted Project, Restricted Project
wallace added inline comments to D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..
Mon, Aug 24, 4:16 PM · Restricted Project, Restricted Project

Fri, Aug 21

wallace updated the diff for D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..
  • Added libipt as a dependency to build the new intel pt plugin. It's merely a copy paste of what the old intel pt plugin does.
  • Added a test with a sample trace.json definition file. It includes a simple a.out object file, the source and an intel pt trace that goes from main to the final return 0. The test checks that the plugin does the parsing of the structured data correctly and creates the target, process and modules correctly.
  • Added the necessary parsing functionalities in Trace.cpp to make sense of the structured data.
  • Added a simple parsing of the intel pt-specific information in the structured data.
Fri, Aug 21, 12:18 PM · Restricted Project, Restricted Project
wallace commandeered D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..
Fri, Aug 21, 12:10 PM · Restricted Project, Restricted Project

Aug 20 2020

wallace commandeered D84974: Enable Launching the Debugee in VSCode Terminal.

We found a very strange issue with lldb not stopping at any breakpoint after attaching. I'll figure that out

Aug 20 2020, 2:01 PM · Restricted Project
wallace accepted D86261: Add hashing of the .text section to ProcessMinidump..
Aug 20 2020, 12:10 AM · Restricted Project

Aug 19 2020

wallace accepted D86235: Fix swig scripts install target name.
Aug 19 2020, 11:46 AM · Restricted Project

Aug 16 2020

wallace retitled D84974: Enable Launching the Debugee in VSCode Terminal from [WIP] Enable Launching the Debugee in VSCode Terminal to Enable Launching the Debugee in VSCode Terminal.
Aug 16 2020, 11:16 PM · Restricted Project

Aug 12 2020

wallace added a comment to D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages..

Thanks for doing this. There are some basic bits to make this that I was unaware of, so this saves a lot of time for me :)
I'll take over this patch next week and fill it with intel-pt stuff, I already have a good amount of code that is quite mergeable with this patch.

Aug 12 2020, 6:01 PM · Restricted Project, Restricted Project

Aug 4 2020

wallace updated the summary of D85241: [intel-pt] Disable/Enable tracing to guarantee the trace is correct.
Aug 4 2020, 1:27 PM · Restricted Project
wallace updated the diff for D85241: [intel-pt] Disable/Enable tracing to guarantee the trace is correct.

remove an empty line

Aug 4 2020, 1:01 PM · Restricted Project
wallace updated the summary of D85241: [intel-pt] Disable/Enable tracing to guarantee the trace is correct.
Aug 4 2020, 1:00 PM · Restricted Project
wallace requested review of D85241: [intel-pt] Disable/Enable tracing to guarantee the trace is correct.
Aug 4 2020, 12:57 PM · Restricted Project

Aug 3 2020

wallace added a comment to D84555: [lldb-vscode ]Add Syntax Highlighting to Disassembly View.

I don't think there's any easy way to test this. I was reading the VSCode documentation and there's no nice test framework for this. So at this point I'm okay with it without test, as it's very easy to check that it works by visual inspection.

Aug 3 2020, 4:04 PM · Restricted Project, Restricted Project
wallace abandoned D85158: [intel-pt] Refactor 3: refactor PTDecoder into SBPTProcess.

After syncing up with Greg, we decided to redo this in a different way

Aug 3 2020, 3:00 PM · Restricted Project
wallace abandoned D85070: [intel-pt] Refactor 2: create a new folder structure.

After syncing up with Greg, we decided to redo this in a different way

Aug 3 2020, 3:00 PM · Restricted Project
wallace abandoned D85068: [intel-pt] Refactor 1: rename the namespace.

After syncing up with Greg, we decided to redo this in a different way

Aug 3 2020, 3:00 PM · Restricted Project
wallace abandoned D84810: [intel-pt] Simplify Python API configuration.

After syncing up with Greg, we decided to redo this in a different way

Aug 3 2020, 3:00 PM · Restricted Project
wallace abandoned D84791: [intel-pt] Fix python support and add a full python test.

After syncing up with Greg, we decided to redo this in a different way

Aug 3 2020, 3:00 PM · Restricted Project
wallace updated the summary of D85158: [intel-pt] Refactor 3: refactor PTDecoder into SBPTProcess.
Aug 3 2020, 1:53 PM · Restricted Project
wallace requested review of D85158: [intel-pt] Refactor 3: refactor PTDecoder into SBPTProcess.
Aug 3 2020, 1:48 PM · Restricted Project
wallace added a comment to D84555: [lldb-vscode ]Add Syntax Highlighting to Disassembly View.

Move the sample files to the syntaxes folder.
Besides, the path /llvm/syntaxes/disassembly.json should just be syntaxes/disassembly.json. There's no need to have that llvm subfolder there

Aug 3 2020, 1:24 PM · Restricted Project, Restricted Project

Aug 1 2020

wallace requested review of D85070: [intel-pt] Refactor 2: create a new folder structure.
Aug 1 2020, 12:40 AM · Restricted Project

Jul 31 2020

wallace requested review of D85068: [intel-pt] Refactor 1: rename the namespace.
Jul 31 2020, 11:34 PM · Restricted Project

Jul 30 2020

wallace requested changes to D84974: Enable Launching the Debugee in VSCode Terminal.

This is very good for a first implementation. You need to write a test for this as well. Once this works, you have to work on the launcher helper Greg and I mentioned to you that will help you guarantee you can always attach to the target.

Jul 30 2020, 1:15 PM · Restricted Project
wallace retitled D84974: Enable Launching the Debugee in VSCode Terminal from Enable Launching the Debugee in VSCode Terminal to [WIP] Enable Launching the Debugee in VSCode Terminal.
Jul 30 2020, 1:05 PM · Restricted Project

Jul 29 2020

wallace added a comment to D84555: [lldb-vscode ]Add Syntax Highlighting to Disassembly View.

It would be possible as well to have two media types with their corresponding syntax files, one for x86, and another for arm

Jul 29 2020, 2:07 PM · Restricted Project, Restricted Project

Jul 28 2020

wallace requested review of D84810: [intel-pt] Simplify Python API configuration.
Jul 28 2020, 4:25 PM · Restricted Project
wallace added a reviewer for D84791: [intel-pt] Fix python support and add a full python test: kusmour.
Jul 28 2020, 1:09 PM · Restricted Project
wallace updated the diff for D84791: [intel-pt] Fix python support and add a full python test.

fix incorrect comment

Jul 28 2020, 12:59 PM · Restricted Project
wallace added inline comments to D84791: [intel-pt] Fix python support and add a full python test.
Jul 28 2020, 12:46 PM · Restricted Project
wallace requested review of D84791: [intel-pt] Fix python support and add a full python test.
Jul 28 2020, 12:44 PM · Restricted Project

Jul 24 2020

wallace accepted D84555: [lldb-vscode ]Add Syntax Highlighting to Disassembly View.

Very good!

Jul 24 2020, 3:03 PM · Restricted Project, Restricted Project
wallace requested changes to D84555: [lldb-vscode ]Add Syntax Highlighting to Disassembly View.

can you include a screenshot of how the view looked before (without the colors)?

Jul 24 2020, 2:29 PM · Restricted Project, Restricted Project

Jul 20 2020

wallace accepted D83731: Add Debug Info Size to Symbol Status.
Jul 20 2020, 9:16 PM · Restricted Project, Restricted Project

Jul 17 2020

wallace accepted D83731: Add Debug Info Size to Symbol Status.
Jul 17 2020, 4:54 PM · Restricted Project, Restricted Project
wallace requested changes to D83731: Add Debug Info Size to Symbol Status.

The logic looks very good, just some final comments and the code will be high quality

Jul 17 2020, 3:30 PM · Restricted Project, Restricted Project

Jul 16 2020

wallace added a comment to D83731: Add Debug Info Size to Symbol Status.

@clayborg, the tests will fail on linux because the Makefile is expecting a .dsym file. I think it's okay if she does it just for Darwin and then I update the test to cover also linux, as I have my linux already well set up.

Jul 16 2020, 6:08 PM · Restricted Project, Restricted Project
wallace added inline comments to D83731: Add Debug Info Size to Symbol Status.
Jul 16 2020, 4:47 PM · Restricted Project, Restricted Project
wallace closed D83900: [intel-pt] Fix building due to CMake + python changes.

Committed as 4c5d52397e8c8015046ff5541fd0abc738953870

Jul 16 2020, 12:30 PM · Restricted Project

Jul 15 2020

Herald added a project to D83900: [intel-pt] Fix building due to CMake + python changes: Restricted Project.
Jul 15 2020, 12:37 PM · Restricted Project

Jul 13 2020

wallace requested changes to D83731: Add Debug Info Size to Symbol Status.

Please include a test case

Jul 13 2020, 4:51 PM · Restricted Project, Restricted Project

Jul 7 2020

wallace accepted D83306: [lldb/API] Overwrite variables with SBLaunchInfo::SetEnvironment(append=true).

Thanks!

Jul 7 2020, 8:03 AM · Restricted Project

Jun 24 2020

wallace updated the summary of D82477: [lldb-vscode] Add Support for Module Event.
Jun 24 2020, 10:15 AM · Restricted Project, Restricted Project

Jun 17 2020

wallace updated the diff for D81978: Redo of Add terminateCommands to lldb-vscode protocol.

.

Jun 17 2020, 6:22 PM · Restricted Project
wallace updated the diff for D81978: Redo of Add terminateCommands to lldb-vscode protocol.

remove unwanted changes

Jun 17 2020, 6:22 PM · Restricted Project

Jun 16 2020

wallace updated the summary of D81978: Redo of Add terminateCommands to lldb-vscode protocol.
Jun 16 2020, 4:28 PM · Restricted Project
wallace created D81978: Redo of Add terminateCommands to lldb-vscode protocol.
Jun 16 2020, 4:28 PM · Restricted Project
wallace updated the diff for D81200: [vscode] set default values for terminateDebuggee for the disconnect request.

remove unwanted changes

Jun 16 2020, 3:23 PM · Restricted Project
wallace updated the summary of D81200: [vscode] set default values for terminateDebuggee for the disconnect request.
Jun 16 2020, 3:23 PM · Restricted Project
wallace added a comment to D81200: [vscode] set default values for terminateDebuggee for the disconnect request.

@labath , does this look better now?

Jun 16 2020, 1:44 PM · Restricted Project

Jun 15 2020

wallace updated the diff for D81200: [vscode] set default values for terminateDebuggee for the disconnect request.

The tests were weird indeed, I think I had to revisit them after making some changes.
Anyway, I've updated the tests and they do make sense now. They test if the inferior performs some side effect after
it's disconnected.

Jun 15 2020, 11:31 AM · Restricted Project

Jun 11 2020

wallace added inline comments to D81200: [vscode] set default values for terminateDebuggee for the disconnect request.
Jun 11 2020, 3:26 PM · Restricted Project

Jun 10 2020

wallace updated the diff for D81200: [vscode] set default values for terminateDebuggee for the disconnect request.

I updated the tests and did some minor changes.

Jun 10 2020, 11:25 PM · Restricted Project

Jun 4 2020

wallace added a comment to D81200: [vscode] set default values for terminateDebuggee for the disconnect request.

Well, the problem that i've seen happens mostly with long running processes like services that just don't die. So this fixes those issues anyway because those processes are not dying when they should. I tried an older version of the IDE and it sends all the time the terminateDebuggee flag.
Anyway, what you mention made me notice something. This is the existing flow without this change

Jun 4 2020, 6:46 PM · Restricted Project
wallace created D81200: [vscode] set default values for terminateDebuggee for the disconnect request.
Jun 4 2020, 4:36 PM · Restricted Project
wallace added a reviewer for D81200: [vscode] set default values for terminateDebuggee for the disconnect request: kusmour.
Jun 4 2020, 4:36 PM · Restricted Project

May 11 2020

wallace added a comment to D79726: Add terminateCommands to lldb-vscode protocol.

I don't know if terminate is the best name, as it could seem related to the inferior. What about finalize? It reminds me of the try/catch/finalize words, and it's harder to associate it with the inferior's own exit event.

May 11 2020, 11:50 AM · Restricted Project