backend: dwarfdbginf: refactor DWARF abbreviations

[ljmf00]

Signed-off-by: Luís Ferreira contact@lsferreira.net

---

Motivation for this change:

• All abbreviations are LEB128 encoded and a lot of code uses byte writes, which is technically right, because, before a certain range (> 0x80), LEB128 encoding doesn't matter, but in situations like backend: dwarfdbginf: Add DWARF DW_AT_noreturn attribute #13202 , people (like me) struggle at finding why such simple addition doesn't work as expected. Therefore this increases a lot on readability and simplifies the logic.
• Compile-time LEB128 encoding, increasing performance for bulk append.
• Modernize the code, but still maintain compatibility with C API.

[dlang-bot]

Thanks for your pull request, @ljmf00!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#13237"

[ljmf00]

#12832 seem to break this. I reverted it locally and it does not segfault. @andralex do you have any idea?

Here is the stacktrace and locals

[ljmf00]

#12832 seem to break this. I reverted it locally and it does not segfault. @andralex do you have any idea?

Outbuffer dtor got uncommented and double-free happened. Fixed.

[RazvanN7]

@ljmf00 are you still working on this?

[ljmf00]

Yes, I still have notes about this on my task list. I stopped this due to the fact that I have a lot of other changes on DWARF generator that will need refactoring for sure. I'm going to wait for merge of those changes and then proceed with the rest of the refactor.

[RazvanN7]

Can you point out what other PRs are stalling this? I can put them on the fast lane.

[ljmf00]

Can you point out what other PRs are stalling this? I can put them on the fast lane.

I still have the following:

• backend: use correct type names for D #13274
• backend: dwarfdbginf: Use LEB128 encoding for lines and columns #13246

After I have the type naming correct (#13274), I need to do a similar change on top to #13254 for char and wchar types to make those types UTF encoded too. After that I think I can proceed to work on this.

EDIT: The other patches I had here marked as stale were already merged.

[RazvanN7]

Ok, I merged one of those and the other one should get in soon. Ping me whenever you are stalled on something.

[ljmf00]

Weirdly, I can't reproduce dshell testsuite locally.

[thewilsonator]

This is green, is it ready for review?

[ljmf00]

This is green, is it ready for review?

I still have a lot to change. This can be iteratively done tho. I'm thinking of adding more testing on DWARF before proceeding, but probably a half-backed refactor is not good to merge. The test suite I want to add is pretty much adding support for checking the debug info generated with llvm-dwarfdump --verify.

[ljmf00]

Needs #13237 .

[thewilsonator]

Circular reference, stack overflow.

[ljmf00]

Circular reference, stack overflow.

Oh, sorry 🙃 I meant #13754.

[ljmf00]

Finally ready for review 😊

[WalterBright]

This is very hard to review because it tries to do way too much. The "Motivation" section lists 4 distinct refactorings. 4 PRs, one for each, would be much more reviewable. This one is nearly 500 lines long.

[ljmf00]

This is very hard to review because it tries to do way too much. The "Motivation" section lists 4 distinct refactorings. 4 PRs, one for each, would be much more reviewable. This one is nearly 500 lines long.

I try to make refactors atomic, but for this one, I didn't see the need.

This PR pretty much removes manual LEB128 encoded arrays with abuf.writeByte() calls and turns them into compile-time LEB128 arrays writing it with a separate interface for DWARF Abbreviations. Everything listed in the motivation is tied up together. __gshared variables are removed because they don't make sense with that approach.

The hardest part I see in the review is the move of the dwarf_abbrev_code function, because git tries to be smart. I can split it up in a separate PR, if you prefer. The rest is just the same thing but done in the entire file, which is naturally huge. What I can do is do that changes incrementally, as they are independent of each other, although they are all the same refactor.

[ljmf00]

@WalterBright I just split this PR up into smaller pieces. I will create upcoming PRs to refactor the missing parts. Are you able to review it now?

[ljmf00]

Ping @WalterBright or perhaps @RazvanN7 can you review this?