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

Avoid trampolines in emmalloc #20496

Merged
merged 2 commits into from
Oct 19, 2023
Merged

Avoid trampolines in emmalloc #20496

merged 2 commits into from
Oct 19, 2023

Conversation

kripken
Copy link
Member

@kripken kripken commented Oct 19, 2023

Rather than define malloc as a small function that calls emmalloc_malloc, just make it
an alias.

In an optimized build the small functions get inlined away, but we can save that work,
and also it can have downsides (e.g. they have different function pointers, which
aliases do not, see discussion in #20487).

For that to work, we need to make the aliases weak. I think that might have been the
reason to not do this beforehand, but it seems to work ok.

@kripken kripken requested a review from sbc100 October 19, 2023 20:41
#define EMMALLOC_ALIAS(ALIAS, ORIGINAL)
#else
#define EMMALLOC_EXPORT __attribute__((weak, __visibility__("default")))
#define EMMALLOC_ALIAS(ALIAS, ORIGINAL) extern __typeof(ORIGINAL) ALIAS __attribute__((alias(#ORIGINAL)));
#define EMMALLOC_ALIAS(ALIAS, ORIGINAL) extern __typeof(ORIGINAL) ALIAS __attribute__((weak, alias(#ORIGINAL)));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to make these weak?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is to match the previous behavior.

If we don't do this then e.g. malloc would not be weak, so the user couldn't override it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure that it true though.. the existing symbols are strong in that if you link the object file you are forced to take that definition. Do you have a test that fails without weak?

Overriding malloc as far as I know only works today if you don't actually use any symbols from emmalloc.o (or dlmalloc.o).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh sorry. I see the existing EMMALLOC_EXPORT is weak... I'm not sure it should be but lgtm given that

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also dlmalloc.c uses weak aliases for all of these..

@kripken kripken merged commit 054bf30 into main Oct 19, 2023
3 checks passed
@kripken kripken deleted the emmalloc.trampoline.no branch October 19, 2023 22:01
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.

2 participants