[Sparc] Don't use SunStyleELFSectionSwitchSyntax

[ro]

Summary

As discussed in D85414, two tests currently FAIL on Sparc since that backend currently
uses the Sun assembler syntax for the .section directive, controlled by SunStyleELFSectionSwitchSyntax.

Instead of adapting the affected tests, this patch changes that default. The internal assembler still accepts both forms as input, only the output syntax is affected.

Current support for the Sun syntax is cursory at best: the built-in assembler cannot even assemble some of the directives emitted by GCC, and the set supported by the Solaris assembler is even larger: SPARC Assembly Language Reference Manual, 3.4 Pseudo-Op Attributes.

A few Sparc test cases need to be adjusted. At the same time, the patch fixes the failures from D85414.

Tested on sparcv9-sun-solaris2.11.

I believe this patch needs wider testing first, though: I have a hard time getting decent test results for a 2-stage build on Solaris/SPARC. I'm currently trying a sparc64-unknown-linux-gnu build to see if things fare better there.

[ro]

Ping? It's been a week.

Although D85414 has landed meanwhile, it still seems sensible to switch to the common ELF syntax on Sparc.

[efriedma]

Instead of adapting the affected tests, this patch changes that default. The internal assembler still accepts both forms as input, only the output syntax is affected.

There's a check in the parser for usesSunStyleELFSectionSwitchSyntax(); what effect does that have?

If we expect that everyone targeting SPARC these days is using an assembler that understands the standard ELF syntax, printing that syntax seems fine. I assume that's true on Linux; not sure what assemblers are commonly used on Solaris.

[ro]

In D85415#2216469, @efriedma wrote:

Instead of adapting the affected tests, this patch changes that default. The internal assembler still accepts both forms as input, only the output syntax is affected.

There's a check in the parser for usesSunStyleELFSectionSwitchSyntax(); what effect does that have?

It now rejects the Sun-style syntax on input. That's certainly not what I wanted to happen: instead my intent was to produce common ELF syntax on output, but accept both on input as the current/unpatched clang does. I must have been dreaming when I tested this!

When I checked large LLVM users (NetBSD and OpenBSD), I found that there is still one large codebase the relies on the ability to read the Sun-style syntax, i.e. OpenSSL. This is certainly crucial enough to keep support in the parser.

If we expect that everyone targeting SPARC these days is using an assembler that understands the standard ELF syntax, printing that syntax seems fine. I assume that's true on Linux; not sure what assemblers are commonly used on Solaris.

It's not true for the Solaris assembler, but a recent gas is always bundled on Solaris 11.4, too. In fact, the included versions of gcc are configured to use gas, not /bin/as. The situation is similar to what was remedied for the linker with D84029: if you rely on one of two incompatible versions (/bin/ld instead of /usr/gnu/bin/ldin that case), you must somehow make certain that you always use the right one.

There's normally no issue on Solaris anyway since clang uses the internal assembler. I guess clang/lib/Driver/ToolChains/Solaris.cpp (solaris::Assembler::ConstructJob) needs a treatment similar to solaris::Linker::ConstructJob and GetLinkerPath.

[ro]

In D85415#2223611, @ro wrote:

In D85415#2216469, @efriedma wrote:

Instead of adapting the affected tests, this patch changes that default. The internal assembler still accepts both forms as input, only the output syntax is affected.

There's a check in the parser for usesSunStyleELFSectionSwitchSyntax(); what effect does that have?

It now rejects the Sun-style syntax on input. That's certainly not what I wanted to happen: instead my intent was to produce common ELF syntax on output, but accept both on input as the current/unpatched clang does. I must have been dreaming when I tested this!

If we expect that everyone targeting SPARC these days is using an assembler that understands the standard ELF syntax, printing that syntax seems fine. I assume that's true on Linux; not sure what assemblers are commonly used on Solaris.

