Improve debugging support #8538

skuznetsov · 2019-11-30T05:05:26Z

Crystal debug support is here. It allows LLDB to show Crystal variables properly in C lang style.

…iables properly in C lang style. To work properly some LLDB patching is needed due to the dereferencing bug in DWARF Expression Evaluator.

ysbaddaden

I'm afraid there is no benefit here, except disastrous performance hit by default because of a forced NoInline.

Oh, maybe LLDB is stupid and doesn't try to behave correctly with an unknown identifier, unlike GDB?

src/compiler/crystal/codegen/fun.cr

src/llvm/di_builder.cr

src/compiler/crystal/codegen/debug.cr

asterite

Looks great!

I didn't try it, because I will need to compile LLVM with a patch to try it out, but I trust OP and in any case debugging support isn't great right now and looking at all the progress they have shown in the forums makes me think this is really good.

ysbaddaden · 2019-12-04T13:07:33Z

I.e. I believe LLDB is buggy and too picky. Use GDB it will just work.

skuznetsov · 2019-12-04T13:53:12Z

Sadly GDB does not work on macOS properly. It just hangs badly. I reinstalled it many times and even reinstalled OS and it is still hanging. So for me LLDB is just the option All the Best! Sergey

…

On Dec 4, 2019, at 8:07 AM, Julien Portalier ***@***.***> wrote: I.e. I believe LLDB is buggy and too picky. Use GDB it will just work. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

ysbaddaden · 2019-12-04T15:27:30Z

I don't use LLDB. I can't really help or provide guidance here —except for properly declaring debug type definitions, which we're lacking right now, so all debuggers would benefit.

skuznetsov · 2019-12-04T15:31:37Z

I am looking into enrichment of DWARF info for Crystal.

RX14 · 2019-12-05T01:57:11Z

If we make crystal executables pretend their source code is C++ then no debug tool can ever add crystal-specific debugging support! They won't know which executable is Crystal and which is C++. This is clearly not beneficial long-term.

I too, suggest you file at LLDB and close this.

Again, a macos problem that you can't use gdb! Crystal was developed on macos and it still works far better on linux...

skuznetsov · 2019-12-05T02:02:01Z

@RX14 As I said earlier this is a temporary solution to piggyback on C++ type system until proper Crystal type system will be implemented for LLDB. I also have to look how DWARF information is used in GDB to see if it useful enough for Crystal.

No one from outside of Crystal community will implement Crystal support. This is our responsibility and I am planning to implement it for LLDB, using C++ and Go Type systems as an example and boilerplate.

RX14 · 2019-12-05T11:42:27Z

But as soon as you want to implement proper LLDB support, you will have to revert this change. This means that all the existing LLDB versions which won't be upgraded for years after you upstream your patch will suddenly break.

I whole-heartedly support adding support to LLDB, but this is a step backwards from that. This makes adding LLDB support harder, so it cannot be accepted.

The suggestion to file upstream was to discuss treating unknown DWARF languages as C++, as gdb does, not to ask them to support crystal properly. That'd be far too much to ask.

skuznetsov · 2019-12-05T11:58:11Z

@RX14 : It does makes sense.

We still have to get official Language ID to use for the language though and it will break versions support either as we use user defined (0x8002) (not official those are below 0x8000) Language ID.

Also at least one of the changes still needs to be accepted. The one where I replace 0 with false to do proper Optimisation setting parameter on DWARF Compile Unit.

RX14 · 2019-12-05T12:07:26Z

@skuznetsov the other changes are great, thanks.

Still, you cannot even develop the support without changing the language ID to something other than C++. I'm trying to understand why you're advocating for this despite making your job harder.

ysbaddaden · 2020-01-20T08:15:52Z

Now it's starting to look interesting, with proper DWARF type definitions :)

src/compiler/crystal/program.cr

RX14 · 2020-01-20T10:06:03Z

FWIW, I'm happy to merge a change to the debugging type to C++ if you speak to the LLVM people and they determine that this is the best way forward. I'd really like to see a path towards getting our own official DWARF type laid out as part of that.

RX14 · 2020-01-20T12:01:24Z

Actually, I'll be at FOSDEM in 2 weeks and i'll try and find one of the LLDB/GDB people there and ask

skuznetsov · 2020-01-20T12:31:54Z

FWIW, I'm happy to merge a change to the debugging type to C++ if you speak to the LLVM people and they determine that this is the best way forward. I'd really like to see a path towards getting our own official DWARF type laid out as part of that.

It is still nice to have our official DWARF Language ID but for now to keep C++ identifier until we implement LLDB and GDB plugins for Crystal language support.

skuznetsov · 2020-01-20T12:42:48Z

Actually, I'll be at FOSDEM in 2 weeks and i'll try and find one of the LLDB/GDB people there and ask

Can you tell LLDB team that they have a bug in dereferencer that I pointed on the forum it will be great! Otherwise my changes will be neat but not much useful for Primitive types as they bill be shown incorrectly.
Here is the link to explanation: https://forum.crystal-lang.org/t/debugger-support/1325/34?u=computermage

skuznetsov

I just republishing the issue link for request of official DWARF language ID that I posted on Crystal forum: http://dwarfstd.org/ShowIssue.php?issue=200120.1

RX14 · 2020-01-21T12:45:13Z

Can you tell LLDB team that they have a bug in dereferencer that I pointed on the forum it will be great!

You should just submit that upstream to LLVM: https://llvm.org/docs/Contributing.html

skuznetsov · 2020-01-21T12:49:37Z

Thanks for the link! Will patch it and will send the patch to them.

… well as fixed line linfo for blocks.

ysbaddaden · 2020-01-30T13:33:23Z

Note that all the LLVMDI functions are official LLVM C API since LLVM 7. The bindings should be declared in lib_llvm.cr instead of lib_llvm_ext.cr.

It has some consequences that should be discussed: with these, Crystal will now depend on LLVM 7+ when it currently supports down to LLVM 3.x.

Maybe dropping support for anything below LLVM 7 is fine —that would really clean things up. Alternatively, we can polyfill the LLVMDI functions for a few older LLVM versions, but is it worth it? Or maybe just not enable full debug support when Crystal is built against LLVM 6 and below?

… debug

Generate a lldb script from magic comments and check the output using FileCheck

bcardiff · 2020-04-09T01:01:35Z

Hey @skuznetsov I've been playing a bit today with this :-)
It took me a while to understand that I should use the native lldb and not the one from homebrew.

Otherwise I was getting

lldb module importing failed: This script interpreter does not support importing modules

Or

error: module importing failed: loading unimplemented

Depending on llvm version.

Either way, in mentioned before that to incorporate these changes and avoid breaking the funcionality in the future there needs to be some kind of specs.

I merged master and apply a couple of changes to start with that story in master...bcardiff:debug-specs

The interesting commit is bcardiff@3291ec8

It is inspired in rust runner and specs.

What do you think about this using this structure to show the scenarios improved by this PR?

The spec/debug/strings.cr for example will not pass if the crystal_formatters.py is not imported 😎

skuznetsov · 2020-04-09T02:05:52Z

@bcardiff Yeah, I found it frustrating as well that Homebrew versions are not compiling it with proper Python support. Initially I built my own LLDB version with embedded python scripting so this Homebrew issue didn't hit me initially. I found that builtin Xcode version /usr/bin/lldb works just fine but it monolith version and does not have liblldb.dylib library so because of that CodeLLDB does not support scripting and my formatters as well :(
Did you try to play with it in system LLDB CLI? How do you like it so far?

I will look into your debug spec helpers. I will see what I can implement.

bcardiff · 2020-04-09T14:15:04Z

Using CodeLLDB with

#setting.json
    "lldb.launch.initCommands": [
        "command script import ~/path/to/crystal_formatters.py"
    ]

worked fine here

Although it seems that CodeLLDB bundles an lldb itself:

