• Move declarations to new sanitizer_solaris.h.
• Augment check_linker_flag do reject unknown -z options that GNU ld noisily accepts.

getpass.cpp changed to UNSUPPORTED.

• Switch disablement to SKIP_ON_* form.
• Add FIXME comment.

Only disable actually failing tests using the SKIP_ON_* form.

In D91617#2403618, @eugenis wrote:

Very interesting, thank you for the explanation.
It looks like this can be fixed by applying sign extension to the masked address?

In D91617#2401121, @eugenis wrote:

Does it mean CompactRingBuffer is broken, or just the test?

In D91622#2400952, @vitalybuka wrote:

Have you tried to change kAddressSpaceSize above?

In D90524#2393747, @glaubitz wrote:

In D90524#2393651, @glaubitz wrote:

In D90524#2393366, @glaubitz wrote:

In D90524#2393320, @ro wrote:

In D90524#2393312, @glaubitz wrote:

Ping. It would be nice to get this finally merged so that the testsuite noise finally goes down on the sparc64 Linux worker.

Please be a little more patient: one ping a week is considered appropriate, but after only two days is a bit over the top.

The problem is that LLVM is a very fast moving target and when waiting long for changes to be merged, one constantly runs
into the risk of having to rebase patches.

Superceded by D90524.

In D90524#2393353, @glaubitz wrote:

In D90524#2393319, @ro wrote:

In D90524#2388214, @glaubitz wrote:

I think it should be good for merging now. I addressed all remarks. I'm still convinced that "workaround" is the proper term though.

Quite the contrary: the comment you cited

// FIXME: This is a bit of a hack. We should really unify this code for
// reasoning about oslibdir spellings with the lib dir spellings in the
// GCCInstallationDetector, but that is a more significant refactoring.

pretty clearly is about how/where support for that layout is implemented in the clang Driver code, not about the layout itself.

I don't understand that argument. I call it "workaround", the source comment calls it "hack". It's clearly not to stay forever as it's an ugly
workaround, but until a proper fix comes around, I would like to add "sparc" here as well so the testsuite failures drop from over
400 to just below 70.

In D90524#2393312, @glaubitz wrote:

Ping. It would be nice to get this finally merged so that the testsuite noise finally goes down on the sparc64 Linux worker.

In D90524#2388214, @glaubitz wrote:

I think it should be good for merging now. I addressed all remarks. I'm still convinced that "workaround" is the proper term though.

As I'd mentioned in D90524, I believe both patches should be merged into one. Otherwise, no matter in which order you commit them, the buildbots might show failures otherwise.

As it happens, I'd arrived at exactly the same patch when I tried a build on a Debian/sparc64 system in the GCC compile farm (gcc202), submitted as D85582.

Align the last affected instance.

In 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.

In D85630#2318618, @brad wrote:

What about reverting this now that you have found this is an issue with the Sun linker?

In D88824#2318568, @efriedma wrote:

Why are we trying to enforce alignment like this in the first place? Writing an API that expects an overaligned ArrayRef<char> is just asking for trouble. There should be two possibilities:

In D88824#2317728, @thakis wrote:

This breaks tests on Windows: http://45.33.8.238/win/25377/step_11.txt

PTAL, and if it takes a while to fix please revert while you investigate.

Use alignas.

Add assertion.
Run clang-format.

Ping? It's been almost a week.

This patch broke builds with gcc, e.g. Builder clang-solaris11-amd64 Build #6724:

[23/122] Building CXX object tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o
FAILED: tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o 
/usr/gcc/9/bin/c++  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/tools/extra/clangd/tool -I/opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/clang-tools-extra/clangd/tool -Itools/clang/tools/extra/clangd/../clang-tidy -I/opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/clang/include -Itools/clang/include -Iinclude -I/opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/llvm/include -I/opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/llvm/include/llvm/Support/Solaris -I/opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/clang-tools-extra/clangd/tool/.. -Itools/clang/tools/extra/clangd/tool/.. -Itools/clang/tools/extra/clangd -fPIC -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess -Wno-redundant-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O3     -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o -MF tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o.d -o tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o -c /opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/clang-tools-extra/clangd/tool/ClangdMain.cpp
In file included from /opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/clang-tools-extra/clangd/tool/ClangdMain.cpp:9:
/opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/clang-tools-extra/clangd/tool/../ClangdLSPServer.h:47:36: error: declaration of ‘llvm::Optional<clang::clangd::OffsetEncoding> clang::clangd::ClangdLSPServer::Options::OffsetEncoding’ changes meaning of ‘OffsetEncoding’ [-fpermissive]
   47 |     llvm::Optional<OffsetEncoding> OffsetEncoding;
      |                                    ^~~~~~~~~~~~~~
In file included from /opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/clang-tools-extra/clangd/tool/../Headers.h:12,
                 from /opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/clang-tools-extra/clangd/tool/../CodeComplete.h:19,
                 from /opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/clang-tools-extra/clangd/tool/../ClangdServer.h:13,
                 from /opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/clang-tools-extra/clangd/tool/../ClangdLSPServer.h:12,
                 from /opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/clang-tools-extra/clangd/tool/ClangdMain.cpp:9:
/opt/llvm-buildbot/home/solaris11-amd64/clang-solaris11-amd64/llvm/clang-tools-extra/clangd/tool/../Protocol.h:370:12: note: ‘OffsetEncoding’ declared here as ‘enum class clang::clangd::OffsetEncoding’
  370 | enum class OffsetEncoding {
      |            ^~~~~~~~~~~~~~

Wrap whole file in if(LLVM_ENABLE_PIC).

Just found this had already been fixed in commit f0546173fa4bdde03ecb21a174fcaa8a6490adbd.

As described in D87825, this patch broke Debug builds on sparcv9-sun-solaris2.11 and amd64-pc-solaris2.11.

In D87615#2278436, @MaskRay wrote:

This patch avoid the issue by defaulting to -mstackrealign, just like gcc.

This sentence from the description should be removed.

In D87615#2278002, @joerg wrote:

I'm still curious about the source of the vptr diff, but that's a minor question, otherwise. LGTM

In D87615#2275045, @efriedma wrote:

Also, it would be nice to have some regression test coverage; add a Solaris RUN line to llvm/test/CodeGen/X86/stack-align2.ll ?

• Rely on stackAlignment default for 32-bit Solaris/x86
• Handle Solaris in llvm/test/CodeGen/X86/stack-align2.ll

Allow for whitespace differences in vptr.cpp.

In D87615#2273427, @ro wrote:

Tested on amd64-pc-solaris2.11. However, compared to the -mstackrealign version
there's one regression that I still need to investigate:

UBSan-Standalone-i386 :: TestCases/TypeCheck/vptr.cpp

That claim of 16-byte alignment on Solaris is half-wrong: it's definitely wrong on Solaris, but seems to be true on Illumos.

In D87615#2271646, @efriedma wrote:

In D87615#2271268, @RKSimon wrote:

Would it be possible to add some tests?

Ping? It's been a week and AFAICT there's nothing left for me to do.

Is there anything left to do to get approval? Thanks.

Add FIXME for SparcV8 MaxAtomicInlineWidth.