Skip to content

Normalize Nil PublicKeyToken to an empty array#69169

Merged
VSadov merged 6 commits intodotnet:mainfrom
VSadov:asmNameFix
May 13, 2022
Merged

Normalize Nil PublicKeyToken to an empty array#69169
VSadov merged 6 commits intodotnet:mainfrom
VSadov:asmNameFix

Conversation

@VSadov
Copy link
Copy Markdown
Member

@VSadov VSadov commented May 11, 2022 •
edited
Loading

This fixes #66785
(I verified manually, but I also had to re-baseline couple existing tests)

Also fixes #69153

@ghost ghost assigned VSadov May 11, 2022
@ghost
Copy link
Copy Markdown

ghost commented May 11, 2022

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

Issue Details

This fixes #66785
(I verified manually)

I wonder if we have existing tests for this. Let's see is anything fails.

Author: VSadov
Assignees: -
Labels:

area-System.Reflection.Metadata
Milestone: -

@VSadov VSadov mentioned this pull request May 12, 2022
Closed
@jkotas
Copy link
Copy Markdown
Member

jkotas commented May 12, 2022

Is the current change fully addressing the problem from #69153 ?

@VSadov
Copy link
Copy Markdown
Member Author

VSadov commented May 12, 2022 •
edited
Loading

Is the current change fully addressing the problem from #69153 ?

I think so. The repro assembly has public key but does not have any flags, so we ended up using the full key value as a token.
Thus we would report 160 byte array for the token and null for the key.

Forcing the PublicKey flag to be set unconditionally in definitions, once we see that there is a key, like in the native implementation, fixes the problem.
After the change the repro assembly reports 8 byte token and 160 byte key, as in 6.0

@VSadov
Copy link
Copy Markdown
Member Author

VSadov commented May 13, 2022

Thanks!!

@VSadov VSadov merged commit 10883a7 into dotnet:main May 13, 2022
@VSadov VSadov deleted the asmNameFix branch May 13, 2022 02:40
This was referenced May 23, 2022
Closed
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Jun 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jkotas jkotas jkotas approved these changes

Assignees

@VSadov VSadov

Labels

area-System.Reflection.Metadata

Projects

None yet

Milestone

No milestone

3 participants

@VSadov @jkotas @joperezr