version
lldb version 10.0.0 (local revision aaae14ad1e80b489efe194b068b4b5d337ded6c8)
  clang revision aaae14ad1e80b489efe194b068b4b5d337ded6c8
  llvm revision aaae14ad1e80b489efe194b068b4b5d337ded6c8
  rust-enabled

And that does not match the native one, nor the homebrew

$ /usr/bin/lldb 
(lldb) version
lldb-1100.0.30.12

$ /usr/local/opt/llvm\@9/bin/lldb
(lldb) version
lldb version 9.0.1

$ /usr/local/opt/llvm\@10/bin/lldb
(lldb) version
lldb version 10.0.0

RX14 · 2020-04-09T15:27:50Z

I found it frustrating as well that Homebrew versions are not compiling it with proper Python support

Could this be noted/tweaked/fixed upstream? Might want to bug homebrew about that.

… debug

bcardiff · 2020-04-14T18:29:37Z

Could this be noted/tweaked/fixed upstream? Might want to bug homebrew about that.

It is a known thing it seems in llvm https://lldb.llvm.org/resources/caveats.html and there are a couple of issues in homebrew-legacy related to formula options that are no longer available in homrebrew/core. I could not find a more recent issue in the core repo related to this. It seems like and old issue already settle in the current status to me.

RX14 · 2020-04-14T19:28:25Z

Any chance of getting a review?

asterite

This looks good. It seems to improve the current debugging landscape. Is it perfect or does it have all the necessary specs? No. But since debugging right now is pretty much barebones and this is a solid improvement, I think we should merge this. Tests can come later, either from @skuznetsov or from someone else.

… debug

bcardiff

At least there are some specs ;-)

Without more specs for debugging, it will be really hard to accept further contributions in a confident manner.

The code changes seems fine to me regarding the existing compiler design. But the heavy part of the additions are hard to grasp whether they are still working without manually verifying.

