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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D107421#2936139 <https://reviews.llvm.org/D107421#2936139>, @Esme wrote:

> Thank you! @jhenderson and @Higuoxing 
> After reconsidered from a user's perspective and referred to ELF's approach, I modified some features of the patch.
> We can see these features from cases in llvm/test/tools/yaml2obj/XCOFF/string-table.yaml.
>
>> This allows you to have a test case for a table which is too short for the length field.
>
> I don't think we could do it because we only generate the string table if its size is bigger than 4: `StrTbl.getSize() > 4`.

It sounds to me like this is an arbitrary an unnecessary check in yaml2obj?

> I didn't add the raw "Contents" field because I'm not sure this feature is necessary.

Remember, the goal of yaml2obj is to help craft inputs for tests. How else could you test the case where the string table data is too short/not null terminated etc? Similarly, don't forget that obj2yaml should be able to cope with somewhat malformed input, although in this case, that might not apply.



================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:160
+      } else
+        // Unoverwritten names are still stored in the string table.
+        StrTbl.add(YamlSym.SymbolName);
----------------
I don't think "unoverwritten" is a word. How about "Names that are not overwritten..."?


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:396-397
   // Write the string table.
-  if (Strings.getSize() > 4)
-    Strings.write(W.OS);
+  if (StrTbl.getSize() > 4 && !Obj.StrTbl.Suppressed)
+    StrTbl.write(W.OS);
   return true;
----------------
Higuoxing wrote:
> Esme wrote:
> > Higuoxing wrote:
> > > I believe You're are able to pad the string table to a specified length without modifying the interface of `StringTableBuilder`. Please let me know if it doesn't work :)
> > > 
> > If so, we have to rewrite the length value in the string table, otherwise these padding zeros  will not be treated as string table contents.
> I think you can do that by creating a temporary buffer.
> 
> 1. Serialize string table builder's content together with padding zeros to a temporary buffer.
> 2. Modify the first 4-byte of the temporary buffer.
> 3. Serialize the temporary buffer to the actual output stream.
> 
> @jhenderson any ideas?
Sounds like a reasonable way to do it to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107421