allow COFF .def directive in module assembly when using ThinLTO

[inglorion]

Summary

Using COFF's .def directive in module assembly used to crash ThinLTO
with "this directive only supported on COFF targets" when getting
symbol information in ModuleSymbolTable. This change allows
ModuleSymbolTable to process such code and adds a test to verify that
the .def directive has the desired effect on the native object file,
with and without ThinLTO.

Fixes https://bugs.llvm.org/show_bug.cgi?id=36789

[rnk]

clang/test/CodeGen/inline-asm-coff.c

3–4 ↗

(On Diff #183002)

This test isn't going to do the right thing on Windows ARM, and I don't think we need this high-level of an integration test.

I'd recommend writing a .ll test similar to the llvm/test/Transforms/ThinLTOBitcodeWriter/symver.ll one. You should be able to use the same assembly as module level asm in LLVM IR, and exercise the same behavior.

[inglorion]

replaced .c test with an .ll test

[rnk]

lgtm

[inglorion]

add missing .1

[pcc]

llvm/test/MC/COFF/module-asm-coff.ll

2

This isn't really a test of the MC layer. Can it be simplified to use llvm-nm and moved under test/Object?

[inglorion]

Just saw your comment, pcc. I'll look into that.

[inglorion]

pcc, does D57192 implement your suggestion?

llvm/lib/Object/RecordStreamer.h

void EmitInstruction(const MCInst &Inst, const MCSubtargetInfo &STI,
                        bool) override;
   void EmitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override;
   void EmitAssignment(MCSymbol *Symbol, const MCExpr *Value) override;
   bool EmitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
   void EmitZerofill(MCSection *Section, MCSymbol *Symbol, uint64_t Size,
                     unsigned ByteAlignment, SMLoc Loc = SMLoc()) override;
   void EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         unsigned ByteAlignment) override;
+
+  // Ignore COFF-specific directives; we do not need any information from them,
+  // but the default implementation of these methods crashes, so we override
+  // them with versions that do nothing.
+  void BeginCOFFSymbolDef(const MCSymbol *Symbol) override {}
+  void EmitCOFFSymbolStorageClass(int StorageClass) override {}
+  void EmitCOFFSymbolType(int Type) override {}
+  void EndCOFFSymbolDef() override {}
+
   /// Record .symver aliases for later processing.
   void emitELFSymverDirective(StringRef AliasName,
                               const MCSymbol *Aliasee) override;
 
   // Emit ELF .symver aliases and ensure they have the same binding as the
   // defined symbol they alias with.
   void flushSymverDirectives();
 

llvm/test/MC/COFF/module-asm-coff.ll

+; Tests COFF-specific directives in module level assembly.
+
+; RUN: llc -filetype=obj %s -o %t.obj
+; RUN: llvm-readobj -t %t.obj | FileCheck %s
+; RUN: opt -thinlto-bc %s -o %t.thinlto.bc
+; RUN: llvm-lto2 run %t.thinlto.bc -o %t.thinlto.obj -r=%t.thinlto.bc,foo,plx
+; RUN: llvm-readobj -t %t.thinlto.obj.1 | FileCheck %s
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.0.24210"
+
+module asm ".text"
+module asm ".def foo; .scl 3; .type 32; .endef"
+module asm ".global foo"
+module asm "foo:"
+module asm "  ret"
+
+; CHECK: Symbol {
+; CHECK:   Name: foo
+; CHECK:   StorageClass:
+; CHECK-SAME: Static (0x3)