[debug-info] refactor emitDwarfUnitLength

[shchenz]

Summary

This is from code review comments in D95998, see https://reviews.llvm.org/D95998#2553505

comments from @ikudrin

The idea was to remove the Lo argument from MCStreamer::emitDwarfUnitLength(Hi, Lo, Comment) so that it becomes just MCStreamer::emitDwarfUnitLength(Hi, Comment).

[ikudrin]

llvm/lib/MC/MCStreamer.cpp

1014

The line contains trailing space. Please, remove.

llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp

330–331

Lo is no longer used in the tests and should be removed.

332

Sec should be local to the init method.

339–352

I'd prefer to check that a label passed as the second argument is emitted after calling emitAbsoluteSymbolDiff(), but that requires many changes not directly connected with the purpose of this patch, so that should be done separately. I'll prepare the patch a bit later.

[ikudrin]

@shchenz, please take a look at D96710. When it is accepted, you can integrate it here or commit it separately within the whole set.

[shchenz]

1: address comments

[shchenz]

@MaskRay @ikudrin Thanks for your review. Also thanks for your help with the test case changes, Igor.

[ikudrin]

llvm/test/DebugInfo/X86/addr_comments.ll

4–5 ↗

(On Diff #323747)

It is better to avoid checking specific names of temporary labels. They may change with changes in other parts, which makes the test fragile. What about using substitutions instead?

llvm/test/DebugInfo/X86/dwarf-pubnames-split.ll

10

This label now does not identify the section well and also makes the test fragile. How about this:

; CHECK:      .section .debug_pubtypes
; CHECK-NEXT: .long
; CHECK-NEXT: .Ltmp{{.+}}:
...

llvm/test/DebugInfo/X86/length_symbol_difference.ll

3–4 ↗

(On Diff #323747)

Ditto.

llvm/test/MC/WebAssembly/comdat-sections.ll

20–21 ↗

(On Diff #323747)

The same.

[shchenz]

1: use RE to represent the length label

[shchenz]

@ikudrin Thanks for your comment. I guess LBL is for label length, so I also used it for .debug_info section.

[shchenz]

1: address @dblaikie comments in D95998

[shchenz]

1: integrate D96710

[dblaikie]

Generally looks good to me (please wait for @ikudrin too, to ensure their issues have been addressed)

At some point it'd probably be good to include a second (probably as the first parameter) to this function to use as a prefix for the label names to make them more informative/easier to read/modify/etc. And the function may need to be renamed - it currently includes the word "unit" un it, even though it can be/is used for other DWARF sections, not just the debug_info/debug_types unit sections.

[ikudrin]

(Moving the thread, as it's more suitable here)

In D95998#2572951, @dblaikie wrote:

In D95998#2568193, @ikudrin wrote:

In D95998#2567599, @dblaikie wrote:

What if the function created and returned Hi rather than taking it as an argument?

Hmm, I can't say I'm a vivid fan of this change. All methods of MCStremer instruct it to do something, i.e. it roles as just a performer. With this change, it now requests a caller to do some action. It looks like inverting customer vs. performer roles for this particular case.

In the sense that it now requests the user emit the end label at some point?

More in the sense that it limits the flexibility of calling code without any real profit. For example, the caller now cannot create the label in advance. For instance, Dwarf5AccelTableWriter::ContributionEnd was initialized when the object of that class was created. Simple and solid. Now the initialization is postponed until a method of another class, Dwarf5AccelTableWriter::Header is called. The tracking is a bit more challenging now.

It seems pretty tidy to me, and I'm pretty sure we follow this pattern elsewhere in the CodeGen/AsmPrinter code.

Could you get an example? I can't remember anything similar.

[ikudrin]

In D96409#2572715, @dblaikie wrote:

At some point it'd probably be good to include a second (probably as the first parameter) to this function to use as a prefix for the label names to make them more informative/easier to read/modify/etc.

Maybe it'll make sense to do that in this patch. There will be fewer changes in the tests.

And the function may need to be renamed - it currently includes the word "unit" un it, even though it can be/is used for other DWARF sections, not just the debug_info/debug_types unit sections.

The method emits a unit length field. The name is used throughout the DWARF standard for all kinds of tables. And it hardly can be used for anything else because it may emit a DWARF64 mark.

[dblaikie]

In D96409#2574186, @ikudrin wrote:

(Moving the thread, as it's more suitable here)

In D95998#2572951, @dblaikie wrote:

In D95998#2568193, @ikudrin wrote:

In D95998#2567599, @dblaikie wrote:

What if the function created and returned Hi rather than taking it as an argument?

Hmm, I can't say I'm a vivid fan of this change. All methods of MCStremer instruct it to do something, i.e. it roles as just a performer. With this change, it now requests a caller to do some action. It looks like inverting customer vs. performer roles for this particular case.

In the sense that it now requests the user emit the end label at some point?

More in the sense that it limits the flexibility of calling code without any real profit. For example, the caller now cannot create the label in advance.

Any particular benefit to being able to do that, though? I think this is similar to rolling "Lo" into the function - the caller now can't reuse that label, but it doesn't seem especially important/beneficial to be able to reuse it, compared to simplifying the API.

For instance, Dwarf5AccelTableWriter::ContributionEnd was initialized when the object of that class was created. Simple and solid. Now the initialization is postponed until a method of another class, Dwarf5AccelTableWriter::Header is called. The tracking is a bit more challenging now.

Fair - though I'm not sure it's a huge impediment. If the label weren't used in the header it'd probably be a bug - one that hide more silently with the code as-is (perhaps the header code would end up using some other label that is never defined/emitted - and then the assembler would produce some error about that invariant being violated).

It seems pretty tidy to me, and I'm pretty sure we follow this pattern elsewhere in the CodeGen/AsmPrinter code.

Could you get an example? I can't remember anything similar.

Let's see what I can find...

mcdwarf::emitListsTableHeaderStart (& related APIs like emitLoclistsTableHeader)
AddressPool::emitHeader

Those are a couple I could find at a glance at least.

[shchenz]

It seems pretty tidy to me, and I'm pretty sure we follow this pattern elsewhere in the CodeGen/AsmPrinter code.

Could you get an example? I can't remember anything similar.

Let's see what I can find...

mcdwarf::emitListsTableHeaderStart (& related APIs like emitLoclistsTableHeader)
AddressPool::emitHeader

Those are a couple I could find at a glance at least.

Is std::pair<MCSymbol *, MCSymbol *>MCDwarfLineTableHeader::Emit( also an example for this? It returns LineEndSym to its caller and its caller will emits the label.

In D96409#2574195, @ikudrin wrote:

In D96409#2572715, @dblaikie wrote:

At some point it'd probably be good to include a second (probably as the first parameter) to this function to use as a prefix for the label names to make them more informative/easier to read/modify/etc.

Maybe it'll make sense to do that in this patch. There will be fewer changes in the tests.

Sure, I will update this when the API design is settled.

[ikudrin]

In D96409#2575680, @dblaikie wrote:

In D96409#2574186, @ikudrin wrote:

More in the sense that it limits the flexibility of calling code without any real profit. For example, the caller now cannot create the label in advance.

Any particular benefit to being able to do that, though? I think this is similar to rolling "Lo" into the function - the caller now can't reuse that label, but it doesn't seem especially important/beneficial to be able to reuse it, compared to simplifying the API.

Why should a service class like MCStreamer define the architecture of its clients? It is just not its area of responsibility.

For instance, Dwarf5AccelTableWriter::ContributionEnd was initialized when the object of that class was created. Simple and solid. Now the initialization is postponed until a method of another class, Dwarf5AccelTableWriter::Header is called. The tracking is a bit more challenging now.

Fair - though I'm not sure it's a huge impediment. If the label weren't used in the header it'd probably be a bug - one that hide more silently with the code as-is (perhaps the header code would end up using some other label that is never defined/emitted - and then the assembler would produce some error about that invariant being violated).

I agree that the difference is subtle. If I am not able to convince you, well, so be it. Not a big deal. Just a nice theoretical prattling.

It seems pretty tidy to me, and I'm pretty sure we follow this pattern elsewhere in the CodeGen/AsmPrinter code.

Could you get an example? I can't remember anything similar.

Let's see what I can find...

mcdwarf::emitListsTableHeaderStart (& related APIs like emitLoclistsTableHeader)
AddressPool::emitHeader

Those are a couple I could find at a glance at least.

Yeah, they seem similar but aren't they merely helper methods for a limited number of well-known callers? AddressPool::emitHeader is even private. They are not methods in a distant service class, where callers are not known in advance.

In D96409#2576024, @shchenz wrote:

Is std::pair<MCSymbol *, MCSymbol *>MCDwarfLineTableHeader::Emit( also an example for this? It returns LineEndSym to its caller and its caller will emits the label.

Note that MCDwarfLineTableHeader is a private class of MCDwarfLineTable. So this is also an implementation peculiarity of MCDwarfLineTable which does not affect others.

[shchenz]

1: add symbol prefix as a parameter of emitDwarfUnitLength

[shchenz]

Thanks for your review @ikudrin @dblaikie Both of you provide good thoughts about how to design a fundamental API. Let's design emitDwarfUnitLength like this for now. If we find some obvious weakness in the future, I think it is not hard for us to change to another design.

[dblaikie]

Seems good to me. Wouldn't mind second set of eyes especially on the unit test - the mock stuff I'm not especially familiar with.

[shchenz]

@dblaikie thanks for your review, David.
@ikudrin could you please help to have another look, Igor? Do you have any objections if I start to commit the patches like D95932, D95998? Thanks.

[ikudrin]

LGTM, thanks!

llvm/lib/MC/MCStreamer.cpp

1013–1014

Maybe use "_start" and "_end" here and avoid passing the trailing '_' in the argument?

[shchenz]

llvm/lib/MC/MCStreamer.cpp

1013–1014

hmm, if Prefix is passed as "", will it be a little strange to have a label like: .L_start0?

[ikudrin]

llvm/lib/MC/MCStreamer.cpp

1013–1014

The signature of the method does not encourage a caller to pass an empty string because there is no default value for the argument. To gracefully support the empty string, it would be better not to add a suffix in that case. But we probably do not need that kind of fine-tuning right now.

[shchenz]

llvm/lib/MC/MCStreamer.cpp

1013–1014

OK, I will update this later. Thanks.

[shchenz]

1: put "_" in the function body to avoid strange string parameters

llvm/include/llvm/CodeGen/AsmPrinter.h

public:
   void emitDwarfLengthOrOffset(uint64_t Value) const;
 
   /// Emit a unit length field. The actual format, DWARF32 or DWARF64, is chosen
   /// according to the settings.
   void emitDwarfUnitLength(uint64_t Length, const Twine &Comment) const;
 
   /// Emit a unit length field. The actual format, DWARF32 or DWARF64, is chosen
   /// according to the settings.
-  void emitDwarfUnitLength(const MCSymbol *Hi, const MCSymbol *Lo,
+  /// Return the end symbol generated inside, the caller needs to emit it.
+  MCSymbol *emitDwarfUnitLength(const Twine &Prefix,
                                 const Twine &Comment) const;
 
   /// Emit reference to a call site with a specified encoding
   void emitCallSiteOffset(const MCSymbol *Hi, const MCSymbol *Lo,
                           unsigned Encoding) const;
   /// Emit an integer value corresponding to the call site encoding
   void emitCallSiteValue(uint64_t Value, unsigned Encoding) const;
 
   /// Get the value for DW_AT_APPLE_isa. Zero if no isa encoding specified.

llvm/include/llvm/MC/MCStreamer.h

public:
   void maybeEmitDwarf64Mark();
 
   /// Emit a unit length field. The actual format, DWARF32 or DWARF64, is chosen
   /// according to the settings.
   virtual void emitDwarfUnitLength(uint64_t Length, const Twine &Comment);
 
   /// Emit a unit length field. The actual format, DWARF32 or DWARF64, is chosen
   /// according to the settings.
-  virtual void emitDwarfUnitLength(const MCSymbol *Hi, const MCSymbol *Lo,
+  /// Return the end symbol generated inside, the caller needs to emit it.
+  virtual MCSymbol *emitDwarfUnitLength(const Twine &Prefix,
                                         const Twine &Comment);
 };
 
 /// Create a dummy machine code streamer, which does nothing. This is useful for
 /// timing the assembler front end.
 MCStreamer *createNullStreamer(MCContext &Ctx);
 
 } // end namespace llvm
 
 #endif // LLVM_MC_MCSTREAMER_H

llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp

struct Header {
     uint32_t AbbrevTableSize = 0;
     uint32_t AugmentationStringSize = sizeof(AugmentationString);
     char AugmentationString[8] = {'L', 'L', 'V', 'M', '0', '7', '0', '0'};
 
     Header(uint32_t CompUnitCount, uint32_t BucketCount, uint32_t NameCount)
         : CompUnitCount(CompUnitCount), BucketCount(BucketCount),
           NameCount(NameCount) {}
 
-    void emit(const Dwarf5AccelTableWriter &Ctx) const;
+    void emit(Dwarf5AccelTableWriter &Ctx);
   };
   struct AttributeEncoding {
     dwarf::Index Index;
     dwarf::Form Form;
   };
 
   Header Header;
   DenseMap<uint32_t, SmallVector<AttributeEncoding, 2>> Abbreviations;
   ArrayRef<MCSymbol *> CompUnits;
   llvm::function_ref<unsigned(const DataT &)> getCUIndexForEntry;
-  MCSymbol *ContributionStart = Asm->createTempSymbol("names_start");
-  MCSymbol *ContributionEnd = Asm->createTempSymbol("names_end");
+  MCSymbol *ContributionEnd = nullptr;
   MCSymbol *AbbrevStart = Asm->createTempSymbol("names_abbrev_start");
   MCSymbol *AbbrevEnd = Asm->createTempSymbol("names_abbrev_end");
   MCSymbol *EntryPool = Asm->createTempSymbol("names_entries");
 
   DenseSet<uint32_t> getUniqueTags() const;
 
   // Right now, we emit uniform attributes for all tags.
   SmallVector<AttributeEncoding, 2> getUniformAttributes() const;
 
   void emitCUList() const;
   void emitBuckets() const;
   void emitStringOffsets() const;
   void emitAbbrevs() const;
   void emitEntry(const DataT &Entry) const;
   void emitData() const;
 
 public:
   Dwarf5AccelTableWriter(
       AsmPrinter *Asm, const AccelTableBase &Contents,
       ArrayRef<MCSymbol *> CompUnits,
       llvm::function_ref<unsigned(const DataT &)> GetCUIndexForEntry);
 
-  void emit() const;
+  void emit();
 };
 } // namespace
 
 void AccelTableWriter::emitHashes() const {
   uint64_t PrevHash = std::numeric_limits<uint64_t>::max();
   unsigned BucketIdx = 0;
   for (auto &Bucket : Contents.getBuckets()) {
     for (auto &Hash : Bucket) {
void AppleAccelTableWriter::emit() const {
   HeaderData.emit(Asm);
   emitBuckets();
   emitHashes();
   emitOffsets(SecBegin);
   emitData();
 }
 
 template <typename DataT>
-void Dwarf5AccelTableWriter<DataT>::Header::emit(
-    const Dwarf5AccelTableWriter &Ctx) const {
+void Dwarf5AccelTableWriter<DataT>::Header::emit(Dwarf5AccelTableWriter &Ctx) {
   assert(CompUnitCount > 0 && "Index must have at least one CU.");
 
   AsmPrinter *Asm = Ctx.Asm;
-  Asm->emitDwarfUnitLength(Ctx.ContributionEnd, Ctx.ContributionStart,
-                           "Header: unit length");
-  Asm->OutStreamer->emitLabel(Ctx.ContributionStart);
+  Ctx.ContributionEnd =
+      Asm->emitDwarfUnitLength("names", "Header: unit length");
   Asm->OutStreamer->AddComment("Header: version");
   Asm->emitInt16(Version);
   Asm->OutStreamer->AddComment("Header: padding");
   Asm->emitInt16(Padding);
   Asm->OutStreamer->AddComment("Header: compilation unit count");
   Asm->emitInt32(CompUnitCount);
   Asm->OutStreamer->AddComment("Header: local type unit count");
   Asm->emitInt32(LocalTypeUnitCount);
Dwarf5AccelTableWriter<DataT>::Dwarf5AccelTableWriter(
   DenseSet<uint32_t> UniqueTags = getUniqueTags();
   SmallVector<AttributeEncoding, 2> UniformAttributes = getUniformAttributes();
 
   Abbreviations.reserve(UniqueTags.size());
   for (uint32_t Tag : UniqueTags)
     Abbreviations.try_emplace(Tag, UniformAttributes);
 }
 
-template <typename DataT> void Dwarf5AccelTableWriter<DataT>::emit() const {
+template <typename DataT> void Dwarf5AccelTableWriter<DataT>::emit() {
   Header.emit(*this);
   emitCUList();
   emitBuckets();
   emitHashes();
   emitStringOffsets();
   emitOffsets(EntryPool);
   emitAbbrevs();
   emitData();

llvm/lib/CodeGen/AsmPrinter/AddressPool.cpp

unsigned AddressPool::getIndex(const MCSymbol *Sym, bool TLS) {
   HasBeenUsed = true;
   auto IterBool =
       Pool.insert(std::make_pair(Sym, AddressPoolEntry(Pool.size(), TLS)));
   return IterBool.first->second.Number;
 }
 
 MCSymbol *AddressPool::emitHeader(AsmPrinter &Asm, MCSection *Section) {
   static const uint8_t AddrSize = Asm.getDataLayout().getPointerSize();
-  StringRef Prefix = "debug_addr_";
-  MCSymbol *BeginLabel = Asm.createTempSymbol(Prefix + "start");
-  MCSymbol *EndLabel = Asm.createTempSymbol(Prefix + "end");
 
-  Asm.emitDwarfUnitLength(EndLabel, BeginLabel, "Length of contribution");
-  Asm.OutStreamer->emitLabel(BeginLabel);
+  MCSymbol *EndLabel =
+      Asm.emitDwarfUnitLength("debug_addr", "Length of contribution");
   Asm.OutStreamer->AddComment("DWARF version number");
   Asm.emitInt16(Asm.getDwarfVersion());
   Asm.OutStreamer->AddComment("Address size");
   Asm.emitInt8(AddrSize);
   Asm.OutStreamer->AddComment("Segment selector size");
   Asm.emitInt8(0); // TODO: Support non-zero segment_selector_size.
 
   return EndLabel;

llvm/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp

void AsmPrinter::emitDwarfLengthOrOffset(uint64_t Value) const {
   OutStreamer->emitIntValue(Value, getDwarfOffsetByteSize());
 }
 
 void AsmPrinter::emitDwarfUnitLength(uint64_t Length,
                                      const Twine &Comment) const {
   OutStreamer->emitDwarfUnitLength(Length, Comment);
 }
 
-void AsmPrinter::emitDwarfUnitLength(const MCSymbol *Hi, const MCSymbol *Lo,
+MCSymbol *AsmPrinter::emitDwarfUnitLength(const Twine &Prefix,
                                           const Twine &Comment) const {
-  OutStreamer->emitDwarfUnitLength(Hi, Lo, Comment);
+  return OutStreamer->emitDwarfUnitLength(Prefix, Comment);
 }
 
 void AsmPrinter::emitCallSiteOffset(const MCSymbol *Hi, const MCSymbol *Lo,
                                     unsigned Encoding) const {
   // The least significant 3 bits specify the width of the encoding
   if ((Encoding & 0x7) == dwarf::DW_EH_PE_uleb128)
     emitLabelDifferenceAsULEB128(Hi, Lo);
   else

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

 
 void DwarfDebug::emitDebugPubSection(bool GnuStyle, StringRef Name,
                                      DwarfCompileUnit *TheU,
                                      const StringMap<const DIE *> &Globals) {
   if (auto *Skeleton = TheU->getSkeleton())
     TheU = Skeleton;
 
   // Emit the header.
-  MCSymbol *BeginLabel = Asm->createTempSymbol("pub" + Name + "_begin");
-  MCSymbol *EndLabel = Asm->createTempSymbol("pub" + Name + "_end");
-  Asm->emitDwarfUnitLength(EndLabel, BeginLabel,
-                           "Length of Public " + Name + " Info");
-
-  Asm->OutStreamer->emitLabel(BeginLabel);
+  MCSymbol *EndLabel = Asm->emitDwarfUnitLength(
+      "pub" + Name, "Length of Public " + Name + " Info");
 
   Asm->OutStreamer->AddComment("DWARF Version");
   Asm->emitInt16(dwarf::DW_PUBNAMES_VERSION);
 
   Asm->OutStreamer->AddComment("Offset of Compilation Unit Info");
   emitSectionReference(*TheU);
 
   Asm->OutStreamer->AddComment("Compilation Unit Length");

llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

if (uint32_t AlignInBytes = DT->getAlignInBytes())
     addUInt(StaticMemberDIE, dwarf::DW_AT_alignment, dwarf::DW_FORM_udata,
             AlignInBytes);
 
   return &StaticMemberDIE;
 }
 
 void DwarfUnit::emitCommonHeader(bool UseOffsets, dwarf::UnitType UT) {
   // Emit size of content not including length itself
-  if (!DD->useSectionsAsReferences()) {
-    StringRef Prefix = isDwoUnit() ? "debug_info_dwo_" : "debug_info_";
-    MCSymbol *BeginLabel = Asm->createTempSymbol(Prefix + "start");
-    EndLabel = Asm->createTempSymbol(Prefix + "end");
-    Asm->emitDwarfUnitLength(EndLabel, BeginLabel, "Length of Unit");
-    Asm->OutStreamer->emitLabel(BeginLabel);
-  } else
+  if (!DD->useSectionsAsReferences())
+    EndLabel = Asm->emitDwarfUnitLength(
+        isDwoUnit() ? "debug_info_dwo" : "debug_info", "Length of Unit");
+  else
     Asm->emitDwarfUnitLength(getHeaderSize() + getUnitDie().getSize(),
                              "Length of Unit");
 
   Asm->OutStreamer->AddComment("DWARF version number");
   unsigned Version = DD->getDwarfVersion();
   Asm->emitInt16(Version);
 
   // DWARF v5 reorders the address size and adds a unit type.

llvm/lib/MC/MCStreamer.cpp

 void MCStreamer::emitDwarfUnitLength(uint64_t Length, const Twine &Comment) {
   assert(Context.getDwarfFormat() == dwarf::DWARF64 ||
          Length <= dwarf::DW_LENGTH_lo_reserved);
   maybeEmitDwarf64Mark();
   AddComment(Comment);
   emitIntValue(Length, dwarf::getDwarfOffsetByteSize(Context.getDwarfFormat()));
 }
 
-void MCStreamer::emitDwarfUnitLength(const MCSymbol *Hi, const MCSymbol *Lo,
+MCSymbol *MCStreamer::emitDwarfUnitLength(const Twine &Prefix,
                                           const Twine &Comment) {
   maybeEmitDwarf64Mark();
   AddComment(Comment);
+  MCSymbol *Lo = Context.createTempSymbol(Prefix + "_start");
+  MCSymbol *Hi = Context.createTempSymbol(Prefix + "_end");
+
   emitAbsoluteSymbolDiff(
       Hi, Lo, dwarf::getDwarfOffsetByteSize(Context.getDwarfFormat()));
+  // emit the begin symbol after we generate the length field.
+  emitLabel(Lo);
+  // Return the Hi symbol to the caller.
+  return Hi;
 }
 
 void MCStreamer::emitAssignment(MCSymbol *Symbol, const MCExpr *Value) {
   visitUsedExpr(*Value);
   Symbol->setVariableValue(Value);
 
   MCTargetStreamer *TS = getTargetStreamer();
   if (TS)

llvm/test/DebugInfo/X86/dwarf-pubnames-split.ll

 ; RUN: llc -mtriple=x86_64-pc-linux-gnu -split-dwarf-file=foo.dwo %s -o - | FileCheck %s
 ; Derived from:
 
 ; int main (void) {
 ;    return 0;
 ; }
 
 ; Check that we get a symbol off of the debug_info section when using split dwarf and pubnames.
 
-; CHECK: .LpubTypes_begin0:
+; CHECK: .LpubTypes_start0:
 ; CHECK-NEXT: .short    2                       # DWARF Version
 ; CHECK-NEXT: .long     .Lcu_begin0             # Offset of Compilation Unit Info
 
 ; Function Attrs: nounwind uwtable
 define i32 @main() #0 !dbg !4 {
 entry:
   %retval = alloca i32, align 4
   store i32 0, i32* %retval

llvm/unittests/CodeGen/AsmPrinterDwarfTest.cpp

 
 class AsmPrinterEmitDwarfSymbolReferenceTest : public AsmPrinterFixtureBase {
 protected:
   bool init(const std::string &TripleStr, unsigned DwarfVersion,
             dwarf::DwarfFormat DwarfFormat) {
     if (!AsmPrinterFixtureBase::init(TripleStr, DwarfVersion, DwarfFormat))
       return false;
 
-    // Create a symbol which will be emitted in the tests and associate it
-    // with a section because that is required in some code paths.
+    // AsmPrinter::emitDwarfSymbolReference(Label, true) gets the associated
+    // section from `Label` to find its BeginSymbol.
+    // Prepare the test symbol `Val` accordingly.
 
     Val = TestPrinter->getCtx().createTempSymbol();
-    Sec = TestPrinter->getCtx().getELFSection(".tst", ELF::SHT_PROGBITS, 0);
+    MCSection *Sec =
+        TestPrinter->getCtx().getELFSection(".tst", ELF::SHT_PROGBITS, 0);
     SecBeginSymbol = Sec->getBeginSymbol();
     TestPrinter->getMS().SwitchSection(Sec);
-    TestPrinter->getMS().emitLabel(Val);
+    Val->setFragment(&Sec->getDummyFragment());
+
     return true;
   }
 
   MCSymbol *Val = nullptr;
-  MCSection *Sec = nullptr;
   MCSymbol *SecBeginSymbol = nullptr;
 };
 
 TEST_F(AsmPrinterEmitDwarfSymbolReferenceTest, COFF) {
   if (!init("x86_64-pc-windows", /*DwarfVersion=*/4, dwarf::DWARF32))
     return;
 
   EXPECT_CALL(TestPrinter->getMS(), EmitCOFFSecRel32(Val, 0));

 
 class AsmPrinterEmitDwarfUnitLengthAsHiLoDiffTest
     : public AsmPrinterFixtureBase {
 protected:
   bool init(const std::string &TripleStr, unsigned DwarfVersion,
             dwarf::DwarfFormat DwarfFormat) {
     if (!AsmPrinterFixtureBase::init(TripleStr, DwarfVersion, DwarfFormat))
       return false;
 
-    Hi = TestPrinter->getCtx().createTempSymbol();
-    Lo = TestPrinter->getCtx().createTempSymbol();
     return true;
   }
-
-  MCSymbol *Hi = nullptr;
-  MCSymbol *Lo = nullptr;
 };
 
 TEST_F(AsmPrinterEmitDwarfUnitLengthAsHiLoDiffTest, DWARF32) {
   if (!init("x86_64-pc-linux", /*DwarfVersion=*/4, dwarf::DWARF32))
     return;
 
-  EXPECT_CALL(TestPrinter->getMS(), emitAbsoluteSymbolDiff(Hi, Lo, 4));
-  TestPrinter->getAP()->emitDwarfUnitLength(Hi, Lo, "");
+  InSequence S;
+  const MCSymbol *Hi = nullptr;
+  const MCSymbol *Lo = nullptr;
+  EXPECT_CALL(TestPrinter->getMS(), emitAbsoluteSymbolDiff(_, _, 4))
+      .WillOnce(DoAll(SaveArg<0>(&Hi), SaveArg<1>(&Lo)));
+  MCSymbol *LTmp = nullptr;
+  EXPECT_CALL(TestPrinter->getMS(), emitLabel(_, _))
+      .WillOnce(SaveArg<0>(&LTmp));
+
+  MCSymbol *HTmp = TestPrinter->getAP()->emitDwarfUnitLength("", "");
+  EXPECT_NE(Lo, nullptr);
+  EXPECT_EQ(Lo, LTmp);
+  EXPECT_NE(Hi, nullptr);
+  EXPECT_EQ(Hi, HTmp);
 }
 
 TEST_F(AsmPrinterEmitDwarfUnitLengthAsHiLoDiffTest, DWARF64) {
   if (!init("x86_64-pc-linux", /*DwarfVersion=*/4, dwarf::DWARF64))
     return;
 
   InSequence S;
+  const MCSymbol *Hi = nullptr;
+  const MCSymbol *Lo = nullptr;
   EXPECT_CALL(TestPrinter->getMS(), emitIntValue(dwarf::DW_LENGTH_DWARF64, 4));
-  EXPECT_CALL(TestPrinter->getMS(), emitAbsoluteSymbolDiff(Hi, Lo, 8));
-
-  TestPrinter->getAP()->emitDwarfUnitLength(Hi, Lo, "");
+  EXPECT_CALL(TestPrinter->getMS(), emitAbsoluteSymbolDiff(_, _, 8))
+      .WillOnce(DoAll(SaveArg<0>(&Hi), SaveArg<1>(&Lo)));
+  MCSymbol *LTmp = nullptr;
+  EXPECT_CALL(TestPrinter->getMS(), emitLabel(_, _))
+      .WillOnce(SaveArg<0>(&LTmp));
+
+  MCSymbol *HTmp = TestPrinter->getAP()->emitDwarfUnitLength("", "");
+  EXPECT_NE(Lo, nullptr);
+  EXPECT_EQ(Lo, LTmp);
+  EXPECT_NE(Hi, nullptr);
+  EXPECT_EQ(Hi, HTmp);
 }
 
 class AsmPrinterHandlerTest : public AsmPrinterFixtureBase {
   class TestHandler : public AsmPrinterHandler {
     AsmPrinterHandlerTest &Test;
 
   public:
     TestHandler(AsmPrinterHandlerTest &Test) : Test(Test) {}

llvm/unittests/CodeGen/TestAsmPrinter.h

public:
   MOCK_METHOD3(emitCommonSymbol,
                void(MCSymbol *Symbol, uint64_t Size, unsigned ByteAlignment));
   MOCK_METHOD5(emitZerofill,
                void(MCSection *Section, MCSymbol *Symbol, uint64_t Size,
                     unsigned ByteAlignment, SMLoc Loc));
 
   // The following are mock methods to be used in tests.
 
+  MOCK_METHOD2(emitLabel, void(MCSymbol *Symbol, SMLoc Loc));
   MOCK_METHOD2(emitIntValue, void(uint64_t Value, unsigned Size));
   MOCK_METHOD3(emitValueImpl,
                void(const MCExpr *Value, unsigned Size, SMLoc Loc));
   MOCK_METHOD3(emitAbsoluteSymbolDiff,
                void(const MCSymbol *Hi, const MCSymbol *Lo, unsigned Size));
   MOCK_METHOD2(EmitCOFFSecRel32, void(MCSymbol const *Symbol, uint64_t Offset));
 };