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

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 24 04:30:08 PDT 2019


jhenderson added inline comments.


================
Comment at: test/Object/X86/obj2yaml-dup-section-name.s:1
 # RUN: llvm-mc %s -o %t.o -filetype=obj -triple=x86_64-pc-linux
 # RUN: obj2yaml %t.o | FileCheck %s
----------------
I think you still need test cases in both section and symbol cases that show the unique ID is unique for subsequent sections of the same name (i.e. you need a .text.foo, a .text.foo [1], and a .text.foo [2] at least).


================
Comment at: test/tools/yaml2obj/duplicate-section-names.test:66
+## Check that yaml2obj can produce an object when symbols are defined
+## relative to a sections with duplicate names (but different name suffixes).
+
----------------
relative to sections


================
Comment at: test/tools/yaml2obj/duplicate-section-names.test:110-111
+
+## Check that yaml2obj can produce SHT_GROUP sections, described with
+## use of sections and symbols with suffixes in the names.
+
----------------
This isn't a clear sentence to me. Perhaps the following:

"Check that yaml2obj can produce SHT_GROUP sections that reference sections and symbols with name suffixes."

I might even change the last part to "name ID suffixes", because it is not necessarily clear what the suffixes refer to, but then again, this is probably not needed in the context of this test.


================
Comment at: test/tools/yaml2obj/duplicate-symbol-names.test:55-57
+## Check that yaml2obj can produce correct relocations when
+## they are described with use of symbols containing suffixes
+## in their names.
----------------
This comment can be rewritten slightly, in a similar manner to the group one:

"Check that yaml2obj can produce correct relocations that reference symbols with name suffixes."


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

https://reviews.llvm.org/D63596





More information about the llvm-commits mailing list