[PATCH] D67757: [yaml2obj/obj2yaml] - Add support for .stack_sizes sections.

[George Rimar via Phabricator via llvm-commits]

grimar marked an inline comment as done.
grimar added inline comments.


================
Comment at: include/llvm/ObjectYAML/ELFYAML.h:183
+  static bool nameMatches(StringRef Name) {
+    return Name.startswith(".stack_sizes");
+  }
----------------
MaskRay wrote:
> jhenderson wrote:
> > grimar wrote:
> > > grimar wrote:
> > > > MaskRay wrote:
> > > > > grimar wrote:
> > > > > > MaskRay wrote:
> > > > > > > I think we can just use `Name == ".stack_sizes"` for now. MC doesn't create `.stack_sizes.*`
> > > > > > > 
> > > > > > > Moreover, the error messages below just use plain `.stack_sizes`.
> > > > > > I am not sure here. I did it because llvm-readobj test case uses YAML with `.stack_sizes.*`:
> > > > > > https://github.com/llvm-mirror/llvm/blob/master/test/tools/llvm-readobj/stack-sizes.test#L35
> > > > > > 
> > > > > > I do not know was it intention or the person who wrote the test just did not know how to desribe sections with the same name in YAML.
> > > > > > It probably might be not that bad to support `.stack_sizes.*` too (and may be teach MC to produve them)?
> > > > > > Or we can fix `llvm-readobj` test first. Or go with `.stack_sizes.*` here and deside what to do in a follow-up patch for YAML+readobj.
> > > > > > What would you prefer?
> > > > > `Name.startswith(".stack_sizes");` is fine if we eventually emit `.stack_sizes.*` for clang -ffunction-sections -fstack-size-section. However, there are two problems:
> > > > > 
> > > > > * Larger code size. `.stack_sizes.foo`, `.text.foo` and `.rodata.foo` can take large amount of space in `.strtab`. This is why `-fno-unique-section-names` is sometimes useful.
> > > > > * Linkers don't combine `.stack_sizes.*` into `.stack_sizes`. ld.bfd and lld will have to be taught the special rule. In lld, it is in `getOutputSectionName`.
> > > > Seems the right way for now is to fix `llvm-readobj` then. I'll do a patch.
> > > Patch for `llvm-readobj`: D67824
> > I know this is due to how LLD and other linkers behave, but I still find it less than helpful that you can't have sections with different suffixes for many section types. Why allow .text.foo, but not .stack_sizes.foo for example? I'd personally prefer to change LLD and MC (but that's another change) than change llvm-readelf. That way, it's immediately recognisable which section a .stack_sizes section is associated with when looking at the output, without needing to decipher the Link field.
> The main concern is that .stack_sizes.foo can increase the object file size considerably.  I agree that .stack_sizes.foo is immediately recognisable but I am not sure such trade-off is worthwhile. .stack_sizes is not alone - some sanitizers synthesized sections (e.g. .sancov_cntrs) are not suffixed, either. Lots of .group are not suffixed, etc.
> Why allow .text.foo, but not .stack_sizes.foo for example?
I had a time to think about this a bit. `text.foo` does not have a Link or Info fields set. It has to have a
function name encoded in the section name. On the other side `.stack_sizes` have `Link`.
My vision is that руку it is a dumping tools problem probably: readelf has a format that makes hard to show a useful output here,
though it has full information. I.e. it shows Link as a numeric value what makes matching task for a human harder.
We can change/improve it in llvm-readobj. I am not sure it worth to add section suffixes to a binary produced because of that.
Our aim is not to make human readable objects, we have set of tools for dumping them. We might want to improve them.




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

https://reviews.llvm.org/D67757