[PATCH] D70859: [XCOFF] fixed a bug of XCOFFObjectFile.cpp and adding new test case to verify one mergeable string for xcoffobjectfile

David Tenty via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 3 08:32:24 PST 2019


daltenty requested changes to this revision.
daltenty added a comment.
This revision now requires changes to proceed.

I really think this should probably be split, at least definitely for the commits, as there are a few separate issues going on here that don't seem related.



================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:359
 
-    assert(CurrentAddressLocation == Section->Address &&
-           "We should have no padding between sections.");
+    CurrentAddressLocation = Section->Address;
     for (const auto *Group : Section->Groups) {
----------------
Why were we previously expecting and enforcing no padding, but now we allow it?


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:413
   } else {
-    char Name[XCOFF::NameSize];
+    char Name[XCOFF::NameSize+1];
     std::strncpy(Name, SymbolName.data(), XCOFF::NameSize);
----------------
Is it possible to use `__attribute__ ((nonstring))` instead?


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-mergeablestring.ll:1
+; RUN: llc -verify-machineinstrs -mcpu=pwr9 -mtriple powerpc-ibm-aix-xcoff -filetype=obj -o %t.o  < %s 
+; RUN: llvm-objdump -D %t.o | FileCheck --check-prefix=CHECK %s
----------------
This should be merged with the existing tests in llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-str.ll? Also see the comment there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70859





More information about the llvm-commits mailing list