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.

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

Fix how empty function pointer cast tables are handled #6308

Merged

Conversation

mdboom
Copy link
Contributor

@mdboom mdboom commented Mar 7, 2018

This fixes what I think is a fairly straightforward bug when EMULATE_FUNCTION_POINTER_CASTS is turned on.

When compiling Python as here, some of the tables end up having 0 length. In that case, they look like [] and when fixup_metadata_tables tries to make all the tables the same length, it sets these empty ones to [,0,0] rather than [0,0] as it should. This triggers the assertion:

          assert i not in function_pointer_targets

later on in make_function_tables_defs, since many of the tables have the same empty string in the first entry in the table.

This patch seems to get me to a working state, but I don't know if having empty tables in the first place is the sign of some deeper issue.

@kripken
Copy link
Member

kripken commented Mar 7, 2018

Looks good, thanks! And the CI failure looks like a random bot problem we can ignore.

Please just add yourself to AUTHORS before we merge.

Empty tables is odd, but not necessarily a bug. It can be caused by doing a function pointer call but not having a function implemented of that type - perhaps such a function is expected to arrive from a dynamically linked module.

@mdboom
Copy link
Contributor Author

mdboom commented Mar 8, 2018

Thanks. I've added my name to AUTHORS.

@kripken kripken merged commit deabf16 into emscripten-core:incoming Mar 8, 2018
@kripken
Copy link
Member

kripken commented Mar 8, 2018

Thanks again!

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.

None yet

2 participants