Added LLVM_source attributes to line programs #432
Conversation
| panic!("Can only write LineString::LineStringRef for embedded sources, got {:?}", val); | ||
| } | ||
| (true, None) => { | ||
| panic!("has_embedded_source is true, but no source set"); |
There was a problem hiding this comment.
this is highly suboptimal. I am thinking of just writing a empty string reference here.
|
I haven't looked in detail yet.
I think that's why the DWARF issue (http://dwarfstd.org/ShowIssue.php?issue=180201.1) includes a
That's fine. |
| )?; | ||
| } | ||
| (true, Some(ref val)) => { | ||
| panic!("Can only write LineString::LineStringRef for embedded sources, got {:?}", val); |
There was a problem hiding this comment.
If LineStringRef is the only option, then no point using LineString? Just use the reference directly.
There was a problem hiding this comment.
Was considering that. I wasn't sure if there is already a pattern in gimli for this.
There was a problem hiding this comment.
Looking at the DWARF issue, it doesn't restrict the string form, so probably shouldn't here either.
There was a problem hiding this comment.
The LLVM extension however is restricted:
Which I assume makes sense because with the form descriptor system you need to match them across all files.
There was a problem hiding this comment.
LLVM permits DW_FORM_string too: source. I don't see a good reason to restrict the form in gimli when supporting all can be done by simply calling LineString::write.
There was a problem hiding this comment.
I can do that, but i need to know which type i'm going to emit for the form descriptor before I see the file info. So which one do I pick? The string type of the first file info's source and then require them all to match?
There was a problem hiding this comment.
Yes. That's what the file_form handling above does.
|
Heh. Writing the empty string is not so easy because at the point where that would be convenient to do I only have the line string offsets. Not sure if adding a new empty string parameter is a great idea. |
|
Commit title: *LLVM_source |
|
Where were you thinking of adding the empty string parameter? I don't have any great ideas for how to handle this. Maybe something along the lines of requiring an empty string to be added whenever |
|
Yeah not sure. First I thought about just adding it as an extra parameter to |
|
Looking at the LLVM implementation a bit more they require source to be set for all files in a program or none. So maybe that's actually the way to go here. |
|
Let's leave it how you have it then. If the caller sets |
|
@mitsuhiko Do you want to finish this off before I do another release? |
|
I will come back to that later. I wasn't able to use it myself yet because all the dwarfs I have are dwarf4 so I need to first upgrade them to dwarf5 before I can actually utilize this myself. |
|
DWARF6 will add support for this in a way that doesn't require all files to have the same format. We can revisit this when DWARF6 support is added. |
WIP implementation for #431.
Annoyingly due to how these form descriptors work it means that all files need to have the same format. I wonder if it makes sense to just write an empty string in such cases. Effectively something similar happens with md5 hashes already.
Second issue is that this change required to remove a
Copybound :(