[PATCH] D107421: [yaml2obj][XCOFF] Customize the string table.

[Esme Yi via Phabricator via llvm-commits]

Esme added inline comments.


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:420
+  size_t StrTblBuilderSize = StrTblBuilder.getSize();
+  // Write the string table directly if no length value is specified.
+  if (!Obj.StrTbl.Length && !Obj.StrTbl.ContentSize) {
----------------
jhenderson wrote:
> I'm not sure why you're doing this. `Length` should be optional and a default value generated for it, based on other specified fields, if an explicit one is not specified.
If neither Length nor ContentSize is specified, we follow the original code logic, i.e. use `StrTblBuilder.write(W .OS)` to write the whole table directly with a calculated default length value.
If Length or ContentSize is specified, we have to serialize the string table and rewrite the length field manually (as Higuoxing's comment).
Since the latter approach is more time-consuming, I am doing this here to determine in advance whether we can write the string table directly.


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:433
+  StrTblBuilder.write(Ptr);
+  // Replace the first 4 bytes with the specified length value.
+  memset(Ptr, 0, 4);
----------------
jhenderson wrote:
> Unless I'm missing something, this doesn't seem right. You need 4 bytes for the Length field, and then the entire string table contents. The Length field doesn't overwrite the first four bytes worth of strings...
`StrTblBuilder.write(Ptr);` writes the whole content of string table (including the length field) into the buffer and we don't want to change the interface of StringTableBuilder, that's why I have to rewrite the length field here.
```
void StringTableBuilder::write(uint8_t *Buf) const {
...
  else if (K == XCOFF)
    support::endian::write32be(Buf, Size);
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107421