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

Esme Yi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 1 02:45:10 PDT 2021


Esme added inline comments.


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:149
+  size_t RawContentSize = Obj.StrTbl.RawContent.binary_size();
+  if (Obj.StrTbl.TableSize && Obj.StrTbl.TableSize < (RawContentSize + 4)) {
+    ErrHandler("specified TableSize (" + Twine(Obj.StrTbl.TableSize) +
----------------
jhenderson wrote:
> You don't need additional space for the Length field if you are using RawContent. If RawContent is used, then you should just have that content, plus some possible trailing padding if TableSize is greater than than the size of the RawContent.
I think it would be confusing that the RawContent includes the length field and also the TableSize is specified, since there would be 3 different values for the string table size, ie. the length field in RawContent, the size of RawContent and the TableSize value.

For example, the RawContent is "000000090062006300" (where the "00000009" is the length field) and the TableSize is 10, then what would the string table look like? "00000009006200630000" or "00000010006200630000"? 
And what if the RawContent is incorrect (ex. 000000070062006300) but the TableSize (ex. 9+) is correct?

Besides, I think it is not very convenient for users to calculate the content size for the input RawContent even they don't want to test the length value.

Any thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107421



More information about the llvm-commits mailing list