Skip to content

Fix GetGenerationWithRange when we have a gen 2 object in the ephemeral segment (#60189) - #60207

Merged
Anipik merged 1 commit into
dotnet:release/6.0from
cshung:public/backport-fix-get-generation-with-range
Oct 11, 2021
Merged

Fix GetGenerationWithRange when we have a gen 2 object in the ephemeral segment (#60189)#60207
Anipik merged 1 commit into
dotnet:release/6.0from
cshung:public/backport-fix-get-generation-with-range

Conversation

@cshung

@cshungcshung commented Oct 9, 2021

Copy link
Copy Markdown
Contributor

Backporting #60189

Customer Impact

Since the GetObjectGeneration() API returned -1 instead of 2 for a Gen 2 object lying on the ephemeral heap segment. A less defensive ICorProfiler implementation might index into the -1 entry of an array and leading to buffer underflow. Even for a defensive implementation, we would have wrong statistics.

After the change, we will output 2 as the correct generation number.

Testing

The diagnostics team confirmed the change fixed the problem through their automated testing.

Risk

Low – this is a new code path introduced in .NET 6.

@ghostghost added the area-GC-coreclr label Oct 9, 2021
@ghost

ghost commented Oct 9, 2021

Copy link
Copy Markdown

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

Issue Details

WIP

Author:cshung
Assignees:-
Labels:

area-GC-coreclr

Milestone:-

@cshung
cshung requested a review from Maoni0October 9, 2021 16:49
@Maoni0

Copy link
Copy Markdown
Member

just a comment on your description of the bug -

Since the GetObjectGeneration() API returned -1 instead of 2 for a Gen 2 object lying on the ephemeral heap segment. A careless ICorProfiler implementation might index into the -1 entry

I'm confused why you would categorize this ICorProfiler implementation as "careless", is -1 considered a valid value that means gen2 for profilers?

@cshung

cshung commented Oct 9, 2021

Copy link
Copy Markdown
ContributorAuthor

I'm confused why you would categorize this ICorProfiler implementation as "careless", is -1 considered a valid value that means gen2 for profilers?

No. -1 is not considered as a valid value to mean gen2. Perhaps "less defensive" is a better description than "careless" in this case. One could imagine a less defensive implementation look like this.

int gen = GetObjectGeneration(...);
counter[gen]++;

or a more defensive one look like this:

int gen = GetObjectGeneration(...);
if (0 <= gen && gen <= 4)
{
counter[gen]++;
}
else
{
assert(false);
}

In the former case, we will have a buffer underflow issue, in the latter case, the statistics are wrong. Either case is not great, but the former one is worse.

@Anipik

Copy link
Copy Markdown
Contributor

approved in email

@Anipik
Anipik merged commit b1aedd5 into dotnet:release/6.0Oct 11, 2021
@AnipikAnipik added the Servicing-approved Approved for servicing release label Oct 11, 2021
@cshung
cshung deleted the public/backport-fix-get-generation-with-range branch October 11, 2021 16:05
@ghostghost locked as resolved and limited conversation to collaborators Nov 10, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-GC-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@cshung@Maoni0@Anipik