Skip to content
This repository has been archived by the owner on Nov 15, 2021. It is now read-only.

Incorrectly identifying branch as excluded branch #722

Closed
6 of 24 tasks
sawilde opened this issue May 28, 2017 · 7 comments
Closed
6 of 24 tasks

Incorrectly identifying branch as excluded branch #722

sawilde opened this issue May 28, 2017 · 7 comments

Comments

@sawilde
Copy link
Member

sawilde commented May 28, 2017

Please provide the following information when submitting an issue.

Where appropriate replace the [ ] with a [X]

My Framework

  • .NET 2
  • .NET 3.5
  • .NET 4
  • .NET 4.5
  • .NET 4.6
  • .NET 4.6.1
  • .NET 4.6.2
  • .NET Core 1.0.0
  • Something else

My Environment

  • Windows 7 or below (not truly supported due to EOL)
  • Windows 8
  • Windows 8.1
  • Windows 10
  • Windows 10 IoT Core
  • Windows Server 2012
  • Windows Server 2012 R2
  • Windows Server 2016

I have already...

  • repeated the problem using the latest stable release of OpenCover.
  • reviewed the usage guide and usage document.
  • have looked at the opencover output xml file in an attempt to resolve the issue.
  • reviewed the current issues to check that the issue isn't already known.

My issue is related to (check only those which apply):

  • no coverage being recorded
  • 32 or 64 bit support
  • feature request

Expected Behavior

Should not incorrect identify branches as excluded

Actual Behavior

Incorrectly identifying code as excluded

Steps to reproduce the problem:

Expected Generated IL was either wrong or has since changed - new expected sequence is

image

@ddur
Copy link
Contributor

ddur commented May 29, 2017

Is that instrumentation code starting with ldsfld class OpenCove..?

By reading issue title, I'm not sure what is going on here.
Do you want to include excluded branch or want to exclude included branch?

As far as I can remember, I did not exclude branches based on IL code matching.
In debug mode, every sequence of available source code is mapped to IL sequence.
If branch is within source/IL code sequence that should not contain branches (like { or }), branches are ignored. Not sure now, I think that debug sequence info does not contain end-offset, so end-offset is calculated as next debug sequence start-offset - 1. That can possibly expand source/IL sequence (like {) over code injected between two sequences.

@sawilde
Copy link
Member Author

sawilde commented May 29, 2017

The idea with this area is to identify a branch and if it follows a certain sequence to ignore the branch

2 things seem to have happened since the original code was written

  1. the code didn't work correctly and incorrectly matched, and therefore excluded, branches that should not be ignored
  2. the IL sequence has changed

I may look to expand it to also look at the names of the fields etc but this first pass fixes the broken implementation.

@ddur
Copy link
Contributor

ddur commented May 30, 2017

Well, I'm afraid that IL code can possibly change with every new compiler release.
If you cover only last compiler release IL-patterns, testing with older compiler may fail.
Same IL-sequence can possibly appear anywhere in the code. Do you check at specific offset only?

Can you identify source code sequence-point covering that IL-sequence?

I think, it would be a improvement to get Cecil to return end-offset of each debug sequence, if possible. That would maybe help to identify compiler injected code between debug sequences

@sawilde
Copy link
Member Author

sawilde commented May 30, 2017

Well, I'm afraid that IL code can possibly change with every new compiler release.

yup, I think I did raise that issue at the time.

Same IL-sequence can possibly appear anywhere in the code.

yup, but if the branch has no identified sequence point in the range, then we have no attributable source code and can assume it is auto-generated.

I think, it would be a improvement to get Cecil to return end-offset of each debug sequence

I am not sure if cecil would know that, it gets its data from the PDB.

@ddur
Copy link
Contributor

ddur commented May 30, 2017

Unfortunately, because there is no end-offset, all sequence points between first { and last } cover all IL code. Only if auto-generated code (branch) is injected inside or before first { or after last } method offsets, it can be safely recognized as auto-generated. Most of auto-generated code (code-contracts) is injected outside of that range, but not all.

I know that Cecil gets data from PDB, but do not know if PDB contains end-offset at all.

@sawilde
Copy link
Member Author

sawilde commented May 31, 2017

perhaps the end offset of a sequence point could be the next/closest sequence point?

@ddur
Copy link
Contributor

ddur commented May 31, 2017

That is exactly how it is done. End-offset is next sequence (start-offset - 1).
But that does not leave space to detect auto-generated code between sequence-points.
Sometimes last } sequence point is missing.

@OpenCover OpenCover deleted a comment from LAMBFAMILY Aug 5, 2017
sawilde added a commit that referenced this issue Aug 19, 2017
#722 identify excluded branch based on IL
@sawilde sawilde closed this as completed Jan 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants