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

musl: fix weak_alias in projects expecting glibc behavior #14352

Closed
wants to merge 1 commit into from

Conversation

msabwat
Copy link
Contributor

@msabwat msabwat commented Jun 2, 2021

This patch fixes #14350, by adding a semicolon. The macro can be
used in both cases :

  • weak_alias(fun, alias)
  • weak_alias(fun, alias);

This is possible because calling (for eg.) :

extern __typeof(__memchr) memchr __attribute__((__weak__, __alias__("__memchr")));;

with two semicolons is valid, it won't break the current weak_alias(); calls in musl with the semicolon

This patch fixes emscripten-core#14350, by adding a semicolon. The macro can be
used in both cases :
- weak_alias(fun, alias)
- weak_alias(fun, alias);
@sbc100
Copy link
Collaborator

sbc100 commented Jun 3, 2021

libc.h is an internal header that is only used when compiling musl.

It should be impossible to ever include this file except when compiling musl.. how are you including it?

@sbc100
Copy link
Collaborator

sbc100 commented Jun 3, 2021

If could be that in some older version of emscripten is was possible to include this file... can you try with the latest version?

@msabwat
Copy link
Contributor Author

msabwat commented Jun 4, 2021

If could be that in some older version of emscripten is was possible to include this file... can you try with the latest version?

I did, tested with 2.0.23

It should be impossible to ever include this file except when compiling musl.. how are you including it?

my bad, I think I patched the wrong file

#define weak_alias(old, new) \

@sbc100
Copy link
Collaborator

sbc100 commented Jun 4, 2021

Ah yes, it looks like this issue was already fixed upstream in musl but splitting this into a public features.h and an internal one (the public one no longer contains this macro.

I've been working on a musl upgrade for a while not which would address this.

Where is this code being used downstream? Presumably downstream contains its own defintion of weak_alias guards with and #ifndef. Could you instead patch that code to do #undef weak_alias to avoid getting the musl version. This macro is not standard and musl should not really be defining it at all IIUC.

@sbc100
Copy link
Collaborator

sbc100 commented Jun 4, 2021

Actually can you just try just removing that macro from features.h... it looks like it was added locally at some point and does not exist upstream at all.

@sbc100
Copy link
Collaborator

sbc100 commented Jun 4, 2021

It looks like it was added in #11215

sbc100 added a commit that referenced this pull request Jun 4, 2021
This was added in #11215 to maintain comatability with upgraded version
of stpcpy.c/stpncpy.c/strchrnul.c.

However, this macro was never indended to exposed outside of musl
itself.  In musl v1.2.2 this macro lives in the *internal* version of
features.h but musl v1.1.15 does't have an internal features.h.  Instead
this macro lives in libc.h in v1.1.15.

See: #14352
Fixes: #14350
@sbc100
Copy link
Collaborator

sbc100 commented Jun 4, 2021

I created a PR to remove it completely: #14379

@msabwat
Copy link
Contributor Author

msabwat commented Jun 4, 2021

Ah yes, it looks like this issue was already fixed upstream in musl but splitting this into a public features.h and an internal one (the public one no longer contains this macro.

I've been working on a musl upgrade for a while not which would address this.

Where is this code being used downstream? Presumably downstream contains its own defintion of weak_alias guards with and #ifndef. Could you instead patch that code to do #undef weak_alias to avoid getting the musl version. This macro is not standard and musl should not really be defining it at all IIUC.

I don't think it's possible to undef it, I tried and it gets reincluded if, for example you include string.h (which will include features.h). But it does correctly handle the case where weak_alias is not defined at all. Thanks !

@msabwat
Copy link
Contributor Author

msabwat commented Jun 4, 2021

I created a PR to remove it completely: #14379

Thank you very much Sam!

@msabwat msabwat closed this Jun 4, 2021
sbc100 added a commit that referenced this pull request Jun 4, 2021
This was added in #11215 to maintain comatability with upgraded version
of stpcpy.c/stpncpy.c/strchrnul.c.

However, this macro was never indended to exposed outside of musl
itself.  In musl v1.2.2 this macro lives in the *internal* version of
features.h but musl v1.1.15 does't have an internal features.h.  Instead
this macro lives in libc.h in v1.1.15.

See: #14352
Fixes: #14350
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.

[gnutls]: does not compile if it uses musl's weak_alias()
2 participants