[PATCH] D88824: [Support][unittests] Enforce alignment in ConvertUTFTest

Rainer Orth via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 13 02:20:33 PDT 2020


ro added a comment.

In D88824#2325457 <https://reviews.llvm.org/D88824#2325457>, @rnk wrote:

> I may have been the one that picked `ArrayRef<char>` as the parameter type here, and I think what I was thinking is: "files are full of bytes, the caller is most likely to have a bag of bytes that they want to hand to the re-encoding routine, so it should accept bytes, and I'll pick some arbitrary byte-like character type here". For example, MemoryBuffer gives you back a StringRef for the file contents, and you should be able to just plug that in here. MemoryBuffer probably happens to return aligned file contents in practice, but it doesn't advertise any alignment guarantee. So maybe the best fix is to underalign the UTF16 typedef, if that's possible. This is a serialization routine, after all: it should be really generous about what it accepts.

I suspect this works fine in all cases where some allocation function comes into play: they tend to guarantee the strictest alignment.  Only in those cases (like the current one) where local variables are used there is no alignment guarantee.



================
Comment at: llvm/unittests/Support/ConvertUTFTest.cpp:83
   // Src is the look of disapproval.
   static const char Src[] = "\xff\xfe\xa0\x0c_\x00\xa0\x0c";
   ArrayRef<UTF16> SrcRef = makeArrayRef((const UTF16 *)Src, 4);
----------------
rnk wrote:
> I think you just need to align this to reland.
That would be really nice: this way we could decouple getting the Solaris/sparcv9 bot closer to green from devising a full solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88824



More information about the llvm-commits mailing list