Page MenuHomePhabricator

[ObjectYAML] Let dynamic entries use section names as values
Needs ReviewPublic

Authored by amontanez on Feb 8 2019, 1:52 PM.

Details

Summary

This change adds functionality to yaml2obj that allows dynamic entries to have string values via the String field. In this change, using a section name in the String field tells yaml2obj to use that section's address (pulled from the section header).

This is the first patch seeking to implement the associated RFC here: http://lists.llvm.org/pipermail/llvm-dev/2019-January/129231.html

Added:

  • String values treated as section names that are converted to addresses.

Diff Detail

Repository
rL LLVM

Event Timeline

amontanez created this revision.Feb 8 2019, 1:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 8 2019, 1:52 PM
amontanez updated this revision to Diff 187107.Feb 15 2019, 4:18 PM
amontanez retitled this revision from [ObjectYAML][yaml2obj][ELF] Add support for writing dynamic entries to [ObjectYAML] Let dynamic entries use section names as values.
amontanez edited the summary of this revision. (Show Details)

Rebase, reduced patch contents to only include one feature addition. Rather than using a unified Value field, there's now Value and String fields. This was done because a unified Value field that could accept strings and numbers made it somewhat difficult for obj2yaml to output an appropriately formatted value. This is up for discussion, though, as I'm not sure what I've done is the best solution.

Rebase, reduced patch contents to only include one feature addition. Rather than using a unified Value field, there's now Value and String fields. This was done because a unified Value field that could accept strings and numbers made it somewhat difficult for obj2yaml to output an appropriately formatted value. This is up for discussion, though, as I'm not sure what I've done is the best solution.

Honestly, I feel that a unified field would be better (note, I haven't thought much about how that would affect the implementation). I think that matches what we already do with other fields like Section's Info etc. It is also more intuitive, and you don't have to handle the case of both being specified. I also don't think it matters if yaml2obj -> obj2yaml produces the output identical to the input, as long as it is still valid. In this case, it could (should?) use the actual address of the referenced section, rather than trying to figure out the section name.

llvm/lib/ObjectYAML/ELFYAML.cpp
666–667

Why has this assert disappeared? It seems unrelated to the change?

llvm/test/tools/yaml2obj/dynamic-entries.yaml
2

Nit: Avoid mixing single and double dash long args in the same test. I prefer double-dash to avoid ambiguity with option grouping, but it's not a big deal.

llvm/test/tools/yaml2obj/dynamic-entry-invalid-val.yaml
19

I'd specify the name of the section being searched for in this error message (i.e. "xxyyzz").

llvm/tools/yaml2obj/yaml2elf.cpp
388

Why can't this be done now by calling getDotDynStrSecNo?

417

Assuming you stick with different fields for string and value, I think this and the other comments and error messages in this area might need a bit of tweaking.

780

This is a personal thing, so feel free to ignore, but I find it easier to read if this explicitly compares against null.