>=media-libs/libpng-1.5* fails to compile for me on OS X Mountain Lion with the following message: libtool: compile: x86_64-apple-darwin12-gcc -DHAVE_CONFIG_H -I. -Wa,--noexecstack -march=nocona -O2 -pipe -c arm/filter_neon.S -fno-common -DPIC -o .libs/filter_neon.o FATAL:/Library/Gentoo/usr/lib/binutils/x86_64-apple-darwin12/4.3/x86_64/as: I don't understand '-' flag! The problem is with the -Wa,--noexecstack which is not understood by the assembler. I removed the flags from the Makefile and compiled the ebuild successfully. A diff of the changes is attached. Reproducible: Always
Created attachment 344758 [details, diff] Changes made to the Makefile
Prefix pulls this ebuild from gx86 directly, reassign to base-system. There is a check in configure missing to test if "-Wa,--noexecstack" actually works. Patching the Makefile is not really a great solution for autotools driven builds. A "sed -i '/noexecstack/s/^/#/' configure" would fix that or adding src_compile() { Adefault M_CCASFLAGS=; }
(In reply to comment #2) > src_compile() { Adefault M_CCASFLAGS=; } src_compile() { default AM_CCASFLAGS=; }
*** Bug 465186 has been marked as a duplicate of this bug. ***
Actually the solution from #465186: $ sed -i 's/-Wa,--noexecstack/-Wa,-noexecstack/' configure also works and it maybe even preferred to avoid having an executable stack.
Please use a proper title next time...
while I think the hack is neat, I'd rather just drop the entire argument for linkers that don't support it (test-flags?)
(In reply to comment #7) or don't use the flag at all. there's really no reason for it if you write your assembly code correctly. funny enough, i see the arm asm code (the only asm code in libpng) already does: .section .note.GNU-stack,"",%progbits /* mark stack as non-executable */
*** Bug 465320 has been marked as a duplicate of this bug. ***
(In reply to comment #8) > (In reply to comment #7) > > or don't use the flag at all. there's really no reason for it if you write > your assembly code correctly. The build-system adds it itself (wrongly). With this comment, I guess the sed to remove it altogether is the preferred way to go then.
(In reply to comment #3) > src_compile() { default AM_CCASFLAGS=; } inherit toolchain-funcs src_compile() { emake AM_CCASFLAGS="$(test-flags-CC -Wa,--noexecstack)" }
(In reply to comment #11) > (In reply to comment #3) > > src_compile() { default AM_CCASFLAGS=; } > > inherit toolchain-funcs > > src_compile() { > emake AM_CCASFLAGS="$(test-flags-CC -Wa,--noexecstack)" > } Apple's as just wants noexecstack with one minus.
FWIW no proof of that, it just ignores unknown options (and doesn't do long opts)
(In reply to comment #13) > FWIW no proof of that, it just ignores unknown options (and doesn't do long > opts) I see, so your solution seems to be the most elegant one.
Created attachment 345138 [details, diff] avoid -Wa,--execstack this will need to be sent upstream. the configure hunk obviously is only for our own ebuild usage.
Is this bug fixed in the portage tree yet? I tried to emerge libpng-1.6*, and it also has the same problem, so the fix needs to be applied there as well. I used the fix as in comment #11 in my own portage tree for now.
(In reply to comment #15) > Created attachment 345138 [details, diff] [details, diff] > avoid -Wa,--execstack > > this will need to be sent upstream. the configure hunk obviously is only > for our own ebuild usage. looked good, so I've pointed upstream the patch, this bug, http://www.gentoo.org/proj/en/hardened/gnu-stack.xml#doc_chap6 and minor explanation to go with it hence it's in upstream now: http://sourceforge.net/p/libpng/code/ci/453fbf942ca64b9fc0cbb6ce0391dc0683d3ed9e/
Eh... Right back at the beginning, why is an x86 (Nocona) target attempting to assemble GNU ARM specific code? The assembler file in question (filter_neon.S) is only included in Makefile.am if PNG_ARM_NEON is set, and that is only turned on, unless I've made some truly horrible error, if 'enable-arm-neon' is set on the configure command line. This is important because turning on the NEON optimizations on an x86 system results in a spurious call to the functions that check for NEON support; extra code, extra calls that should never be there on x86.
(In reply to comment #18) > Eh... > > Right back at the beginning, why is an x86 (Nocona) target attempting to > assemble GNU ARM specific code? > > The assembler file in question (filter_neon.S) is only included in > Makefile.am if PNG_ARM_NEON is set, and that is only turned on, unless I've > made some truly horrible error, if 'enable-arm-neon' is set on the configure > command line. > > This is important because turning on the NEON optimizations on an x86 system > results in a spurious call to the functions that check for NEON support; > extra code, extra calls that should never be there on x86. I'm seeing this on x86_64 Linux and ./configure --enable-arm-neon=off: libtool: compile: x86_64-pc-linux-gnu-gcc -DHAVE_CONFIG_H -I. -march=native -O2 -pipe -c arm/arm_init.c -fPIC -DPIC -o .libs/arm_init.o /bin/sh ./libtool --mode=compile x86_64-pc-linux-gnu-gcc -DHAVE_CONFIG_H -I. -Wa,--noexecstack -march=native -O2 -pipe -c -o filter_neon.lo `test -f 'arm/filter_neon.S' || echo './'`arm/filter_neon.S I doubt it's anything to worry about, it's just processes the file as no-op Don't worry, we have USE="neon" masked for everywhere else than ARM arch, so everyone else gets --enable-arm-neon=off
To be clear; I believe the patch is good. The configure.ac assembler flags should never have been introduced. However the reported bug seems to me to come from a different problem. John Bowler <jbowler@acm.org>
Created attachment 346348 [details] arm_init.o, filter_neon.o, libpng16.so.16 .tar.xz file files arm_init.o, filter_neon.o, libpng16.so.16 in .tar.xz after x86_64 Linux compilation with --enable-arm-neon=off If you want to look
(In reply to comment #19) > I'm seeing this on x86_64 Linux and ./configure --enable-arm-neon=off: Ah... Well, it's not meant to be turned off unless it can be on ;-) 'off' is there for people who have the NEON enhancements and don't want it on. Is this *because* Gentoo hard-wires "-neon" in the libpng build? If so, surely it should result in either no options to configure or, maybe: ./configure --disable-arm-neon ? John Bowler <jbowler@acm.org>
(In reply to comment #22) > (In reply to comment #19) > > I'm seeing this on x86_64 Linux and ./configure --enable-arm-neon=off: > > Ah... Well, it's not meant to be turned off unless it can be on ;-) > > 'off' is there for people who have the NEON enhancements and don't want it > on. > > Is this *because* Gentoo hard-wires "-neon" in the libpng build? If so, > surely it should result in either no options to configure or, maybe: > > ./configure --disable-arm-neon --enable-arm-neon=$(usex neon) $(usex neon) is 'hardwired' to USE flag 'neon' will return either 'on', or 'off' and filesize of libpng16.so.16 is exactly same with --enable-arm-neon=off AND not passing anything, 216032 bytes
(In reply to comment #23) > and filesize of libpng16.so.16 is exactly same with --enable-arm-neon=off > AND not passing anything, 216032 bytes and `nm -D` output is identical in both cases too, so it looks like the files do get processed but as no-op like I guessed before it might have revealed the execstack bug (this bug) in odd circumstances but i doubt it matters
'off' sets PNG_NO_ARM_NEON, so it should have identical results to not specifying anything. However, clearly it doesn't, because I can show the issue in my arm-android and arm-linux builds where 'off' is still building filter_neon.o. So there is a libpng bug, but I think it's some error in the #defines. John Bowler <jbowler@acm.org>
(In reply to comment #23) > --enable-arm-neon=$(usex neon) > > $(usex neon) is 'hardwired' to USE flag 'neon' will return either 'on', or > 'off' Okey dokey... At this point Gentoo experts must pounce, because a configure 'enable' option is not 'on' or 'off', rather it is 'yes' or 'no': http://www.gnu.org/software/autoconf/manual/autoconf.html#Package-Options So we have a six-way: arm-neon yes - go fish (an error message will result - this is by design, I didn't want existing build scripts to accidentally do something.) arm-neon no - configure takes care of this. arm-neon on/off/check/api - turns on NEON code and does something appropriate. Maybe 'usex' is the wrong primitive? This part of the problem seems like a Gentoo ebuild issue to me. John Bowler <jbowler@acm.org>
> $(usex neon) is 'hardwired' to USE flag 'neon' will return either 'on', or > 'off' That's not what /usr/portage/eclass/eutils.eclass says if EAPI < 5; 'defaults to "yes"/defaults to "no"'. Clearly there is still a question about why libpng seems to distinguish between "no" and "off", but that's a separate bug (I've counted 3 so far, 2 in libpng, this one in Gentoo.) John Bowler <jbowler@acm.org>
(In reply to comment #27) > > $(usex neon) is 'hardwired' to USE flag 'neon' will return either 'on', or > > 'off' > > That's not what /usr/portage/eclass/eutils.eclass says if EAPI < 5; > 'defaults to "yes"/defaults to "no"'. > > Clearly there is still a question about why libpng seems to distinguish > between "no" and "off", but that's a separate bug (I've counted 3 so far, 2 > in libpng, this one in Gentoo.) > > John Bowler <jbowler@acm.org> only in libpng then, if user reads ./configure --help, knows he doesn't own a ARM machine, he passes the relavent option, ./configure --disable-arm-neon or --disable-arm-neon=off to the ./configure for the build to avoid the build system from guessing what he wants it's even more important for packaging, we don't just pass --enable-foobar if we want foobar, but we also pass --disable-foobar if we don't want foobar then upstream can change the default of eg. AC_ARG_ENABLE or AC_ARG_WITH in-between releases to whatever he wants and we always get the desired outcome in more details here, http://www.gentoo.org/proj/en/qa/automagic.xml
(In reply to comment #28) > (In reply to comment #27) > > > $(usex neon) is 'hardwired' to USE flag 'neon' will return either 'on', or > > > 'off' > > > > That's not what /usr/portage/eclass/eutils.eclass says if EAPI < 5; > > 'defaults to "yes"/defaults to "no"'. > > > > Clearly there is still a question about why libpng seems to distinguish > > between "no" and "off", but that's a separate bug (I've counted 3 so far, 2 > > in libpng, this one in Gentoo.) > > > > John Bowler <jbowler@acm.org> > > only in libpng then, if user reads ./configure --help, knows he doesn't own > a ARM machine, he passes the relavent option or better yet, like my Efika MX cortex-a8 box which is still using old toolchain (gcc-4.5 with no neon support and -mfpu=vfpv3), I know the toolchain doesn't support neon, so I make informed decision of passing ./configure --enable-arm-neon=off to avoid it being built (for whatever indeficency in the upstream builg system). until I upgrade my toolchain to gcc-4.6/4.7 with -mfpu=neon in anycase the upstream build system should respect the --disable, or the 'off'
(In reply to comment #29) > in anycase the upstream build system should respect the --disable, or the > 'off' ...if this was plain --enable/--disable case, this would be a bug in libpng and missing $enableval or $withval in the configure.ac's AC_ARG_{ENABLE,WITH} when talking of default --enable/--disable options we patch those to respect both --enable/--disable, like for in eg. here: http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/media-sound/vorbis-tools/files/vorbis-tools-1.2.0-with-args.patch?hideattic=0&view=markup (sorry everyone for the massive bugspam, btw ;-)
Those Gentoo ebuilds for 1.5 and 1.6 that employ 'usex' rather than 'use_enable' should be modified to completely remove all the 'neon' references. This is because when the fixes were made to --enable-arm-neon (to change it from a yes/no to a multi-value) other fixes were made so that the build does the right thing by default. If there's a problem with the *default* then I'd like to understand what that problem is, and it is easy to patch the NEON support out of libpng in a portable way until it works by passing "-DPNG_NO_ARM_NEON" in the CFLAGS. ./configure --help explains the --enable values. The issue seems to be that --disable-arm-neon didn't actually disable the ARM code on a machine that supports it, which is slightly confusing. I think I have a fix for that, as well as the slightly different treatments of 'no' and 'off'. I believe the hard-wired '-neon' in current Gentoo builds should be unnecessary if the change above is made, once a 1.5 ebuild that doesn't IUSE neon is stable.
(In reply to comment #31) > I believe the hard-wired '-neon' in current Gentoo builds should be > unnecessary if the change above is made, once a 1.5 ebuild that doesn't IUSE > neon is stable. I'm not 100% sure what you mean by hard-wired '-neon' but even if user has ARM NEON capable CPU, and toolchain, he might still want to build libpng without neon so he can distribute the generated binary package across other ARM hosts that not necessarily have sama capabilities as the build host Thus hard-wiring enable/on/yes and disable/off/no must always be respected In case of $(use_enable foobar) when USE="foobar" is enabled --enable-foobar gets passed to ./configure In case of $(use_enable foobar) when USE="-foobar" is enabled --disable-foobar gets passed to ./configure In case of $(usex foobar) when USE="foobar" is enabled 'yes' gets returned In case of $(usex foobar) when USE="-foobar" is enabled 'no' gets returned In case of $(usex foobar on off) when USE="foobar" is enabled 'on' gets returned In case of $(usex foobar on off) when USE="-foobar" is enabled 'off' gets returned Thus, anykind of 'auto' is out of question for the distribution package (ebuild) for the outcome to be portable and predictable. But 'auto' sounds like a sane default, we just don't have use for it
(In reply to comment #32) > I'm not 100% sure what you mean by hard-wired '-neon' I'll try to be more clear. What I am saying applies to *Gentoo* emerge builds; it doesn't apply to binary distributions where the target is not known at compile time and this is what is *MEANT* to happen; clearly there are bugs... Prior to libpng-1.5.15 libpng need to IUSE the neon USE flag because neon support had to be turned on or off by the build system. As of libpng-1.5.15 and libpng-1.6.1 libpng should *NOT* IUSE neon. This is because the libpng build now gets the right answer when building on the target (as is always the case with Gentoo). (If it doesn't that is a bug in libpng.) This is controlled by the owner of the Gentoo system from /etc/portage/make.conf. The easy approach which always works is to set "-march=native" in the CFLAGS, but it's also possible to explicitly ask the compiler to generate, or not generate, NEON instructions. This is exactly the same as, for example, Intel SSE. Gentoo does still need a neon USE flag for packages where explicit enable/disable is required, but libpng should *NOT* be using it. >but even if user has > ARM NEON capable CPU, and toolchain, he might still want to build libpng > without neon so he can distribute the generated binary package across other > ARM hosts that not necessarily have sama capabilities as the build host But that isn't a Gentoo build; Gentoo is a source distribution and libpng always gets built on the target machine. When cross-compiling libpng supports two options to allow NEON support to be turned on at run-time, however it isn't necessary to use these. The intention, once again, is that the cross-compiler sets the right macros to indicate the capabilities of the target system and the libpng build checks these. For that reason --enable-arm-neon should never be set unless the compiler flags need to be overridden. That can be done for testing - indeed, I have to do this when I test builds. It is also necessary when the run-time options are used, in this case the build is being told to build NEON instructions even though the compiler says they are not supported. For x86 targets, however, --enable-arm-neon should *NOT* be specified.
(In reply to comment #33) mmm, not quite. libpng-1.5.15/1.6.1 defaults to turning neon off entirely. AM_CONDITIONAL([PNG_ARM_NEON], [test "${enable_arm_neon:-no}" != "no"]) that means distro builds (Gentoo) are required to specify a flag if they want neon support to be available. if we dig further, the logic in the configure script is dumb and doesn't accept the standard "yes" and "no" values. instead, it requires you to pick from "on" or "off". it even says "check" shouldn't be used. so we have to choose from: off on api since we know what the user wants (USE=neon), picking between off and on are the only logical answer. so the current ebuild is correct.
(In reply to comment #34) > (In reply to comment #33) > > mmm, not quite. libpng-1.5.15/1.6.1 defaults to turning neon off entirely. > > AM_CONDITIONAL([PNG_ARM_NEON], [test "${enable_arm_neon:-no}" != "no"]) Indeed, that is a bug. After all, the help in configure.ac explicitly says that the *default* (when nothing is specified) is to do the optimizations if the compiler says __ARM_NEON__: > that means distro builds (Gentoo) are required to specify a flag if they > want neon support to be available. Or file a bug because it doesn't work the way the help says it should work... > if we dig further, the logic in the configure script is dumb It's broken. > and doesn't > accept the standard "yes" and "no" values. instead, it requires you to pick > from "on" or "off". it even says "check" shouldn't be used. so we have to > choose from: > off on api > > since we know what the user wants (USE=neon), picking between off and on are > the only logical answer. It should accept 'no', that's another bug. I deliberately disabled 'yes' because previously 'yes' meant 'check' and simply didn't work on multiple systems (Android and, potentially, Windows 8). >so the current ebuild is correct. I disagree; it turns on compilation of ARM specific files on x86 builds. Now that is, of course, because of a third bug in libpng, but you can't argue that the ebuild is correct because it avoids two bugs and stumbles over a third. If you delete the IUSE and the associated lines from the ebuild you stop anyone building with the NEON optimizations until all the libpng bugs are fixed, but you also fix this bug, which is reported against x86_64/Darwin. Then you can quite righteously proclaim that the libpng NEON configure support is ill-tested and totally broken and it will get fixed, PDQ.
(In reply to comment #35) yes, i can argue that the current ebuild is correct because there is no way for it to work for arm targets *except* for what is exactly in there right now. assuming a future libpng release acts the way you describe, i don't think we will want to drop the USE flag since, while not exactly the best configuration, it's perfectly valid to have a system that does: CFLAGS="-march=armv7-a -pipe -O2" USE="neon" or to do: CFLAGS="-march=armv7-a -pipe -O2 -mfpu=neon" USE="-noen" in both of these scenarios, the neon optimizations in libpng should be set in a way that contradicts the result from automatic detection. it can even be useful when debugging misbehavior at runtime. plenty of people have written hand coded assembly for mmx/sse/etc... that had subtle bugs and simply turning off the USE flag got them a working system w/out compromising the optimizations by gcc everywhere else. the use of -Wa,--execstack is orthogonal to broken configure logic. there's no good reason for that flag to be in there at all. it's simultaneously GNU binutils/gcc specific and unnecessary even for those targets. the patch i posted should be applied regardless of the configure behavior wrt neon.
(In reply to comment #36) > in both of these scenarios, the neon optimizations in libpng should be set > in a way that contradicts the result from automatic detection. it can even > be useful when debugging misbehavior at runtime. plenty of people have > written hand coded assembly for mmx/sse/etc... that had subtle bugs and > simply turning off the USE flag got them a working system w/out compromising > the optimizations by gcc everywhere else. I've just sent Glenn a patch that, if I got it right this time, removes the safety harness. In other words if --enable-arm-neon=explode is set then 'explode' will happen, regardless of any other information. The 'default' behavior is retained if nothing is passed on the configure command line and, I believe, it may now actually work. A simple --enable-arm-neon should now behave as --enable-arm-neon=on (i.e. it turns the code on unconditionally.) So far as I can see prior to my patch the only way to turn the NEON optimizations on was to pass command line (-march etc) flags that caused the compiler to set __ARM_NEON__ (pretty difficult, IRC) *and* to pass one or other of the --enable-arm-neon options.
(In reply to comment #36) > the use of -Wa,--execstack is orthogonal to broken configure logic. Indeed. That patch is already in the system, at least for 1.6.3. I believe all the changes will be back-ported to 1.5 too.
(In reply to comment #38) It's currently in libpng-1.7.0beta10. It'll be in libpng-1.6.3beta01 and libpng-1.5.16beta01 later this week, after libpng-1.6.2 is released tomorrow and after we are satisfied with the implementation in libpng-1.7.0beta. I don't have the capability of testing on ARM platforms so am eager to see test results.
sounds good guys. i look forward to the next release :).
As Glenn says it would be good to have a positive test result for this; I can cross-compile for ARM but I don't have a working test system at present (I've go a Miele A1000 and I've got a Raspberry Pi but I haven't had time to work out how to actually get a development system on there.) Also my cross-compilers don't set __ARM_NEON__, though they do compile the NEON instructions fine.
(In reply to comment #41) i'm in the process of getting a new armv7 Gentoo system hosted publicly. there was a networking configuration hiccup, but hopefully we should have that resolved soon. i can get you guys ssh access to that system if you want. it's a smdk 5250, so it should be plenty fast.
(In reply to comment #42) > (In reply to comment #41) > > i'm in the process of getting a new armv7 Gentoo system hosted publicly. > there was a networking configuration hiccup, but hopefully we should have > that resolved soon. i can get you guys ssh access to that system if you > want. > > it's a smdk 5250, so it should be plenty fast. That would work work, my test scripts are highly automated, I just need to tweak my firewall to allow access to my development git repos, or maybe I'll just push to github.
1.6.2 in Portage with modified (and temporary) patch from Comment #15 to avoid -Wa,--noexecstack, so with 1.6.3 release we will simply drop the temporary patch and get the upstream fix No reason to keep this open anymore. People can still discuss here if they want.
thanks so much for the work/solution!
