while merging... # TEXTREL ./usr/lib/libmp3lame.so.0.0.0 -------------------------------- Text relocations require a lot of extra work to be preformed by the dynamic linker which will cause serious performance impact on IA-32 and might not function properly on other architectures hppa for example. -------------------------------- These are ELF Q/A problems. eu-findtextrel reports that the the file containing the function choose_table_MMX or the function has_MMX_nasm are not fully pic aware. I first tested the --with-pic without success. I then tested without the $(use_enable x86 nasm) as the other tool hinted towards nasm already and liblame.so is now free of text relocations. Till such time as the asm used within libmp3lame.so is pic aware and free of text relocations we should not enable/force the use of nasm by default. ferringb helped me test lame without the inline nasm on irc and he said "works, no discernible difference mp3 sounds fine also"
fixed in cvs, thanks.
Reopening. We could benefit from a fix instead of disabling it like it has been done. I was thinking about adding a mmx useflag but this would cause the textrels to reappear. If someone has the courrage to propose a fix as it then has textrels and, as a bonus, exec stacks. For quick benchs of the mmx vs non mmx version, see bug #230860 comment #4
Created attachment 160249 [details, diff] textrel fix for lame 3.98 first attempt, someone please go through it and check that i got all the modifications right ;). on a sidenote, lame doesn't have asm code for amd64, so don't enable it there (re bug #230860).
as for GNU_STACK, nasm.h can produce a proper one but it's guarded by #ifdef LINUX and i haven't seen that defined anywhere, i don't know where it was supposed to come from, nasm doesn't define it per se for sure. so you can just define it yourself or just remove the #ifdef altogether. note that the textrel patch is also ELF specific, yet it's not guarded by #ifdef's, so it's not ready for upstream as it is.
(In reply to comment #3) > Created an attachment (id=160249) [edit] > textrel fix for lame 3.98 > > first attempt, someone please go through it and check that i got all the > modifications right ;). Thanks, but it gives me a segfault ;) (gdb) run audiodump.wav Starting program: /usr/bin/lame audiodump.wav LAME 3.98 32bits (http://www.mp3dev.org/) CPU features: MMX (ASM used), 3DNow! (ASM used), SSE (ASM used), SSE2 Using polyphase lowpass filter, transition band: 16538 Hz - 17071 Hz Encoding audiodump.wav to audiodump.wav.mp3 Encoding as 44.1 kHz j-stereo MPEG-1 Layer III (11x) 128 kbps qval=3 Frame | CPU time/estim | REAL time/estim | play/CPU | ETA 0/10322 ( 0%)| 0:00/ 0:00| 0:00/ 0:00| 0.0000x| 0:00 04:29----------------------------------------------------------------------------------------------------------------------- kbps % % 0.0 Program received signal SIGSEGV, Segmentation fault. 0xf7f770f8 in choose_table_MMX.H_dual_lp1 () from /usr/lib/libmp3lame.so.0 (gdb) bt #0 0xf7f770f8 in choose_table_MMX.H_dual_lp1 () from /usr/lib/libmp3lame.so.0 #1 0x00000086 in ?? () #2 0xf7f8bff4 in ?? () from /usr/lib/libmp3lame.so.0 #3 0x0000001e in ?? () #4 0xf7f7265c in noquant_count_bits (gfc=0x971f470, gi=0x9729a34, prev_noise=0x0) at takehiro.c:734 #5 0xf7f72ade in count_bits (gfc=0x971f470, xr=0xff7c2d74, gi=0x9729a34, prev_noise=0x0) at takehiro.c:788 #6 0xf7f6b620 in outer_loop (gfp=0x971e860, cod_info=0x9729a34, l3_xmin=0xff7c3674, xrpow=0xff7c2d74, ch=0, targ_bits=855) at quantize.c:364 #7 0xf7f6ca80 in CBR_iteration_loop (gfp=0x971e860, pe=0xff7c864c, ms_ener_ratio=0xff7c867c, ratio=0xff7c76ec) at quantize.c:2017 #8 0xf7f5adbd in lame_encode_mp3_frame (gfp=0x971e860, inbuf_l=0x971f480, inbuf_r=0x97232c0, mp3buf=0xff7ec770 "��\220d", mp3buf_size=147456) at encoder.c:535 #9 0xf7f5f0d3 in lame_encode_buffer_sample_t (gfp=0x971e860, buffer_l=0x9737090, buffer_r=0x9738298, nsamples=1152, mp3buf=0xff7ec770 "��\220d", mp3buf_size=147456) at lame.c:1422 #10 0x0804b2f7 in lame_encoder (gf=0x971e860, outf=0x971eb30, nogap=0, inPath=0xff8dac99 "audiodump.wav", outPath=0xff8dcc9b "audiodump.wav.mp3") at main.c:485 #11 0x0804c599 in main (argc=-7478668, argv=0x0) at main.c:855 (gdb) x/8i $pc 0xf7f770f8 <choose_table_MMX.H_dual_lp1>: movq (%edx,%ecx,1),%mm0 0xf7f770fc <choose_table_MMX.H_dual_lp1+4>: movq 0x8(%edx,%ecx,1),%mm1 0xf7f77101 <choose_table_MMX.H_dual_lp1+9>: packssdw %mm1,%mm0 0xf7f77104 <choose_table_MMX.H_dual_lp1+12>: movq %mm0,%mm2 0xf7f77107 <choose_table_MMX.H_dual_lp1+15>: paddusw %mm5,%mm0 0xf7f7710a <choose_table_MMX.H_dual_lp1+18>: pcmpgtw %mm6,%mm2 0xf7f7710d <choose_table_MMX.H_dual_lp1+21>: pmaddwd %mm3,%mm0 0xf7f77110 <choose_table_MMX.H_dual_lp1+24>: movd %mm0,%ebx (gdb) i r eax 0x6 6 ecx 0x14c50 85072 edx 0x972a3ac 158507948 ebx 0xfffffef0 -272 esp 0xff7c0bb0 0xff7c0bb0 ebp 0xf7f8bff4 0xf7f8bff4 esi 0xdddfd5bf -572533313 edi 0x971f470 158463088 eip 0xf7f770f8 0xf7f770f8 <choose_table_MMX.H_dual_lp1> eflags 0x10206 [ PF IF RF ] cs 0x23 35 ss 0x2b 43 ds 0x2b 43 es 0x2b 43 fs 0x0 0 gs 0x63 99 please let me know if you need additional information.
Created attachment 160547 [details, diff] fixed lame pic fix patch thanks, it seems i got a bit overzealous in adjusting the stack at one point ;), should work now.
one more thing: it seems that some dependencies are incorrect as i can't build it with -j3.
(In reply to comment #6) > Created an attachment (id=160547) [edit] > fixed lame pic fix patch > > thanks, it seems i got a bit overzealous in adjusting the stack at one point > ;), should work now. Thanks I've committed it. It's still not enabled but people can test it with EXTRA_ECONF=--enable-nasm How do you think we should fix the last remaining issue, exec stacks ? I've replaced the %ifdef LINUX by %ifidn __OUTPUT_FORMAT__,elf in nasm.h and it seems fine. (In reply to comment #7) > one more thing: it seems that some dependencies are incorrect as i can't build > it with -j3. Weird, it never failed here. Though I have only one cpu ;) If you happen to understand the problem, please let me know.
(In reply to comment #8) > Thanks I've committed it. It's still not enabled but people can test it with > EXTRA_ECONF=--enable-nasm > > How do you think we should fix the last remaining issue, exec stacks ? I've > replaced the %ifdef LINUX by %ifidn __OUTPUT_FORMAT__,elf in nasm.h and it > seems fine. that's fine with me, the 'proper' approach really depends on whether and how you want to push this upstream (as they care about more systems than gentoo does). > (In reply to comment #7) > > one more thing: it seems that some dependencies are incorrect as i can't build > > it with -j3. > > Weird, it never failed here. Though I have only one cpu ;) > If you happen to understand the problem, please let me know. try it on a real SMP box ;). anyway, i didn't bother to look, i think you can disable parallel make in portage for now.
Created attachment 160597 [details, diff] one more fix should fix the crash reported in #230860.
Thanks for the updated patch. It doesn't segfault anymore.
patch updated, thanks and parallel make should be fine also
it seems we're all good now, I'm going to commit this (In reply to comment #3) > on a sidenote, lame doesn't have asm code for amd64, so > don't enable it there (re bug #230860). @amd64: FYI I'm going to use.mask mmx useflag for lame on your profiles, it won't be built anyway so it doesn't hurt but that'll save people from installing nasm if they don't have it.
all done, thanks everyone!
Does the PaX team plan to get this upstream?
(In reply to comment #15) > Does the PaX team plan to get this upstream? no, i don't (not enough time to fight these kinds of things through ;), so feel free.
Congrats, your changes made it into lame 3.98.2: sk@jigsaw:~/tmp/lame-398-2-tet$ scanelf -qe frontend/lame sk@jigsaw:~/tmp/lame-398-2-tet$ scanelf -qt frontend/lame 2008-08-31 19:35 robert * libmp3lame/i386/nasm.h (lame3_98): fixing the fix to get it assemble for ELF again 2008-08-31 12:26 robert * libmp3lame/i386/: choose_table.nas, fft3dn.nas, fftsse.nas, nasm.h (lame3_98): fixing none PIC assembling problem 2008-08-25 12:57 rbrito * debian/changelog, libmp3lame/i386/choose_table.nas, libmp3lame/i386/fft3dn.nas, libmp3lame/i386/fftsse.nas (lame3_98): Fix text relocations and update debian changelog. Thanks to the PaX Team and Gentoo people. 2008-08-25 12:43 rbrito * libmp3lame/i386/nasm.h (lame3_98): Fix to executable stack (thanks to Gentoo people) 2008-08-25 12:38 rbrito * libmp3lame/i386/Makefile.am (lame3_98): Fixes building of asm with libtool 2.2 (thanks to Gentoo people)
(In reply to comment #17) > Congrats, your changes made it into lame 3.98.2: > sk@jigsaw:~/tmp/lame-398-2-tet$ scanelf -qe frontend/lame > sk@jigsaw:~/tmp/lame-398-2-tet$ scanelf -qt frontend/lame thanks, any chance to push this version into portage then?
(In reply to comment #18) > (In reply to comment #17) > > Congrats, your changes made it into lame 3.98.2: > > sk@jigsaw:~/tmp/lame-398-2-tet$ scanelf -qe frontend/lame > > sk@jigsaw:~/tmp/lame-398-2-tet$ scanelf -qt frontend/lame > > thanks, any chance to push this version into portage then? *hrm... It's been in portage for a few days now: *lame-3.98.2 (23 Sep 2008) 23 Sep 2008; Peter Alfredsen <loki_val@gentoo.org> +lame-3.98.2.ebuild: Bump to 3.98.2, bug 238443. Drop a bunch of patches that were merged upstream. Thanks for the big effort you put into this. Much appreciated.
(In reply to comment #19) > *hrm... > It's been in portage for a few days now: > *lame-3.98.2 (23 Sep 2008) hah, i must have sync'ed just before it went in, sorry about the noise ;)
