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

[Digger via Phabricator via llvm-commits]

DiggerLin marked 5 inline comments as done.
DiggerLin added inline comments.


================
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) {
----------------
daltenty wrote:
> Why were we previously expecting and enforcing no padding, but now we allow it?
when there is a tail padding of a section, but the value of CurrentAddressLocation do not be increased by the padding size. it will hit assert assert(CurrentAddressLocation == Section->Address && "We should have no padding between sections.");


================
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);
----------------
daltenty wrote:
> Is it possible to use `__attribute__ ((nonstring))` instead?
using the attribute will caused some other compiler  warning . We can not solve a warning and introduce other warning, Current fix a safety fix, I am prefer to keep it.


================
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
----------------
daltenty wrote:
> This should be merged with the existing tests in llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-str.ll? Also see the comment there.
thanks for your suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70859