[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