Bug 70610 – TEXTREL in libjavascriptcoregtk-1.0.so.0.11.0 on x86 (or i586)

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Alexey Shabalin]

own verifying ELF objects script say about error

either the file containing the function '_ZN3JSC3JIT13testPrototypeENS_7JSValueERNS_22AbstractMacroAssemblerINS_12X86AssemblerEE8JumpListE' or the file containing the function 'cti_op_put_by_id_generic' is not compiled with -fpic/-fPIC
either the file containing the function 'jsRoundThunk' or the file containing the function 'expThunk' is not compiled with -fpic/-fPIC
either the file containing the function 'expThunk' or the file containing the function 'logThunk' is not compiled with -fpic/-fPIC
either the file containing the function 'logThunk' or the file containing the function 'floorThunk' is not compiled with -fpic/-fPIC
either the file containing the function 'floorThunk' or the file containing the function 'ceilThunk' is not compiled with -fpic/-fPIC
either the file containing the function 'ceilThunk' or the file containing the function 'jsRound' is not compiled with -fpic/-fPIC

verify-elf: ERROR: ./usr/lib/libjavascriptcoregtk-1.0.so.0.11.0: TEXTREL entry found: 0x00000000

on x86_64 all fine.

[Martin Robinson]

What platform? Did you build yourself? How do you reproduce this?

[Alexey Shabalin]

Created attachment 112169 [details]
fix TEXTREL

fix TEXTREL

[Alexey Shabalin]

I was build for ALTLinux for x86.
Gentoo have some warning. http://gentwoo.elisp.net/emerges/88345

QA Notice: The following files contain runtime text relocations
 Text relocations force the dynamic linker to perform extra
 work at startup, waste system resources, and may pose a security
 risk.  On some architectures, the code may not even function
 properly, if at all.
 For more information, see http://hardened.gentoo.org/pic-fix-guide.xml
 Please include the following list of files in your report:
TEXTREL usr/lib/libjavascriptcoregtk-3.0.so.0.11.0

For reproduce this:
- build for i586 with --disable-silent-rules
- you can see for libjavascriptcoregtk
  "/usr/bin/ld: warning: creating a DT_TEXTREL in a shared object."
- after build you can check:

$ eu-findtextrel ./libwebkitgtk-3.0.so.0.11.0 
eu-findtextrel: no text relocations reported in './libwebkitgtk-3.0.so.0.11.0'

$ eu-findtextrel ./libjavascriptcoregtk-3.0.so.0.11.0 
either the file containing the function '_ZN3JSC3JIT13testPrototypeENS_7JSValueERNS_22AbstractMacroAssemblerINS_12X86AssemblerEE8JumpListE' or the file containing the function 'cti_op_put_by_id_generic' is not compiled with -fpic/-fPIC
either the file containing the function 'jsRoundThunk' or the file containing the function 'expThunk' is not compiled with -fpic/-fPIC
either the file containing the function 'expThunk' or the file containing the function 'logThunk' is not compiled with -fpic/-fPIC
either the file containing the function 'logThunk' or the file containing the function 'floorThunk' is not compiled with -fpic/-fPIC
either the file containing the function 'floorThunk' or the file containing the function 'ceilThunk' is not compiled with -fpic/-fPIC
either the file containing the function 'ceilThunk' or the file containing the function 'jsRound' is not compiled with -fpic/-fPIC

simple patch in attach, but i have doubts about Source/JavaScriptCore/jit/JSInterfaceJIT.h in #elif CPU(X86)
 static const RegisterID regT3 = X86Registers::ebx;

[Martin Robinson]

CCing some people more familiar with JSC internals.

[Oliver Hunt]

Comment on attachment 112169 [details]
fix TEXTREL

This fix seems reasonable to me -- it's just a matter of ensuring the all the asm we have is correctly annotated to allow it to be shuffled around.

[WebKit Review Bot]

Attachment 112169 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files']" exit_code: 1

Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Martin Robinson]

Comment on attachment 112169 [details]
fix TEXTREL

I will land this one. Thanks for the patch Alexey. In the future when you make a patch, do you mind making it against trunk and include a ChangeLog? See: http://www.webkit.org/coding/contributing.html

[Martin Robinson]

Committed r99089: <http://trac.webkit.org/changeset/99089>

[Alexey Shabalin]

