[PATCH] D63596: [yaml2obj/obj2yaml] - Allow having the symbols and sections with duplicated names.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Hmmm... I'm not sure we need any special behaviour at all in the name field of the syntax, and the suggested syntax is not obvious to me. Surely if yaml2obj sees two symbols with the same name, it should just generate two such symbols?

I guess the issue really is how to reference them, but I don't think that should be done through the name field of the symbol personally. I'm still thinking about this, but you could have a separate ID field, which is required to be unique, and allow references be via ID? Example:

  Symbols:
    - Name: foo
      ID: foo1 # error if symbol name of foo1 exists
    - Name: foo
      ID: foo2
  Sections:
    - Name: .rela
      Type: SHT_RELA
      Relocations:
        - Offset: 0
          Type: R_X86_64_NONE
          Symbol: foo1 # use ID
        - Offset: 1
          Type: R_X86_64_NONE
          Symbol: foo # error - ambiguous
  
  # Possible alternative:
        - Offset: 0
          Type: R_X86_64_NONE
          Symbol: foo # use name, but if multiple found, fallback to ID; error if no ID specified during fallback.
          ID: foo1

The same syntax would work for sections.

I'd also like to see more testing of referencing these unique sections and symbols. At the moment, it seems limited to what there is in the obj2yaml test case. I think you should add some dedicated yaml2obj tests to show that the references work.



================
Comment at: test/tools/obj2yaml/duplicate-symbol-names.test:1-2
+## Check that obj2yaml is able to produce the YAML
+## from the object containing symbols with duplicate names.
+
----------------
"produce YAML from an object"


================
Comment at: test/tools/obj2yaml/duplicate-symbol-names.test:34
+## Check we can refer to symbols with the same
+## name from the relocations.
+
----------------
Remove "the"


================
Comment at: test/tools/obj2yaml/duplicate-symbol-names.test:75
+
+## Check obj2yaml does not add a suffix to name if the symbol
+## is in .symtab and .dynsym at the same time.
----------------
to a name


================
Comment at: test/tools/yaml2obj/duplicate-section-names.test:1-2
+## Check that yaml2obj is able to produce an object from the YAML
+## containing section with duplicate names (but different name suffixes).
+
----------------
from YAML
containing sections


================
Comment at: test/tools/yaml2obj/duplicate-symbol-names.test:1-2
+## Check that yaml2obj is able to produce an object from the YAML
+## containing symbols with duplicate names (but different name suffixes).
+
----------------
from YAML


================
Comment at: test/tools/yaml2obj/duplicate-symbol-names.test:20
+
+## Check that yaml2obj reports an error in case we have
+## symbols with equal names and suffixes.
----------------
in case -> when


================
Comment at: test/tools/yaml2obj/duplicate-symbol-names.test:36
+
+## Check that yaml2obj reports an error in case we have
+## symbols without suffixes in the names and their
----------------
in case -> when


================
Comment at: tools/obj2yaml/elf2yaml.cpp:157
+
+  // Dump symbols. We need to do that early because another sections might want
+  // to access the deduplicated symbol names that we also create here.
----------------
that -> this

another -> other


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

https://reviews.llvm.org/D63596