[PATCH] D58119: [obj2yaml][yaml2obj] - Add support of parsing/dumping of the .gnu.version_r section.

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: test/tools/obj2yaml/verneed-section.yaml:60
+            Other:           4
+      - Version:         2
+        File:            dso.so.1
----------------
grimar wrote:
> jhenderson wrote:
> > I'm not sure we should be testing version 2, because the structure will change between versions (otherwise it would still be version 1). Indeed, this could cause the whole yaml2obj design to fall apart! I think a second Version 1 dependency would be better.
> Did that, but FTR my vision of the problem is a bit different.
> yaml2obj is a convenient tool for creating outputs and also test cases. For the latter sometimes
> we want to create broken outputs, i.e. ones that are possible to produce only with a hex editing
> the binaries normally.
> 
> I did not have plans to write a test for versions other than 1 (though as far I can see, we do
> not check the version field when parsing inputs in LLD atm, and hence have no test),
> but generally, we should probably keep in mind that might want to produce invalid outputs and yaml2obj should
> allow doing that probably.
> 
> Anyways, I am ok to use version 1 here, as it was not the main aim of this patch to allow changing it.
> 
> 
Ah, I see your point. In that case, I think the user should use the Content field (assuming that's allowed) instead of the explicit fields, because it is unclear what fields should exist. The version field explicitly exists to mark different formats for the section, so explicit fields may not make that much sense.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58119/new/

https://reviews.llvm.org/D58119