So, after my small testing and contribution for the specs I'm fine merging this, but I would prefer if there were some more specs to ensure we don't go backward. Maybe merging this can encourage others to script their own lldb sessions, but I am not very confident (#ProveMeWrongPlease).

bcardiff · 2020-04-14T20:47:02Z

src/compiler/crystal/codegen/codegen.cr

+            # Naked functions must not have debug info associated with them
+            false
+          else
+            declare_variable(var.name, var.type, pointer, var.location)


Other invocations of declare_variable occur only if if @debug.variables?. With than in mind there are some refactor that can be applied since that method now does a noop unless @debug.variables?

bcardiff · 2020-04-16T18:47:06Z

Thanks @skuznetsov and others for this.

Let's keep in mind the need for some specs in order to move forward and don't lose support for debugging upon compiler/codegen changes.

jkthorne · 2020-04-16T18:54:47Z

thank you @skuznetsov !!

RX14 · 2020-04-16T19:13:47Z

Thank you so much!!

bcardiff · 2020-04-17T21:13:01Z

For those interested in trying this with VSCode I drop the configuration I've been using at https://dev.to/bcardiff/debug-crystal-in-vscode-via-codelldb-3lf

skuznetsov · 2020-04-18T17:04:01Z

For those interested in trying this with VSCode I drop the configuration I've been using at https://dev.to/bcardiff/debug-crystal-in-vscode-via-codelldb-3lf

Sometimes Debugger values dereferencing is become too burdensome as it is gooing too deep on the parsed tree. In that case I recommend to disable "Dereference Pointers" feature:

It solves occasional hangups when LLDB is going way too deep.

@Sija

* Rudimentary debug support is here. It allows LLDB to show Crystal variables properly in C lang style. To work properly some LLDB patching is needed due to the dereferencing bug in DWARF Expression Evaluator. * Rolled back enforced NoInline in debug mode. * Added comment to explain why we have to use C++ Language ID * Added debug information for union and nullable types. * Fixed some of the debug type glitches (MixedUnionType notoriously) as well as fixed line linfo for blocks. * Refactored debug information logging as per bcardiff comments * Added array types debugging with DWARF support. * Some extra features was added to llvm module so now it is possible to access to any block or any instruction in the LLVM function. Also some refactoring as done to show properly locations of block variables. * Added debug support for Tuples, NamedTuples, TypeDefs and partial support for Symbols. * Removed unnecessary debug logs and did some format tidy up. * More of code tidy up. * Moving LLVM functions to the class they belong to. * Neating up the code as per comments of reviewers. * Code cleanup as per Sija's comments. * Moved lldb crystal formatter into etc/lldb Added get_or_create_array_subrange_variable method to LLVM module. * Rolled back previously set_current_debug_location as they are breaking unit tests and not really beneficial. * Fixes as per Sija's suggestions as well as fix for failed unit test due to the missed filename of the unit test * autoformated source code * Removed location code that is not needed and was redundant and also was breaking the unit tests. * Code clean up as per Sija's comments. * Typo fix! Thanks a lot, Sija! * Some clean up to trigger the build to re-test Alpine Linux. * Re-trigger build wth comment commit. * Fixed formatting issues (who knew that ### is not allowed! ) * initial refactor to use proper debug alloca * fixing of alloca to be on alloca block * formatted code * Bug fixing for proper debug implementation. It should work properly now for most of variables and "self" variable. * Fix of code formatting. * Code fixes and avoid generating debug info for naked functions * Fixed function name typo that @Sija found * Addressed PR comments and refactored back to Type for debug_type_hash. * changed the order of 'if' branch execution for setup_fun when --debug flag is used as per PR comments from @RX14 * Removed method attributes irrelevant for debugging as per @RX14 * Refactored code as per suggestions from @Asterlite and @Sija * Code refactoring as per @Sija suggestions * Removed all unused methods that were implemented during debug support implementeation. * Initial debug specs Generate a lldb script from magic comments and check the output using FileCheck * Cherrypicked debug driver from @bcardiff Co-authored-by: Sergey Kuznetsov <[email protected]> Co-authored-by: Brian J. Cardiff <[email protected]>

dinkopehar · 2021-05-06T21:29:35Z

@skuznetsov

Is there a way, when dereferencing is disabled, to see values of variables ?

For example, in a simple program I see all values of variables, but in larger ones, when that option is disabled, I only get hex values which I don't understand...

skuznetsov · 2021-05-13T14:27:22Z

@skuznetsov

Is there a way, when dereferencing is disabled, to see values of variables ?

For example, in a simple program I see all values of variables, but in larger ones, when that option is disabled, I only get hex values which I don't understand...

Usually, you have to add * before dereferencing it (or use -> if it is inside of a structure) as it uses C referencing model.

dinkopehar · 2021-05-14T07:28:07Z

Yes, I'm aware of that. But mostly with nested structures, it states "can not read memory at"... But CodeLLDB reads it normally sometimes.

Rudimentary debug support is here. It allows LLDB to show Crystal var…

6346c1c

…iables properly in C lang style. To work properly some LLDB patching is needed due to the dereferencing bug in DWARF Expression Evaluator.

ysbaddaden reviewed Nov 30, 2019

View reviewed changes

src/compiler/crystal/codegen/fun.cr Show resolved Hide resolved

Rolled back enforced NoInline in debug mode.

401de7a

skuznetsov requested a review from ysbaddaden December 3, 2019 02:17

asterite reviewed Dec 4, 2019

View reviewed changes

src/llvm/di_builder.cr Outdated Show resolved Hide resolved

asterite reviewed Dec 4, 2019

View reviewed changes

src/compiler/crystal/codegen/debug.cr Outdated Show resolved Hide resolved

asterite approved these changes Dec 4, 2019

View reviewed changes

Added comment to explain why we have to use C++ Language ID

b3105f4

ysbaddaden removed their request for review December 5, 2019 14:22

Added debug information for union and nullable types.

574eb32

RX14 reviewed Jan 20, 2020

View reviewed changes

src/compiler/crystal/program.cr Outdated Show resolved Hide resolved

skuznetsov commented Jan 21, 2020

View reviewed changes

Fixed some of the debug type glitches (MixedUnionType notoriously) as…

65cc438

… well as fixed line linfo for blocks.

Sergey Kuznetsov and others added 3 commits April 4, 2020 14:41

Merge branch 'master' of https://github.com/crystal-lang/crystal into…

f81e506

… debug

Merge branch 'master' into debug

7324863

Initial debug specs

3291ec8

Generate a lldb script from magic comments and check the output using FileCheck

Sergey Kuznetsov added 2 commits April 8, 2020 22:27

Merge commit '3291ec8533c4bb505ac122c9e6bd3e17b6502032' into debug

4024e52

Cherrypicked debug driver from @bcardiff

3811814

straight-shoota added kind:feature topic:compiler:debugger labels Apr 9, 2020

Merge branch 'master' of https://github.com/crystal-lang/crystal into…

83d3e8c

… debug

asterite approved these changes Apr 14, 2020

View reviewed changes

Merge branch 'master' of https://github.com/crystal-lang/crystal into…

c0e2417

… debug

bcardiff reviewed Apr 14, 2020

View reviewed changes

bcardiff added this to the 0.35.0 milestone Apr 16, 2020

bcardiff merged commit 642ae96 into crystal-lang:master Apr 16, 2020

skuznetsov deleted the debug branch April 16, 2020 19:58

bcardiff mentioned this pull request Apr 17, 2020

Move recent additions to DIBuilder to LLVMExt #9114

Merged

Improve debugging support #8538

Improve debugging support #8538

Conversation

skuznetsov commented Nov 30, 2019 • edited Loading

ysbaddaden left a comment

Choose a reason for hiding this comment

asterite left a comment

Choose a reason for hiding this comment

ysbaddaden commented Dec 4, 2019

skuznetsov commented Dec 4, 2019 via email

ysbaddaden commented Dec 4, 2019

skuznetsov commented Dec 4, 2019

RX14 commented Dec 5, 2019 • edited Loading

skuznetsov commented Dec 5, 2019 • edited Loading

RX14 commented Dec 5, 2019 • edited Loading

skuznetsov commented Dec 5, 2019

RX14 commented Dec 5, 2019

ysbaddaden commented Jan 20, 2020

RX14 commented Jan 20, 2020 • edited Loading

RX14 commented Jan 20, 2020 • edited Loading

skuznetsov commented Jan 20, 2020

skuznetsov commented Jan 20, 2020

skuznetsov left a comment

Choose a reason for hiding this comment

RX14 commented Jan 21, 2020

skuznetsov commented Jan 21, 2020

ysbaddaden commented Jan 30, 2020

bcardiff commented Apr 9, 2020

skuznetsov commented Apr 9, 2020

bcardiff commented Apr 9, 2020

RX14 commented Apr 9, 2020

bcardiff commented Apr 14, 2020

RX14 commented Apr 14, 2020

asterite left a comment

Choose a reason for hiding this comment

bcardiff left a comment

Choose a reason for hiding this comment

bcardiff Apr 14, 2020

Choose a reason for hiding this comment

bcardiff commented Apr 16, 2020

jkthorne commented Apr 16, 2020

RX14 commented Apr 16, 2020

bcardiff commented Apr 17, 2020

skuznetsov commented Apr 18, 2020

dinkopehar commented May 6, 2021

skuznetsov commented May 13, 2021 • edited Loading

dinkopehar commented May 14, 2021

skuznetsov commented Nov 30, 2019 •

edited

Loading

RX14 commented Dec 5, 2019 •

edited

Loading

skuznetsov commented Dec 5, 2019 •

edited

Loading

RX14 commented Dec 5, 2019 •

edited

Loading

RX14 commented Jan 20, 2020 •

edited

Loading

RX14 commented Jan 20, 2020 •

edited

Loading

skuznetsov commented May 13, 2021 •

edited

Loading