Skip to content

Fixed memory copy error of virtual table under multiple inheritance#1695

Open
hxbb00 wants to merge 7 commits into
mono:mainfrom
hxbb00:main
Open

Fixed memory copy error of virtual table under multiple inheritance#1695
hxbb00 wants to merge 7 commits into
mono:mainfrom
hxbb00:main

Conversation

@hxbb00

@hxbb00 hxbb00 commented Nov 18, 2022

Copy link
Copy Markdown
Contributor

No description provided.

@josetr

josetr commented Nov 18, 2022

Copy link
Copy Markdown
Contributor

Can you add a test please?

@hxbb00

hxbb00 commented Nov 18, 2022

Copy link
Copy Markdown
Contributor Author

I have passed the test in the project of my company. Sorry, the project environment of my company is too big, so it is not suitable to provide the test environment. My company used this in the project, I use it in practice, so it shouldn't be a problem!

@hxbb00

hxbb00 commented Nov 18, 2022

Copy link
Copy Markdown
Contributor Author

image
this is mycase,here the __Internal:
image

@hxbb00

hxbb00 commented Nov 18, 2022

Copy link
Copy Markdown
Contributor Author

image
The difference with this modification is that there is no problem when the offset is 0, but when the offset is not 0, the address is calculated incorrectly

@josetr

josetr commented Nov 18, 2022

Copy link
Copy Markdown
Contributor

My company used this in the project, I use it in practice, so it shouldn't be a problem!

This only tells us that it works for your specific use case and your specific platform / ABI. We have to support multiple ABI's. You may be fixing the issue in one platform and then breaking the rest.

You have made a change to this line before without a test and you are doing it again. Your previous PR shouldn't have been merged.

@tritao

tritao commented Nov 18, 2022

Copy link
Copy Markdown
Collaborator

@hxbb00 We really appreciate your contribution but as @josetr said it would be great if you could add a test.

Can you try to recreate this class hierarchy in https://github.com/mono/CppSharp/blob/main/tests/VTables/VTables.h for example?

@hxbb00 hxbb00 closed this Nov 23, 2022
@tritao tritao reopened this Nov 23, 2022
@tritao

tritao commented Nov 23, 2022

Copy link
Copy Markdown
Collaborator

Leaving this open for now so we don't forget about it

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 this pull request may close these issues.

3 participants