[DWARFLinker] Fix matching logic to remove type 1 missing offset (#151427)

Reverts the [revert](#151424)
and fixed some typos.

Original PR description:

Second attempt to fix
https://discourse.llvm.org/t/rfc-new-dwarf-attribute-for-symbolication-of-merged-functions/79434/29?u=alx32

(First attempt: #143656)

Context: the sequence offset to row index we parsed may not be complete.
And we need to add manual matching to it.

#143656 attempts to do trivial
1:1 matching, however, sometimes they don't line up perfectly, as shown
below:

// While SeqOffToOrigRow parsed from CU could be the ground truth,
         // e.g.
         //
         // SeqOff     Row
         // 0x08        9
         // 0x14       15
         //
         // The StmtAttrs and SeqStartRows may not match perfectly, e.g.
         //
         // StmtAttrs  SeqStartRows
         // 0x04        3
         // 0x08        5
         // 0x10        9
         // 0x12       11
         // 0x14       15
         //
// In this case, we don't want to assign 5 to 0x08, since we know 0x08
// maps to 9. If we do a dummy 1:1 mapping 0x10 will be mapped to 9
// which is incorrect. The expected behavior is ignore 5, realign the
         // table based on the result from the line table:
         //
         // StmtAttrs  SeqStartRows
         // 0x04        3
         //   --        5
         // 0x08        9 <- LineTableMapping ground truth
         // 0x10       11
         // 0x12       --
         // 0x14       15 <- LineTableMapping ground truth
In this case, we need to use the mapping we read from the line table as
a ground truth and organize them properly to prevent duplicated
offset/missing offset.

Test:

Updated the test case

---------

Signed-off-by: Peter Rong <[email protected]>
Co-authored-by: Ellis Hoag <[email protected]>

Author: DataCorrupted

llvm/lib/DWARFLinker/Classic/DWARFLinker.cpp

@@ -413,6 +413,117 @@ static bool isTlsAddressCode(uint8_t DW_OP_Code) {
          DW_OP_Code == dwarf::DW_OP_GNU_push_tls_address;
 }
 
+static void constructSeqOffsettoOrigRowMapping(
+    CompileUnit &Unit, const DWARFDebugLine::LineTable &LT,
+    DenseMap<uint64_t, unsigned> &SeqOffToOrigRow) {
+
+  // Use std::map for ordered iteration.
+  std::map<uint64_t, unsigned> LineTableMapping;
+
+  // First, trust the sequences that the DWARF parser did identify.
+  for (const DWARFDebugLine::Sequence &Seq : LT.Sequences)
+    LineTableMapping[Seq.StmtSeqOffset] = Seq.FirstRowIndex;
+
+  // Second, manually find sequence boundaries and match them to the
+  // sorted attributes to handle sequences the parser might have missed.
+  auto StmtAttrs = Unit.getStmtSeqListAttributes();
+  llvm::sort(StmtAttrs, [](const PatchLocation &A, const PatchLocation &B) {
+    return A.get() < B.get();
+  });
+
+  std::vector<unsigned> SeqStartRows;
+  SeqStartRows.push_back(0);
+  for (auto [I, Row] : llvm::enumerate(ArrayRef(LT.Rows).drop_back()))
+    if (Row.EndSequence)
+      SeqStartRows.push_back(I + 1);
+
+  // While SeqOffToOrigRow parsed from CU could be the ground truth,
+  // e.g.
+  //
+  // SeqOff     Row
+  // 0x08        9
+  // 0x14       15
+  //
+  // The StmtAttrs and SeqStartRows may not match perfectly, e.g.
+  //
+  // StmtAttrs  SeqStartRows
+  // 0x04        3
+  // 0x08        5
+  // 0x10        9
+  // 0x12       11
+  // 0x14       15
+  //
+  // In this case, we don't want to assign 5 to 0x08, since we know 0x08
+  // maps to 9. If we do a dummy 1:1 mapping 0x10 will be mapped to 9
+  // which is incorrect. The expected behavior is ignore 5, realign the
+  // table based on the result from the line table:
+  //
+  // StmtAttrs  SeqStartRows
+  // 0x04        3
+  //   --        5
+  // 0x08        9 <- LineTableMapping ground truth
+  // 0x10       11
+  // 0x12       --
+  // 0x14       15 <- LineTableMapping ground truth
+
+  ArrayRef StmtAttrsRef(StmtAttrs);
+  ArrayRef SeqStartRowsRef(SeqStartRows);
+
+  // Dummy last element to make sure StmtAttrsRef and SeqStartRowsRef always
+  // run out first.
+  constexpr uint64_t DummyKey = UINT64_MAX;
+  constexpr unsigned DummyVal = UINT32_MAX;
+  LineTableMapping[DummyKey] = DummyVal;
+
+  for (auto [NextSeqOff, NextRow] : LineTableMapping) {
+    // Explict capture to avoid capturing structured bindings and make C++17
+    // happy.
+    auto StmtAttrSmallerThanNext = [N = NextSeqOff](const PatchLocation &SA) {
+      return SA.get() < N;
+    };
+    auto SeqStartSmallerThanNext = [N = NextRow](const unsigned &Row) {
+      return Row < N;
+    };
+    // If both StmtAttrs and SeqStartRows points to value not in
+    // the LineTableMapping yet, we do a dummy one to one mapping and
+    // move the pointer.
+    while (!StmtAttrsRef.empty() && !SeqStartRowsRef.empty() &&
+           StmtAttrSmallerThanNext(StmtAttrsRef.front()) &&
+           SeqStartSmallerThanNext(SeqStartRowsRef.front())) {
+      SeqOffToOrigRow[StmtAttrsRef.consume_front().get()] =
+          SeqStartRowsRef.consume_front();
+    }
+    // One of the pointer points to the value at or past Next in the
+    // LineTableMapping, We move the pointer to re-align with the
+    // LineTableMapping
+    StmtAttrsRef = StmtAttrsRef.drop_while(StmtAttrSmallerThanNext);
+    SeqStartRowsRef = SeqStartRowsRef.drop_while(SeqStartSmallerThanNext);
+    // Use the LineTableMapping's result as the ground truth and move
+    // on.
+    if (NextSeqOff != DummyKey) {
+      SeqOffToOrigRow[NextSeqOff] = NextRow;
+    }
+    // Move the pointers if they are pointed at Next.
+    // It is possible that they point to later entries in LineTableMapping.
+    // Therefore we only increment the pointers after we validate they are
+    // pointing to the `Next` entry. e.g.
+    //
+    // LineTableMapping
+    // SeqOff      Row
+    // 0x08         9    <- NextSeqOff/NextRow
+    // 0x14        15
+    //
+    // StmtAttrs  SeqStartRows
+    // 0x14       13    <- StmtAttrsRef.front() / SeqStartRowsRef.front()
+    // 0x16       15
+    //  --        17
+    if (!StmtAttrsRef.empty() && StmtAttrsRef.front().get() == NextSeqOff)
+      StmtAttrsRef.consume_front();
+    if (!SeqStartRowsRef.empty() && SeqStartRowsRef.front() == NextRow)
+      SeqStartRowsRef.consume_front();
+  }
+}
+
 std::pair<bool, std::optional<int64_t>>
 DWARFLinker::getVariableRelocAdjustment(AddressesMap &RelocMgr,
                                         const DWARFDie &DIE) {
@@ -2297,8 +2408,12 @@ void DWARFLinker::DIECloner::generateLineTableForUnit(CompileUnit &Unit) {
 
         // Create a map of stmt sequence offsets to original row indices.
         DenseMap<uint64_t, unsigned> SeqOffToOrigRow;
-        for (const DWARFDebugLine::Sequence &Seq : LT->Sequences)
-          SeqOffToOrigRow[Seq.StmtSeqOffset] = Seq.FirstRowIndex;
+        // The DWARF parser's discovery of sequences can be incomplete. To
+        // ensure all DW_AT_LLVM_stmt_sequence attributes can be patched, we
+        // build a map from both the parser's results and a manual
+        // reconstruction.
+        if (!LT->Rows.empty())
+          constructSeqOffsettoOrigRowMapping(Unit, *LT, SeqOffToOrigRow);
 
         // Create a map of original row indices to new row indices.
         DenseMap<size_t, size_t> OrigRowToNewRow;