[PATCH] D58168: [yaml2obj] - Do not ignore explicit addresses for .dynsym and .dynstr

[George Rimar via Phabricator via llvm-commits]

grimar marked 2 inline comments as done.
grimar added inline comments.


================
Comment at: tools/yaml2obj/yaml2elf.cpp:324-325
 
+  // If .dynsym section is explicitly described in the YAML
+  // then we want to use its section address.
+  if (!IsStatic) {
----------------
jhenderson wrote:
> Nit: Let clang-format reflow your comments across multiple lines, rather than doing it by hand.
Clang format would produce the following:

```
  // If .dynsym section is explicitly described in the YAML then we want to use
  // its section address.
```

The first line is long, while the second is too short. It does not look nice. So you still want me to do that?
( I am often doing that by hand in such cases, btw).


================
Comment at: tools/yaml2obj/yaml2elf.cpp:377
+    if (SecNdx < Doc.Sections.size())
+      SHeader.sh_addr = Doc.Sections[SecNdx]->Address;
+  }
----------------
jhenderson wrote:
> Do you think this should have a sanity assertion to show that Doc.Sections[SecNdx] is the right section? If I'm right, this only works because an auto-generated .dynstr happens to be added to the end of the sections, but it would not be unreasonable to change that behaviour.
I am using `getDotDynStrSecNo()` here to get the section index.
It implementation just maps a section name to index:
https://github.com/llvm-mirror/llvm/blob/master/tools/yaml2obj/yaml2elf.cpp#L178

We build the `SN2I` mapping in `buildSectionIndex` here:
https://github.com/llvm-mirror/llvm/blob/master/tools/yaml2obj/yaml2elf.cpp#L605
(i.e. before calling this place)

So ordering is not important I believe.


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

https://reviews.llvm.org/D58168