[DebugInfo] Improve tests for AsmPrinter::emitDwarfUnitLength()

[shchenz]

Summary

This is a follow-up for D96409 to add checks for the label that is used as a lower bound for the unit length.

[shchenz]

LGTM. Thanks for doing this.
It is better to wait for another reviewer's comments for some days.

[dblaikie]

Sure, sounds OK. Though a lot of this is probably OK/already tested by assembly level tests, I'd expect.

[ikudrin]

In D96710#2566459, @dblaikie wrote:

Sure, sounds OK. Though a lot of this is probably OK/already tested by assembly level tests, I'd expect.

Thanks! Right, there are other tests that go through this path, but their purposes are different, so they might be changed. Contrary, these are unit tests, and they are focused on the expected behavior of particular methods.

[shchenz]

@ikudrin Hi, we made another change for emitDwarfUnitLength in D96409, I think this patch needs to be further improved for Hi parameter. Let me know if you need me to update this patch. Thanks.

[ikudrin]

In D96710#2567786, @shchenz wrote:

@ikudrin Hi, we made another change for emitDwarfUnitLength in D96409, I think this patch needs to be further improved for Hi parameter. Let me know if you need me to update this patch. Thanks.

I believe it would be more convenient for everyone if you just integrate this change into D96409 and take care of all adjustments there. Or you may commandeer this revision, whatever you prefer.

[shchenz]

In D96710#2568111, @ikudrin wrote:

In D96710#2567786, @shchenz wrote:

@ikudrin Hi, we made another change for emitDwarfUnitLength in D96409, I think this patch needs to be further improved for Hi parameter. Let me know if you need me to update this patch. Thanks.

I believe it would be more convenient for everyone if you just integrate this change into D96409 and take care of all adjustments there. Or you may commandeer this revision, whatever you prefer.

I have integrated this patch to D96409. Thanks for your patch @ikudrin

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();
 
-    Sec = TestPrinter->getCtx().getELFSection(".tst", ELF::SHT_PROGBITS, 0);
-    TestPrinter->getMS().SwitchSection(Sec);
     return true;
   }
 
   MCSymbol *Hi = nullptr;
-  MCSymbol *Lo = nullptr;
-  MCSection *Sec = nullptr;
 };
 
 TEST_F(AsmPrinterEmitDwarfUnitLengthAsHiLoDiffTest, DWARF32) {
   if (!init("x86_64-pc-linux", /*DwarfVersion=*/4, dwarf::DWARF32))
     return;
 
-  EXPECT_CALL(TestPrinter->getMS(), emitAbsoluteSymbolDiff(Hi, _, 4));
+  InSequence S;
+  const MCSymbol *Lo = nullptr;
+  EXPECT_CALL(TestPrinter->getMS(), emitAbsoluteSymbolDiff(Hi, _, 4))
+      .WillOnce(SaveArg<1>(&Lo));
+  MCSymbol *LTmp = nullptr;
+  EXPECT_CALL(TestPrinter->getMS(), emitLabel(_, _))
+      .WillOnce(SaveArg<0>(&LTmp));
+
   TestPrinter->getAP()->emitDwarfUnitLength(Hi, "");
+  EXPECT_NE(Lo, nullptr);
+  EXPECT_EQ(Lo, LTmp);
 }
 
 TEST_F(AsmPrinterEmitDwarfUnitLengthAsHiLoDiffTest, DWARF64) {
   if (!init("x86_64-pc-linux", /*DwarfVersion=*/4, dwarf::DWARF64))
     return;
 
   InSequence S;
   EXPECT_CALL(TestPrinter->getMS(), emitIntValue(dwarf::DW_LENGTH_DWARF64, 4));
-  EXPECT_CALL(TestPrinter->getMS(), emitAbsoluteSymbolDiff(Hi, _, 8));
+  const MCSymbol *Lo = nullptr;
+  EXPECT_CALL(TestPrinter->getMS(), emitAbsoluteSymbolDiff(Hi, _, 8))
+      .WillOnce(SaveArg<1>(&Lo));
+  MCSymbol *LTmp = nullptr;
+  EXPECT_CALL(TestPrinter->getMS(), emitLabel(_, _))
+      .WillOnce(SaveArg<0>(&LTmp));
 
   TestPrinter->getAP()->emitDwarfUnitLength(Hi, "");
+  EXPECT_NE(Lo, nullptr);
+  EXPECT_EQ(Lo, LTmp);
 }
 
 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));
 };