It's not true for the Solaris assembler, but a recent gas is always bundled on Solaris 11.4, too. In fact, the included versions of gcc are configured to use gas, not /bin/as. The situation is similar to what was remedied for the linker with D84029: if you rely on one of two incompatible versions (/bin/ld instead of /usr/gnu/bin/ldin that case), you must somehow make certain that you always use the right one.

I've done more checking now:

• gas claims to support Sun style section syntax for all ELF targets, but a quick test on Solaris/amd64 and Linux/x86_64 didn't confirm that.
• While clang defaults to the internal assembler on Solaris, with -fno-integrated-as it creates output that the native Solaris assembler cannot handle:
  • While as requires the section name in double quotes, clang emits it unquoted.
  • clang unconditionally emits gas-only directives, e.g. .type var,@object where #object should be used, or .p2align 2 which isn't supported at all.

Overall the Sun style syntax is support is mostly useless with an external assembler (maybe gas could handle it, but it can already handle the common ELF syntax).

So my suggestion would be

• Default to common ELF style on Sparc for output.
• Make certain that only gas is used on Solaris/Sparc to match just in case someone decides to use -fno-integrated-as.
• Still allow Sun style syntax on input (or rather the meagre subset clang currently supports). This could either be achieved by doing it unconditionally for all ELF targets (by removing the usesSunStyleELFSectionSwitchSyntax check in ELFAsmParser.cpp (ELFAsmParser::ParseSectionArguments) or splitting usesSunStyleELFSectionSwitchSyntax into separate emitSunStyleELFSectionSwitchSyntax and readSunStyleELFSectionSwitchSyntax. The latter seems overkill for such obscure a feature.

Comments?

[efriedma]

That all makes sense.

I don't see any obvious reason why we shouldn't accept the Sun syntax on all targets; the parsing is straightforward, and it's not ambiguous.

[ro]

I've now discovered that not only the sparc assembler output requires GNU as, but the same is true for x86:

• as is invoked without -m64/-m32, thus chokes on amd64 insns since it default to 32-bit mode
• Even if adding -Wa,-m64, assembly fails:

Assembler: main.c
	"/var/tmp/main-faae1e.s", line 24 : Syntax error
	Near line: "	.section	".note.GNU-stack","",@progbits"

• There must be no double quotes around the section name.
• Without the quotes, the section name is no valid identifier: - isn't allowed.
• On top of that, Solaris doesn't care about those .note sections: to achieve non-executable stacks, you need e.g. ld -z sx=nxstack (Solaris 11.4) or ld -z nxstack (Solaris 11.3).

However, to make -fno-integrated-as work on Solaris is more work than just enforcing use of /usr/gnu/bin/as. I guess I'll split that off into a separate patch: the Sun style section syntax change makes the situation here neither worse nor better.

[MaskRay]

For Linux, this makes sense. sparc64-linux-gnu-gcc uses the GNU style section directive. If Solaris may prefer SunStyleELFSectionSwitchSyntax, you can add a condition.

[ro]

In D85415#3707384, @MaskRay wrote:

For Linux, this makes sense. sparc64-linux-gnu-gcc uses the GNU style section directive. If Solaris may prefer SunStyleELFSectionSwitchSyntax, you can add a condition.

I don't think I'll bother: /bin/as cannot even assemble a trivial test.s emitted without this patch (use of .p2align, @function instead of #function). I won't even think about what happens for real-world code. If anything, I'll sometime finish my patch to make sure /usr/gnu/bin/as is used on Solaris with -fno-integrated-as.

llvm/lib/MC/MCParser/ELFAsmParser.cpp

if (IsPush && getLexer().isNot(AsmToken::String)) {
       if (getParser().parseExpression(Subsection))
         return true;
       if (getLexer().isNot(AsmToken::Comma))
         goto EndStmt;
       Lex();
     }
 
     if (getLexer().isNot(AsmToken::String)) {
-      if (!getContext().getAsmInfo()->usesSunStyleELFSectionSwitchSyntax()
-          || getLexer().isNot(AsmToken::Hash))
+      if (getLexer().isNot(AsmToken::Hash))
         return TokError("expected string in directive");
       extraFlags = parseSunStyleSectionFlags();
     } else {
       StringRef FlagsStr = getTok().getStringContents();
       Lex();
       extraFlags = parseSectionFlags(getContext().getTargetTriple(), FlagsStr,
                                      &UseLastGroup);
     }

llvm/lib/Target/Sparc/MCTargetDesc/SparcMCAsmInfo.cpp

SparcELFMCAsmInfo::SparcELFMCAsmInfo(const Triple &TheTriple) {
   // .xword is only supported by V9.
   Data64bitsDirective = (isV9) ? "\t.xword\t" : nullptr;
   ZeroDirective = "\t.skip\t";
   CommentString = "!";
   SupportsDebugInformation = true;
 
   ExceptionsType = ExceptionHandling::DwarfCFI;
 
-  SunStyleELFSectionSwitchSyntax = true;
   UsesELFSectionDirectiveForBSS = true;
 }
 
 const MCExpr*
 SparcELFMCAsmInfo::getExprForPersonalitySymbol(const MCSymbol *Sym,
                                                unsigned Encoding,
                                                MCStreamer &Streamer) const {
   if (Encoding & dwarf::DW_EH_PE_pcrel) {

llvm/test/CodeGen/SPARC/2008-10-10-InlineAsmRegOperand.ll

 ; RUN: llc < %s -march=sparc
 ; PR 1557
 
 target datalayout = "E-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-f128:128:128"
-module asm "\09.section\09\22.ctors\22,#alloc,#write"
-module asm "\09.section\09\22.dtors\22,#alloc,#write"
+module asm "\09.section\09.ctors,\22aw\22"
+module asm "\09.section\09.dtors,\22aw\22"
 
 define void @frame_dummy() nounwind {
 entry:
   %asmtmp = tail call void (i8*)* (void (i8*)*) asm "", "=r,0"(void (i8*)* @_Jv_RegisterClasses) nounwind    ; <void (i8*)*> [#uses=0]
   unreachable
 }
 
 declare void @_Jv_RegisterClasses(i8*)

llvm/test/CodeGen/SPARC/constructor.ll

 ; RUN: llc -mtriple sparc-sun-solaris2.11 -use-ctors < %s | FileCheck --check-prefix=CTOR %s
 ; RUN: llc -mtriple sparc-sun-solaris2.11 < %s | FileCheck --check-prefix=INIT-ARRAY %s
 @llvm.global_ctors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @f, i8* null}, { i32, void ()*, i8* } { i32 15, void ()* @g, i8* @v }]
 
 @v = weak_odr global i8 0
 
 define void @f() {
 entry:
   ret void
 }
 
 define void @g() {
 entry:
   ret void
 }
 
-; CTOR:      .section      .ctors,#alloc,#write
+; CTOR:      .section      .ctors,"aw"
 ; CTOR-NEXT: .p2align      2
 ; CTOR-NEXT: .word  f
-; CTOR-NEXT: .section      .ctors.65520,#alloc,#write
+; CTOR-NEXT: .section      .ctors.65520,"aGw"
 ; CTOR-NEXT: .p2align      2
 ; CTOR-NEXT: .word  g
 
-; INIT-ARRAY:    .section  .init_array.15,#alloc,#write
+; INIT-ARRAY:    .section  .init_array.15,"aGw"
 ; INIT-ARRAY-NEXT: .p2align  2
 ; INIT-ARRAY-NEXT: .word g
-; INIT-ARRAY-NEXT: .section  .init_array,#alloc,#write
+; INIT-ARRAY-NEXT: .section  .init_array,"aw"
 ; INIT-ARRAY-NEXT: .p2align  2
 ; INIT-ARRAY-NEXT: .word f