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.

Sign up for GitHub

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

Jump to bottom

Reenable class-memaccess warning #74363

Merged
merged 13 commits into from
Aug 25, 2022
Merged

Reenable class-memaccess warning #74363

merged 13 commits into from
Aug 25, 2022

Conversation

AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 8.0.0 milestone Aug 22, 2022
RobertHenry6bev
RobertHenry6bev reviewed Aug 22, 2022
View reviewed changes
Copy link
Contributor

@RobertHenry6bev RobertHenry6bev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

This was referenced Aug 23, 2022
Closed
Closed
Closed
@AaronRobinsonMSFT AaronRobinsonMSFT marked this pull request as ready for review August 23, 2022 21:22
@am11
Copy link
Member

am11 commented Aug 24, 2022

Looks good. Thanks!

@karelz karelz mentioned this pull request Aug 24, 2022
Closed
@AaronRobinsonMSFT @jkotas
Apply suggestions from code review
55f41a2 
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 5384105 into dotnet:main Aug 25, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the runtime_12684 branch August 25, 2022 20:23
@uweigand
Copy link
Contributor

uweigand commented Sep 9, 2022

After this patch, I now see assertion failures in ilasm:

Assert failure(PID 535521 [0x00082be1], Thread: 535521 [0x82be1]): TypeFromToken(tkTypeDef) == mdtTypeDef
    File: /home/uweigand/runtime/src/coreclr/md/compiler/assemblymd_emit.cpp Line: 768
    Image: /home/uweigand/runtime/artifacts/bin/coreclr/Linux.s390x.Debug/ilasm

It seems there are now AsmManComType structures with random (uninitialized) values in tkClass. These structures used to be memset to zero in AsmMan::StartComType, but that was removed by this patch. The constructor for AsmManComType does initialize some of the members, but not tkClass.

@AaronRobinsonMSFT AaronRobinsonMSFT mentioned this pull request Sep 9, 2022
Merged
@cshung cshung mentioned this pull request Sep 19, 2022
Merged
This was referenced Sep 20, 2022
Closed
Merged
@ghost ghost locked as resolved and limited conversation to collaborators Oct 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@RobertHenry6bev RobertHenry6bev RobertHenry6bev left review comments

@jkotas jkotas jkotas approved these changes

@MichalStrehovsky MichalStrehovsky Awaiting requested review from MichalStrehovsky MichalStrehovsky is a code owner

@elinor-fung elinor-fung Awaiting requested review from elinor-fung

@VSadov VSadov Awaiting requested review from VSadov

Assignees

@AaronRobinsonMSFT AaronRobinsonMSFT

Labels
area-VM-coreclr
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.

Usage of memset with non-trivial types that results in -Wclass-memaccess
5 participants
@AaronRobinsonMSFT @am11 @uweigand @RobertHenry6bev @jkotas