Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem adding sequence points to a method with portable PDB #840

Open
Zastai opened this issue Feb 22, 2022 · 2 comments · May be fixed by #853
Open

Problem adding sequence points to a method with portable PDB #840

Zastai opened this issue Feb 22, 2022 · 2 comments · May be fixed by #853

Comments

@Zastai
Copy link
Contributor

Zastai commented Feb 22, 2022

Whenever I attempt to add a sequence point to a method with a portable PDB involved, the resulting PDB has a broken SequencePoints blob, resulting in

System.BadImageFormatException: Invalid compressed integer.
   at System.Reflection.Throw.InvalidCompressedInteger()
   at System.Reflection.Metadata.SequencePointCollection.Enumerator.MoveNext()

during processing, like when when hovering over the "SequencePoints" value when looking at the MethodDebugInformation table in ILSpy, or when running pdb2xml.

Looking at a before/after, for a case where I added only a single "hidden" (lines set to 0xfeefee) sequence point, the original blob contained 376 bytes:

01-00-00-19-80-D8-07-05-00-1D-10-00-05-02-24-06
00-51-00-5D-18-00-25-00-4B-02-00-20-00-35-04-00
1F-00-3A-0A-00-16-00-00-81-2F-00-36-04-08-1F-00
34-02-00-1A-00-24-02-00-11-00-34-02-00-1A-00-24
02-00-11-00-06-02-00-01-00-36-04-00-1F-00-34-02
00-1A-00-24-02-00-11-00-34-02-00-1A-00-24-02-00
11-00-06-02-00-01-00-36-04-00-1F-00-34-02-00-1A
00-24-02-00-11-00-34-02-00-1A-00-24-02-00-11-00
06-02-00-01-00-36-04-00-1F-00-34-02-00-1A-00-24
02-00-11-00-34-02-00-1A-00-24-02-00-11-00-06-02
00-01-00-36-04-00-1F-00-34-02-00-1A-00-24-02-00
11-00-34-02-00-1A-00-24-02-00-11-00-34-02-00-1A
00-24-02-00-11-00-34-02-00-1A-00-24-02-00-11-00
34-02-00-1A-00-24-02-00-11-00-06-02-00-01-00-36
04-00-1F-00-34-02-00-1A-00-24-02-00-11-00-34-02
00-1A-00-3E-02-00-1F-00-24-02-00-11-00-06-02-00
01-00-36-04-00-1F-00-34-02-00-1A-00-24-02-00-11
00-34-02-00-1A-00-24-02-00-11-00-06-02-00-01-00
36-04-00-1F-00-34-02-00-1A-00-24-02-00-11-00-34
02-00-1A-00-24-02-00-11-00-06-02-00-01-00-36-04
00-1F-00-34-02-00-1A-00-24-02-00-11-00-34-02-00
1A-00-24-02-00-11-00-06-02-00-01-00-36-04-00-1F
00-34-02-00-1A-00-24-02-00-11-00-34-02-00-1A-00
24-02-00-11-00-01-0E-75

and the new, broken one contains 386 bytes:

01-00-00-19-80-D8-07-22-00-1D-10-00-06-02-24-06
00-51-00-5D-18-00-25-00-4B-02-00-20-00-35-04-00
1F-00-3A-0A-00-16-00-00-81-6A-00-36-04-08-1F-00
34-02-00-1A-00-24-02-00-11-00-34-02-00-1A-00-24
02-00-11-00-06-02-00-05-00-36-04-00-1F-00-34-02
00-1A-00-24-02-00-11-00-34-02-00-1A-00-24-02-00
11-00-06-02-00-05-00-36-04-00-1F-00-34-02-00-1A
00-24-02-00-11-00-34-02-00-1A-00-24-02-00-11-00
06-02-00-05-00-36-04-00-1F-00-34-02-00-1A-00-24
02-00-11-00-34-02-00-1A-00-24-02-00-11-00-06-02
00-05-00-36-04-00-1F-00-34-02-00-1A-00-24-02-00
11-00-34-02-00-1A-00-24-02-00-11-00-34-02-00-1A
00-24-02-00-11-00-34-02-00-1A-00-24-02-00-11-00
34-02-00-1A-00-24-02-00-11-00-06-02-00-05-00-36
04-00-1F-00-34-02-00-1A-00-24-02-00-11-00-34-02
00-1A-00-3E-02-00-1F-00-24-02-00-11-00-06-02-00
05-00-36-04-00-1F-00-34-02-00-1A-00-24-02-00-11
00-34-02-00-1A-00-24-02-00-11-00-06-02-00-05-00
36-04-00-1F-00-34-02-00-1A-00-24-02-00-11-00-34
02-00-1A-00-24-02-00-11-00-06-02-00-05-00-36-04
00-1F-00-34-02-00-1A-00-24-02-00-11-00-34-02-00
1A-00-24-02-00-11-00-06-02-00-05-00-36-04-00-1F
00-34-02-00-1A-00-24-02-00-11-00-34-02-00-1A-00
24-02-00-11-00-01-0E-75-FF-FF-F8-49-00-00-87-B9
00-00

My gaze is drawn by that FFFFF8 sequence near the end. The method in question includes a "hidden" sequence point (string hashing code for a string-based switch), so it can't be the 0xfeefee causing an issue there. My suspicion is that either the PDB writing code does not handle negative values correctly for a "compressed integer", or other processing results in a negative value when it shouldn't.

Looking at the code that reads signed values back, the sign bit gets moved to the end. So if that 0xfffff849 is intended to be signed value -1975, I would expect to see something like 0x4f6f instead, if I read the code correctly (bit 0x40 in first byte to indicate: 2 bytes to read in total, then 0xf6f which is 1975 shifted left + the sign bit).

I'll try to make a minimal repro case, but hopefully this may already point in the right direction.

@Zastai Zastai changed the title Problem adding sequence points to a method Problem adding sequence points to a method with portable PDB Feb 22, 2022
@Zastai
Copy link
Contributor Author

Zastai commented Jun 8, 2022

Note that while this issue prevents debugging in Visual Studio (because the entire PDB reading aborts), Rider is less affected (it seems to just silently discard the "bad" sequence points).

@Zastai
Copy link
Contributor Author

Zastai commented Jun 10, 2022

Looks like the problem is not so much the hidden sequence points - the sequence point records are emitted correctly.

However, the problem is that SignatureWriter.WriteSequencePoints assumes that MethodDebugInformation.SequencePoints is sorted by offset. However, that does not seem to be guaranteed. I'm not even certain that it's guaranteed that it would have duplicate offsets in it.

I'll prepare a PR.

Zastai added a commit to Zastai/cecil that referenced this issue Jun 10, 2022
This is required by the Portable PDB specification, and the sequence
point collection in `MethodDebugInfo` does not guarantee it.

This also tweaks the writing of hidden sequence points to use two
writes of 0 as compressed unsigned integer, instead of a single
write of 0 as a plain short, to more closely match the specification.

Fixes jbevain#840.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant