[PATCH] D95505: [yaml2obj] Initial support for 32-bit XCOFF in yaml2obj.

[ChenZheng via Phabricator via llvm-commits]

shchenz added inline comments.


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:264
+    W.write<uint32_t>(YamlSym.Value);
+    W.write<int16_t>(SectionIndexMap[YamlSym.SectionName]);
+    W.write<uint16_t>(YamlSym.Type);
----------------
Esme wrote:
> shchenz wrote:
> > What about a symbol that does not have belonged sections? For example undefined external symbols?
> ```
> Specifies a section number associated with one of the following symbols:
> -2
> Specifies N_DEBUG, a special symbolic debugging symbol.
> -1
> Specifies N_ABS, an absolute symbol. The symbol has a value but is not relocatable.
> 0
> Specifies N_UNDEF, an undefined external symbol.
> Any other value
> Specifies the section number where the symbol was defined.
> ```
> As the spec, there are 3 reserved section numbers for these symbols not defined in sections. The SectionName in the symbol entry is required now, however, we can also set the field optional and use N_UNDEF (maybe...) as the default section. Hmm... I am not sure which strategy is more reasonable. What do you think?
I think `SectionName` for reserved section number `N_UNDEF` is not defined because we don't need to do it. We only need a section number. So making `SectionName` attribute optional for a symbol makes more sense to me. You can not say that a section with section name `N_UNDEF` is the `N_UNDEF`(section number == 0) section. We can define a text section named by `N_UNDEF` by using section attribute like:
```
__attribute__((section ("N_UNDEF")))  int foo2(void)
{
}
```


================
Comment at: llvm/test/tools/yaml2obj/XCOFF/basic-doc.yaml:16
+    Relocations:
+      - Address:     0x3A
+        Type:        0x02
----------------
Esme wrote:
> shchenz wrote:
> > Is it ok that the relocation address is not in the range of `.data` section?
> In general, yaml2obj will not report an error even though the user explicitly specifies an invalid values,  which allows for things like error handling testing.
> 
> Also after getting more familiar with yaml2obj, I think it's reasonable to set more fields to optional, and these omitted values will be filled in with the default zero or values derived from contents. So I set the relocation address optional now and 0 is the default value for  it.
> 
Do you mean we setting the address as `0x3A` on purpose? The address of the `.data` section is 0x8 and its size is 0x8, so if the relocation is valid, the address the relocation entry wants to resolve should be in 0x8 and 0x10.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95505