[COFF] Add support for .secidx directives

[timurrrr]

Summary

This is extracted from D2232, i.e. needed to generate line table information.

[• rafael]

Are these llvm extensions to the gas syntax? If so, please document
them in docs/Extensions.rst.

[• rafael]

lib/MC/MCAssembler.cpp

963 ↗

(On Diff #6197)

Data.size() is always 4. The initialization use 4 zero bytes, so this always returns false, no? If so, why is this part of the relaxation?

[echristo]

Is gas unable to do this sort of thing? If it is able it'd be good to copy the syntax from there.

test/MC/COFF/secref.s

8 ↗

(On Diff #6197)

What about cross section relocations?

test/MC/COFF/static_offset.s

15 ↗

(On Diff #6197)

If you could explain the numbers here in a comment it would be appreciated.

[timurrrr]

Mostly addressed the review comments (questions to follow).

Re: gas - I'm not very familiar with it, but I couldn't find any equivalent of .secref and .offset there.

[timurrrr]

Also added some docs (to be tried with doxygen, "compiling...").

Re: gas - was looking here https://sourceware.org/binutils/docs/as/Pseudo-Ops.html

lib/MC/MCAssembler.cpp

963 ↗

(On Diff #6197)

Data.size() is always 4. The initialization use 4 zero bytes, so this always returns false, no?

Yeah - good catch.
I've updated the code accordingly.

If so, why is this part of the relaxation?

I'm not very familiar with the code, so might be doing this wrong.
Do you recommend to move writing the actual bytes of the AddrDelta?
If so, where do this belongs?

test/MC/COFF/secref.s

8 ↗

(On Diff #6197)

Added more tests

test/MC/COFF/static_offset.s

15 ↗

(On Diff #6197)

OK, done.
I've also one ret rather three nops as in real-world examples .offset will likely be 4-byte aligned.

[• rafael]

lib/MC/MCAssembler.cpp

947 ↗

(On Diff #6209)

Since the size is fixed, this shouldn't need to be part of the relaxation.

Relaxation exits for things that can change size depending on where other stuff is. For example, a short branch can become a long one and a uleb might need another byte if things more far apart.

What prevents this from being just a data fragment with a relocation to get the correct value?

Follow how llvm-mc implements:

---

foo:

.zero 4
.zero 4

bar:

zed = bar - foo

[timurrrr]

2013/12/20 Rafael Ávila de Espíndola <[e-mail]>:

Comment at: lib/MC/MCAssembler.cpp:947
@@ -939,1 +946,3 @@

+bool MCAssembler::relaxCoffStaticOffsetFragment(

+ MCAsmLayout &Layout, MCCoffStaticOffsetFragment &LDF) {

Since the size is fixed, this shouldn't need to be part of the relaxation.

Relaxation exits for things that can change size depending on where other stuff is. For example, a short
branch can become a long one and a uleb might need another byte if things more far apart.

Sorry, I am still very unfamiliar with the MC part of the codebase, I
mostly worked on the Clang part last year.
Thanks for the clarification, I've also looked at the code and I think
now I have at least a vague idea of what relaxations are :)

What prevents this from being just a data fragment with a relocation to get the correct value?

It's just that I'm not familiar with the codebase.

Follow how llvm-mc implements:

---

foo:

.zero 4
.zero 4

bar:

zed = bar - foo

Thanks for the pointer, looking.

[timurrrr]

In fact, I think I can just use

.int To-From

instead of

.offset from From to To

I'll reduce the patch accordingly soon.
Thanks a lot for the pointer!

[timurrrr]

Removed the .offset directive part as it's indeed not needed.

Also, renamed "section reference" to "section index".

[rnk]

LGTM

[timurrrr]

Should I wait for Eric to review?

[rnk]

No need to wait, post-commit should be fine. This seems pretty obviously good, unless there's some gas directive we could be reusing.

[• rafael]

LGTM too.

[timurrrr]

Cool!
r197826, thanks for the review!

docs/Extensions.rst

 ================================
 
 X86/COFF-Dependent
 ------------------
 
 Relocations
 ^^^^^^^^^^^
 
-The following additional relocation type is supported:
+The following additional relocation types are supported:
 
 **@IMGREL** (AT&T syntax only) generates an image-relative relocation that
 corresponds to the COFF relocation types ``IMAGE_REL_I386_DIR32NB`` (32-bit) or
 ``IMAGE_REL_AMD64_ADDR32NB`` (64-bit).
 
 .. code-block:: gas
 
   .text
   fun:
     mov foo@IMGREL(%ebx, %ecx, 4), %eax
 
   .section .pdata
     .long fun@IMGREL
     .long (fun@imgrel + 0x3F)
     .long $unwind$fun@imgrel
 
+**.secrel32** generates a relocation that corresponds to the COFF relocation
+types ``IMAGE_REL_I386_SECREL`` (32-bit) or ``IMAGE_REL_AMD64_SECREL`` (64-bit).
+
+**.secidx** relocation generates an index of the section that contains
+the target.  It corresponds to the COFF relocation types
+``IMAGE_REL_I386_SECTION`` (32-bit) or ``IMAGE_REL_AMD64_SECTION`` (64-bit).
+
+.. code-block:: gas
+
+  .section .debug$S,"rn"
+    .long 4
+    .long 242
+    .long 40
+    .secrel32 _function_name
+    .secidx   _function_name
+    ...
 
 ``.linkonce`` Directive
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Syntax:
 
    ``.linkonce [ comdat type [ section identifier ] ]``
 

include/llvm/MC/MCStreamer.h

public:
   /// EmitCOFFSymbolType - Emit the type of the symbol.
   ///
   /// @param Type - A COFF type identifier (see COFF::SymbolType in X86COFF.h)
   virtual void EmitCOFFSymbolType(int Type) = 0;
 
   /// EndCOFFSymbolDef - Marks the end of the symbol definition.
   virtual void EndCOFFSymbolDef() = 0;
 
+  /// EmitCOFFSectionIndex - Emits a COFF section index.
+  ///
+  /// @param Symbol - Symbol the section number relocation should point to.
+  virtual void EmitCOFFSectionIndex(MCSymbol const *Symbol);
+
   /// EmitCOFFSecRel32 - Emits a COFF section relative relocation.
   ///
   /// @param Symbol - Symbol the section relative realocation should point to.
   virtual void EmitCOFFSecRel32(MCSymbol const *Symbol);
 
   /// EmitELFSize - Emit an ELF .size directive.
   ///
   /// This corresponds to an assembler statement such as:

lib/MC/MCAsmStreamer.cpp

public:
 
   virtual bool EmitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute);
 
   virtual void EmitSymbolDesc(MCSymbol *Symbol, unsigned DescValue);
   virtual void BeginCOFFSymbolDef(const MCSymbol *Symbol);
   virtual void EmitCOFFSymbolStorageClass(int StorageClass);
   virtual void EmitCOFFSymbolType(int Type);
   virtual void EndCOFFSymbolDef();
+  virtual void EmitCOFFSectionIndex(MCSymbol const *Symbol);
   virtual void EmitCOFFSecRel32(MCSymbol const *Symbol);
   virtual void EmitELFSize(MCSymbol *Symbol, const MCExpr *Value);
   virtual void EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                                 unsigned ByteAlignment);
 
   /// EmitLocalCommonSymbol - Emit a local common (.lcomm) symbol.
   ///
   /// @param Symbol - The common symbol to emit.
void MCAsmStreamer::EmitCOFFSymbolType (int Type) {
   EmitEOL();
 }
 
 void MCAsmStreamer::EndCOFFSymbolDef() {
   OS << "\t.endef";
   EmitEOL();
 }
 
+void MCAsmStreamer::EmitCOFFSectionIndex(MCSymbol const *Symbol) {
+  OS << "\t.secidx\t" << *Symbol;
+  EmitEOL();
+}
+
 void MCAsmStreamer::EmitCOFFSecRel32(MCSymbol const *Symbol) {
-  OS << "\t.secrel32\t" << *Symbol << '\n';
+  OS << "\t.secrel32\t" << *Symbol;
   EmitEOL();
 }
 
 void MCAsmStreamer::EmitELFSize(MCSymbol *Symbol, const MCExpr *Value) {
   assert(MAI->hasDotTypeDotSizeDirective());
   OS << "\t.size\t" << *Symbol << ", " << *Value << '\n';
 }
 

lib/MC/MCParser/COFFAsmParser.cpp

virtual void Initialize(MCAsmParser &Parser) {
     addDirectiveHandler<&COFFAsmParser::ParseSectionDirectiveData>(".data");
     addDirectiveHandler<&COFFAsmParser::ParseSectionDirectiveBSS>(".bss");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveSection>(".section");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveDef>(".def");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveScl>(".scl");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveType>(".type");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveEndef>(".endef");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveSecRel32>(".secrel32");
+    addDirectiveHandler<&COFFAsmParser::ParseDirectiveSecIdx>(".secidx");
     addDirectiveHandler<&COFFAsmParser::ParseDirectiveLinkOnce>(".linkonce");
 
     // Win64 EH directives.
     addDirectiveHandler<&COFFAsmParser::ParseSEHDirectiveStartProc>(
                                                                    ".seh_proc");
     addDirectiveHandler<&COFFAsmParser::ParseSEHDirectiveEndProc>(
                                                                 ".seh_endproc");
     addDirectiveHandler<&COFFAsmParser::ParseSEHDirectiveStartChained>(
class COFFAsmParser : public MCAsmParserExtension {
   }
 
   bool ParseDirectiveSection(StringRef, SMLoc);
   bool ParseDirectiveDef(StringRef, SMLoc);
   bool ParseDirectiveScl(StringRef, SMLoc);
   bool ParseDirectiveType(StringRef, SMLoc);
   bool ParseDirectiveEndef(StringRef, SMLoc);
   bool ParseDirectiveSecRel32(StringRef, SMLoc);
+  bool ParseDirectiveSecIdx(StringRef, SMLoc);
   bool parseCOMDATTypeAndAssoc(COFF::COMDATType &Type,
                                const MCSectionCOFF *&Assoc);
   bool ParseDirectiveLinkOnce(StringRef, SMLoc);
 
   // Win64 EH directives.
   bool ParseSEHDirectiveStartProc(StringRef, SMLoc);
   bool ParseSEHDirectiveEndProc(StringRef, SMLoc);
   bool ParseSEHDirectiveStartChained(StringRef, SMLoc);
bool COFFAsmParser::ParseDirectiveEndef(StringRef, SMLoc) {
   Lex();
   getStreamer().EndCOFFSymbolDef();
   return false;
 }
 
 bool COFFAsmParser::ParseDirectiveSecRel32(StringRef, SMLoc) {
   StringRef SymbolID;
   if (getParser().parseIdentifier(SymbolID))
-    return true;
+    return TokError("expected identifier in directive");
 
   if (getLexer().isNot(AsmToken::EndOfStatement))
     return TokError("unexpected token in directive");
 
   MCSymbol *Symbol = getContext().GetOrCreateSymbol(SymbolID);
 
   Lex();
   getStreamer().EmitCOFFSecRel32(Symbol);
   return false;
 }
 
+bool COFFAsmParser::ParseDirectiveSecIdx(StringRef, SMLoc) {
+  StringRef SymbolID;
+  if (getParser().parseIdentifier(SymbolID))
+    return TokError("expected identifier in directive");
+
+  if (getLexer().isNot(AsmToken::EndOfStatement))
+    return TokError("unexpected token in directive");
+
+  MCSymbol *Symbol = getContext().GetOrCreateSymbol(SymbolID);
+
+  Lex();
+  getStreamer().EmitCOFFSectionIndex(Symbol);
+  return false;
+}
+
 /// ::= [ identifier [ identifier ] ]
 bool COFFAsmParser::parseCOMDATTypeAndAssoc(COFF::COMDATType &Type,
                                             const MCSectionCOFF *&Assoc) {
   StringRef TypeId = getTok().getIdentifier();
 
   Type = StringSwitch<COFF::COMDATType>(TypeId)
     .Case("one_only", COFF::IMAGE_COMDAT_SELECT_NODUPLICATES)
     .Case("discard", COFF::IMAGE_COMDAT_SELECT_ANY)

lib/MC/MCStreamer.cpp

 
 void MCStreamer::EmitWin64EHEndProlog() {
   EnsureValidW64UnwindInfo();
   MCWin64EHUnwindInfo *CurFrame = CurrentW64UnwindInfo;
   CurFrame->PrologEnd = getContext().CreateTempSymbol();
   EmitLabel(CurFrame->PrologEnd);
 }
 
+void MCStreamer::EmitCOFFSectionIndex(MCSymbol const *Symbol) {
+  llvm_unreachable("This file format doesn't support this directive");
+}
+
 void MCStreamer::EmitCOFFSecRel32(MCSymbol const *Symbol) {
   llvm_unreachable("This file format doesn't support this directive");
 }
 
 /// EmitRawText - If this file is backed by an assembly streamer, this dumps
 /// the specified string in the output .s file.  This capability is
 /// indicated by the hasRawTextSupport() predicate.
 void MCStreamer::EmitRawTextImpl(StringRef String) {

lib/MC/WinCOFFStreamer.cpp

public:
   virtual void EmitAssemblerFlag(MCAssemblerFlag Flag);
   virtual void EmitThumbFunc(MCSymbol *Func);
   virtual bool EmitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute);
   virtual void EmitSymbolDesc(MCSymbol *Symbol, unsigned DescValue);
   virtual void BeginCOFFSymbolDef(MCSymbol const *Symbol);
   virtual void EmitCOFFSymbolStorageClass(int StorageClass);
   virtual void EmitCOFFSymbolType(int Type);
   virtual void EndCOFFSymbolDef();
+  virtual void EmitCOFFSectionIndex(MCSymbol const *Symbol);
   virtual void EmitCOFFSecRel32(MCSymbol const *Symbol);
   virtual void EmitELFSize(MCSymbol *Symbol, const MCExpr *Value);
   virtual void EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                                 unsigned ByteAlignment);
   virtual void EmitLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                                      unsigned ByteAlignment);
   virtual void EmitZerofill(const MCSection *Section, MCSymbol *Symbol,
                             uint64_t Size,unsigned ByteAlignment);
getAssembler().getOrCreateSymbolData(*CurSymbol).modifyFlags(
     COFF::SF_TypeMask);
 }
 
 void WinCOFFStreamer::EndCOFFSymbolDef() {
   assert(CurSymbol != NULL && "BeginCOFFSymbolDef must be called first!");
   CurSymbol = NULL;
 }
 
+void WinCOFFStreamer::EmitCOFFSectionIndex(MCSymbol const *Symbol)
+{
+  MCDataFragment *DF = getOrCreateDataFragment();
+
+  DF->getFixups().push_back(
+      MCFixup::Create(DF->getContents().size(),
+                      MCSymbolRefExpr::Create (Symbol, getContext ()),
+                      FK_SecRel_2));
+  DF->getContents().resize(DF->getContents().size() + 4, 0);
+}
+
 void WinCOFFStreamer::EmitCOFFSecRel32(MCSymbol const *Symbol)
 {
   MCDataFragment *DF = getOrCreateDataFragment();
 
   DF->getFixups().push_back(
       MCFixup::Create(DF->getContents().size(),
                       MCSymbolRefExpr::Create (Symbol, getContext ()),
                       FK_SecRel_4));

lib/Target/X86/MCTargetDesc/X86WinCOFFObjectWriter.cpp

case X86::reloc_signed_4byte:
     if (Modifier == MCSymbolRefExpr::VK_COFF_IMGREL32)
       return Is64Bit ? COFF::IMAGE_REL_AMD64_ADDR32NB :
                        COFF::IMAGE_REL_I386_DIR32NB;
     return Is64Bit ? COFF::IMAGE_REL_AMD64_ADDR32 : COFF::IMAGE_REL_I386_DIR32;
   case FK_Data_8:
     if (Is64Bit)
       return COFF::IMAGE_REL_AMD64_ADDR64;
     llvm_unreachable("unsupported relocation type");
+  case FK_SecRel_2:
+    return Is64Bit ? COFF::IMAGE_REL_AMD64_SECTION
+                   : COFF::IMAGE_REL_I386_SECTION;
   case FK_SecRel_4:
     return Is64Bit ? COFF::IMAGE_REL_AMD64_SECREL : COFF::IMAGE_REL_I386_SECREL;
   default:
     llvm_unreachable("unsupported relocation type");
   }
 }
 
 MCObjectWriter *llvm::createX86WinCOFFObjectWriter(raw_ostream &OS,
                                                    bool Is64Bit) {
   MCWinCOFFObjectTargetWriter *MOTW = new X86WinCOFFObjectWriter(Is64Bit);
   return createWinCOFFObjectWriter(MOTW, OS);
 }

test/MC/COFF/secidx.s

+// RUN: llvm-mc -filetype=obj -triple i686-pc-win32 %s | llvm-readobj -s -sr | FileCheck %s
+
+// check that we produce the correct relocation for .secidx
+
+Lfoo:
+  .secidx Lfoo
+  .secidx Lbar
+
+.section spam
+Lbar:
+  ret
+
+// CHECK:       Relocations [
+// CHECK-NEXT:    0x0 IMAGE_REL_I386_SECTION .text
+// CHECK-NEXT:    0x4 IMAGE_REL_I386_SECTION spam
+// CHECK-NEXT:  ]