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

Problem: Linking duktape on NixOS with musl #1959

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yrashk
Copy link

@yrashk yrashk commented Aug 14, 2018

This is a little bit of a subtle issue. The way NixOS/Nixpkgs work, they typically
wrap binaries (such as the compiler) with more complicated wrappers to handle
the paths established in Nixpkgs store and other features. In this case, the feature
that's interfering with normal flow is hardening. Without going into too much detail,
there's no way to disable _FORTIFY_SOURCE as it is always passed after any passed
arguments. Something like gcc <PARAMETERS> -D_FORTIFY_SOURCE.

However, this doesn't play well with musl (see https://wiki.musl-libc.org/future-ideas.html)

Solution: disable _FORTIFY_SOURCE if glibc is not present


I've been using this patch for quite a while in SIT https://github.com/sit-fyi/sit/blob/master/sit-core/src/duktape/duktape.h#L144-L146 but I keep transferring it during upgrades and this isn't particularly robust.

I am not sure this is the best way to solve but at least it have worked for a good while.

Any thoughts?

This is a little bit of a subtle issue. The way NixOS/Nixpkgs work, they typically
wrap binaries (such as the compiler) with more complicated wrappers to handle
the paths established in Nixpkgs store and other features. In this case, the feature
that's interfering with normal flow is hardening. Without going into too much detail,
there's no way to disable _FORTIFY_SOURCE as it is always passed after any passed
arguments. Something like `gcc <PARAMETERS> -D_FORTIFY_SOURCE`.

However, this doesn't play well with musl (see https://wiki.musl-libc.org/future-ideas.html)

Solution: disable `_FORTIFY_SOURCE` if glibc is not present
@svaarala
Copy link
Owner

First, tweaks like these would ideally go into duk_config.h which is intended to be the header where all the platform stuff resides. I think in your case you should be able to at least inject the #undef _FORTIFY_SOURCE at the top of duk_config.h manually. This would keep duktape.c and duktape.h clean and avoid the need for platform/compiler stuff outside duk_config.h.

Depending on how _FORTIFY_SOURCE affects included headers, tools/configure.py allows you to insert "fixup" headers from the command line directly, or from external files. For example:

$ cat fortify.h 
#ifndef __GLIBC__
#undef _FORTIFY_SOURCE
#endif

$ python tools/configure.py --output-directory /tmp/out --fixup-file fortify.h
[...]

$ less /tmp/out/duk_config.h
[...]
/*
 *  Fixups
 */

#ifndef __GLIBC__
#undef _FORTIFY_SOURCE
#endif
[...]

There's currently no option to insert fixups at the top of the duk_config.h though. This would be a useful addition for things that affect header includes. Anyway, maybe you could try the above?

But assuming the above doesn't work:

  • Can (or should) this change be musl specific (i.e. detect musl rather than detecting "not glibc")?
  • Could you summarize what breaks with _FORTIFY_SOURCE? I can also also check it out when I get a chance. If possible, it'd be nice to fix that instead.

I'm a bit hesitant in silently disabling _FORTIFY_SOURCE because it may be also given intentionally. It's of course not ideal if compilation fails with the option, but it may be better than silently disabling it.

@svaarala
Copy link
Owner

Also one more question:

  • Should _FORTIFY_SOURCE be disabled when compiling Duktape itself (i.e. duktape.c) or also when compiling any application file including duktape.h?

If this should only affect Duktape, the #ifndef should also check for DUK_COMPILING_DUKTAPE.

@yrashk
Copy link
Author

yrashk commented Aug 15, 2018 via email

@svaarala
Copy link
Owner

@yrashk Regarding duk_config.h, you could try the following fixup file (fortify.h):

#if defined(DUK_COMPILING_DUKTAPE) && !defined(__GLIBC__)
#undef _FORTIFY_SOURCE
#endif

This limits the workaround only to the compiling of duktape.c and not other application files including duktape.h.

Then run:

$ python tools/configure.py --output-directory /tmp/out [... whatever else ...] \
      --fixup-file fortify.h

... and then use the duktape.c, duktape.h, and duk_config.h from the output directory.

If this works, you should be able to activate the workaround with no required Duktape patches.

Regardless of this, we can try to figure out the root cause and see if that's fixable or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants