ilasm produces wrong SequencePoints when a method references several Documents

[yjaradin]

Description

When a method contains .line directives referring to several documents, ilasm ignores all but the last specified document.
This in turn impacts tools (such as Visual Studio) that will, for example, show the wrong file (and therefore an irrelevant line) when stepping through the built code.

Reproduction Steps

After having built the tests, run the following program.

using System.Reflection;
using System.Reflection.Metadata;
using System.Reflection.PortableExecutable;

//Adapt the following two lines to suit your platform/configuration/folders.
var dotnetRuntimeRepo = "C:\\repositories\\dotnet\\runtime";
var pepath = $"{dotnetRuntimeRepo}\\artifacts\\tests\\coreclr\\windows.x64.Debug\\ilasm\\PortablePdb\\IlasmPortablePdbTests\\TestFiles\\TestDocuments1_win.dll";
//compiled from <dotnetRuntimeRepo>\src\tests\ilasm\PortablePdb\TestFiles\TestDocuments1_win.il
var pereader = new PEReader(File.OpenRead(pepath));
pereader.TryOpenAssociatedPortablePdb(pepath, s => File.OpenRead(s), out var pdbreader, out var pdbpath);
if (pdbreader == null) return;
var reader = pdbreader.GetMetadataReader();
foreach (var mdi in reader.MethodDebugInformation)
{
    if (mdi.IsNil) continue;
    foreach (var sp in reader.GetMethodDebugInformation(mdi).GetSequencePoints())
    {
        var d = sp.Document;
        if (d.IsNil) continue;
        Console.WriteLine(reader.GetString(reader.GetDocument(d).Name));
    }
}

For reference, here is TestsDocuments1_win.il:

// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

.assembly extern System.Runtime
{
}

.assembly TestDocuments
{
}

.class public auto ansi beforefieldinit TestDocumentsType
       extends [System.Runtime]System.Object
{
    .method public hidebysig instance void Foo() cil managed
    {
    .line 1,1 : 9,10 'C:\\tmp\\non_existent_source1.cs'
        nop
    .line 2,2 : 9,10 'C:\\tmp\\non_existent_source2.cs'
        ret
    }
}

Expected behavior

The following output is expected (on Windows, on other platforms the paths will be different but the important digit would be the same):

C:\tmp\non_existent_source1.cs
C:\tmp\non_existent_source2.cs

Actual behavior

The following output is produced (again, on Windows with slight changes to the paths but not the digits on other platforms):

C:\tmp\non_existent_source2.cs
C:\tmp\non_existent_source2.cs

Regression?

This is a functional regression compared to .NET Framework although it used a different (non-portable) PDB file format.

Known Workarounds

Using .NET Framework where practical.

Configuration

Observed on:

.NET 6 -> .NET 9 (preview 3)
Windows 10 x64, Windows 11 x64

Not specific to that configuration

Other information

The issue is in the method

runtime/src/coreclr/ilasm/portable_pdb.cpp

Line 201 in 039d2ec

HRESULT PortablePdbWriter::DefineSequencePoints(Method* method)

which doesn't deal with an initial document (L208) and only provides a unique document (L314, notice that it uses currSeqPoint outside the loop).

I have a fix and a test for this issue that will be in an upcoming PR.

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.