Hello.
This patch is not complete fix.
After apply this patch you can not build seed (http://live.gnome.org/Seed) on linux x86(i386, not x86_64). I don't know how fix it.

I think, should be reopen bug.

[Martin Robinson]

(In reply to comment #9)
> After apply this patch you can not build seed (http://live.gnome.org/Seed) on linux x86(i386, not x86_64). I don't know how fix it.
> 
> I think, should be reopen bug.

In that case it's probably best to roll out the patch.

[Martin Robinson]

(In reply to comment #10)
> (In reply to comment #9)
> > After apply this patch you can not build seed (http://live.gnome.org/Seed) on linux x86(i386, not x86_64). I don't know how fix it.

How does the build break?

[Csaba Osztrogonác]

It made many tests crash on the Qt bot :-/ (only 32 bit)

[Zoltan Herczeg]

(In reply to comment #12)
> It made many tests crash on the Qt bot :-/ (only 32 bit)

ThunkGenerators.cpp:140
0xf6ccf694 <floorThunk+8>:      call   0xf5960f18 <floor@plt>
x $ebx+0x218c
0xf171004b:     0x00000000

and there is a crash after that:
0x0:    Cannot access memory at address 0x0

Thus, although the symbol is defined it points to NULL.

I think the idea is good, but we need a more sophisticated check what is available in the target system.

[Zoltan Herczeg]

Oh, one line is missing.

> ThunkGenerators.cpp:140
> 0xf6ccf694 <floorThunk+8>:      call   0xf5960f18 <floor@plt>

0xf5960f18 <floor@plt>: jmp    *0x218c(%ebx)

> x $ebx+0x218c
> 0xf171004b:     0x00000000

So <floor@plt> does an indirect jump to a NULL address.

[Zoltan Herczeg]

I created a rollout: https://bugs.webkit.org/show_bug.cgi?id=71448

[Csaba Osztrogonác]

(In reply to comment #15)
> I created a rollout: https://bugs.webkit.org/show_bug.cgi?id=71448

Rollout landed in http://trac.webkit.org/changeset/99152

[Magnus Granberg]

Any progress on this bug? I still get TEXTREL when building
webkit-gtk-2.2.5 on Gentoo x86.
mv -f Tools/DumpRenderTree/gtk/.deps/Programs_DumpRenderTree-EventSender.Tpo Tools/DumpRenderTree/gtk/.deps/Programs_DumpRenderTree-EventSender.Po
mv -f Tools/DumpRenderTree/gtk/.deps/Programs_DumpRenderTree-PixelDumpSupportGtk.Tpo Tools/DumpRenderTree/gtk/.deps/Programs_DumpRenderTree-PixelDumpSupportGtk.Po
mv -f Source/JavaScriptCore/.deps/Programs_jsc_1-jsc.Tpo Source/JavaScriptCore/.deps/Programs_jsc_1-jsc.Po
mv -f Tools/DumpRenderTree/gtk/.deps/Programs_DumpRenderTree-WorkQueueItemGtk.Tpo Tools/DumpRenderTree/gtk/.deps/Programs_DumpRenderTree-WorkQueueItemGtk.Po
mv -f Tools/DumpRenderTree/gtk/.deps/Programs_DumpRenderTree-DumpRenderTree.Tpo Tools/DumpRenderTree/gtk/.deps/Programs_DumpRenderTree-DumpRenderTree.Po
/usr/lib/gcc/i686-pc-linux-gnu/4.7.3/../../../../i686-pc-linux-gnu/bin/ld: Source/JavaScriptCore/jit/.libs/libjavascriptcoregtk_1_0_la-ThunkGenerators.o: warning: relocation against `floor@@GLIBC_2.0' in readonly section `.text'.
/usr/lib/gcc/i686-pc-linux-gnu/4.7.3/../../../../i686-pc-linux-gnu/bin/ld: warning: creating a DT_TEXTREL in object.
libtool: link: (cd ".libs" && rm -f "libjavascriptcoregtk-1.0.so.0" && ln -s "libjavascriptcoregtk-1.0.so.0.15.10" "libjavascriptcoregtk-1.0.so.0")
libtool: link: (cd ".libs" && rm -f "libjavascriptcoregtk-1.0.so" && ln -s "libjavascriptcoregtk-1.0.so.0.15.10" "libjavascriptcoregtk-1.0.so")
libtool: link: ( cd ".libs" && rm -f "libjavascriptcoregtk-1.0.la" && ln -s "../libjavascriptcoregtk-1.0.la" "libjavascriptcoregtk-1.0.la" )

[Magnus Granberg]

Created attachment 226616 [details]
Updated patch to fix the textrel

This patch is a update of the patch that was posted before.
Don't know if it will have that same prob as the old one for i
can't run the testsuite.

[Early Warning System Bot]

Attachment 226616 [details] did not pass style-queue:


Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Magnus Granberg]

Created attachment 226618 [details]
Patch to add plt to fix the textrel

Update to fix the typo in the last patch.
To fix the textrel.

[Early Warning System Bot]

Attachment 226618 [details] did not pass style-queue:


Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alberto Garcia]

(In reply to comment #20)
> Created an attachment (id=226618) [details]
> Patch to add plt to fix the textrel

The browser crashes all the time with this patch. Did you try to run
GtkLauncher or the MiniBrowser?

[Magnus Granberg]

Created attachment 227339 [details]
add plt and fix %ebx for use with plt

* Source/WTF/wtf/InlineASM.h GLOBAL_REFERENCE: Add plt if PIC
* Source/JavaScriptCore/jit/ThunkGenerators.cpp defineUnaryDoubleOpWrapper: if PIC then setup %ebx so we can use plt

[Magnus Granberg]

(In reply to comment #22)
> (In reply to comment #20)
> > Created an attachment (id=226618) [details] [details]
> > Patch to add plt to fix the textrel
> 
> The browser crashes all the time with this patch. Did you try to run
> GtkLauncher or the MiniBrowser?
The prob is that i can't test any x86 stuff only compile.
the asm code was missing to setup %ebx with the needed info for plt.

[Alberto Garcia]

(In reply to comment #23)
> Created an attachment (id=227339) [details]
> add plt and fix %ebx for use with plt

This actually seems to work fine, thanks!

[Alberto Garcia]

I did a full rebuild of webkitgtk with this patch and I haven't noticed any problem, I think we can go ahead with it.

Thanks again for the patch.

[Alberto Garcia]

Created attachment 227634 [details]
Patch

I rebased your patch and added the ChangeLog entry.

[Alberto Garcia]

Debian has been shipping this patch for a while and we didn't detect any problem.

[Alberto Garcia]

Committed r172759: <http://trac.webkit.org/changeset/172759>