Page MenuHomePhabricator

[LLDB] Addresses can be two bytes in size
AbandonedPublic

Authored by aykevl on Feb 4 2020, 5:57 AM.

Details

Summary

Addresses are usually two bytes in size on AVR, so make sure LLDB deals with that.


I'm not sure I caught all cases. In particular, lldb/include/lldb/DataFormatters/FormattersHelpers.h might be related but I've left it alone as it didn't seem to be necessary.

Honestly I think LLDB would become a lot more forgiving to uncommon architectures when these asserts are removed and avoids assumptions based on the address size. When the address size is relevant (for example, to read a value from memory), it should have an exhaustive test with a default case that does an assert (for easy debugging). For example:

switch (addr_size) {
case 4:
    // do one thing
case 8:
    // do something else
default:
    assert(false && "unknown addr_size");
}

This way, it's easy to find the places that make these assumptions just by following the asserts.

I'm not sure how to add a test for this. I can test it locally by connecting to a remote debugger (simavr). However, I don't know how to do something like that in the LLDB testing framework. Additionally, for that to work I only needed the change in DumpDataExtractor.cpp so I'm not sure how to exhaustively test this.

Diff Detail

Event Timeline

aykevl created this revision.Feb 4 2020, 5:57 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFeb 4 2020, 5:57 AM
labath added a comment.Feb 4 2020, 3:24 PM

You should be able to test most (if not all) of this stuff without an actual process. lldb can display global variables, set breakpoints, resolve addresses, etc. without being connected to a debug stub.

How do you normally generate the binaries with these kinds of addresses? Can they be produced with clang? Can they be produced with llvm-mc? Linked with lld ?

andwar added a subscriber: andwar.Feb 5 2020, 10:54 AM

How do you normally generate the binaries with these kinds of addresses? Can they be produced with clang? Can they be produced with llvm-mc? Linked with lld ?

With avr-gcc. I think it's easiest to just generate a minimal binary using yaml2obj.

For the rest I'm focusing on D73969 now, this patch should probably be abandoned.

labath added a comment.Feb 5 2020, 2:30 PM

How do you normally generate the binaries with these kinds of addresses? Can they be produced with clang? Can they be produced with llvm-mc? Linked with lld ?

With avr-gcc. I think it's easiest to just generate a minimal binary using yaml2obj.

For the rest I'm focusing on D73969 now, this patch should probably be abandoned.

yaml2obj can certainly be useful, the yaml files are not that readable when you need to create more complicated inputs (e.g. with DWARF). Writing assembly could be more useful there.

But anyway, we can evaluate that on a case-by-case basis.

aykevl abandoned this revision.Feb 25 2020, 7:32 AM

Closing this one. Maybe there is something useful in this patch but if there is, I'll submit that in a standalone (more focused) patch. The majority of it has been merged in D73969.

Yes, standalone patches are the way to go. To help you, I've tried to annotate the various assertions, what kind of problems they could cause, and possible testing strategies.

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
71

I don't believe this will cause any failures, but it will stop lldb from using the debug_aranges section (and fall back to other, potentially slower, alternatives).

lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
812

This will definitely mean you won't be able to parse any (DWARF) debug info. For a test inspiration, you could look at the existing .s files in test/Shell/SymbolFile/DWARF.

lldb/source/Symbol/DWARFCallFrameInfo.cpp
42

This will probably cause some problems when unwinding via eh/debug_frame. Testing will be somewhat tricky, as I don't think we'll parse this without a running process. I'd leave this one for the end.

llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp
134

This one is also pretty critical for DWARF debug info. You could probably test it together with the DWARFUnit thingy. To trigger, it should be enough to try to display a global variable ("target variable my_global").