RESOLVED FIXED 137434
[JSC] Segfault on ARM64 with gcc in elf_dynamic_do_Rela 
https://bugs.webkit.org/show_bug.cgi?id=137434
Summary [JSC] Segfault on ARM64 with gcc in elf_dynamic_do_Rela
Akos Kiss
Reported 2014-10-05 07:09:36 PDT
When building a debug jsc for ARM64/Linux/GTK with gcc (4.8.3, as shipped with Ubuntu 14.04.1), the resulting binary is completely unusable. The execution of the app stops with segfault very early, as revealed by gdb: Program received signal SIGSEGV, Segmentation fault. elf_dynamic_do_Rela (skip_ifunc=<optimized out>, lazy=0, nrelative=<optimized out>, relsize=<optimized out>, reladdr=<optimized out>, map=0x7fb7ffb770) at do-rel.h:112 112 do-rel.h: No such file or directory. This is still the dynamic linking phase of jsc to its dependencies. After digging deeper, it turned out that some dynamic relocations in libjavascriptcoregtk.so cause the problem. Further analysis revealed that some symbols in the object file compiled from jit/ThunkGenerators.cpp are in the wrong section, in .text instead of .data. The following is an excerpt from the output of objdump run on ThunkGenerators.cpp.o, i.e., from the symbol table: 0000000000000000 l O .data.rel 0000000000000008 _ZN3JSCL14jsRoundWrapperE 0000000000001e68 l O .text 0000000000000008 _ZN3JSCL10expWrapperE 0000000000001e78 l O .text 0000000000000008 _ZN3JSCL10logWrapperE 0000000000001e88 l O .text 0000000000000008 _ZN3JSCL12floorWrapperE 0000000000001e98 l O .text 0000000000000008 _ZN3JSCL11ceilWrapperE The following lines are the relevant part of the source code: #elif CPU(ARM64) #define defineUnaryDoubleOpWrapper(function) \ asm( \ ".text\n" \ ".align 2\n" \ ".globl " SYMBOL_STRING(function##Thunk) "\n" \ HIDE_SYMBOL(function##Thunk) "\n" \ SYMBOL_STRING(function##Thunk) ":" "\n" \ "b " GLOBAL_REFERENCE(function) "\n" \ ); \ extern "C" { \ MathThunkCallingConvention function##Thunk(MathThunkCallingConvention); \ } \ static MathThunk UnaryDoubleOpWrapper(function) = &function##Thunk; #else #define defineUnaryDoubleOpWrapper(function) \ static MathThunk UnaryDoubleOpWrapper(function) = 0 #endif defineUnaryDoubleOpWrapper(jsRound); defineUnaryDoubleOpWrapper(exp); defineUnaryDoubleOpWrapper(log); defineUnaryDoubleOpWrapper(floor); defineUnaryDoubleOpWrapper(ceil); And now comes the revelation: this bug has nothing to do with the GTK port but it's a bug in gcc. The following minimized example also exhibits the problem: #include <math.h> #define ASM_AND_VAR(fn) \ asm(".text"); \ static double (*var##fn)(double) = &fn; ASM_AND_VAR(exp); ASM_AND_VAR(log); The assembly output is: .cpu cortex-a53+fp+simd .file "asmtext.cpp" #APP .text #NO_APP .data .align 3 .type _ZL6varexp, %object .size _ZL6varexp, 8 _ZL6varexp: .xword exp #APP .text #NO_APP .align 3 .type _ZL6varlog, %object .size _ZL6varlog, 8 _ZL6varlog: .xword log After the second asm(".text"); (or rather: before the second static double variable) gcc does not insert a .data assembler directive. So, it seems GTK is only suffering because it puts the JS engine into a shared lib instead of linking it statically (the static linker gets over this issue).
Attachments
Proposed patch. (1.89 KB, patch)
2014-10-05 07:20 PDT, Akos Kiss 		no flags
DetailsFormatted DiffDiff
Proposed patch, v2 (2.01 KB, patch)
2014-10-09 04:47 PDT, Akos Kiss 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Akos Kiss
Comment 1 2014-10-05 07:20:39 PDT
Created attachment 239299 [details] Proposed patch.
Akos Kiss
Comment 2 2014-10-06 01:22:28 PDT
Update: According to conversations with gcc devs (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63461), it seems that this is not a bug in gcc but expected (alternatively: intentionally undefined) behaviour. The solution proposed at gcc bugzilla is to use .pushsection/.popsection, however, I'd still propose for the current .text/.data approach. .text is apropriately translated to .section __TEXT,__text on Mac and to .section .text on Linux (.data similarly), while .pushsection ".text" would not be so portable. If I got it right.
Michael Saboff
Comment 3 2014-10-07 13:07:11 PDT
(In reply to comment #2) > Update: According to conversations with gcc devs (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63461), it seems that this is not a bug in gcc but expected (alternatively: intentionally undefined) behaviour. > > The solution proposed at gcc bugzilla is to use .pushsection/.popsection, however, I'd still propose for the current .text/.data approach. .text is apropriately translated to .section __TEXT,__text on Mac and to .section .text on Linux (.data similarly), while .pushsection ".text" would not be so portable. If I got it right. What about using .previous instead of .data? That should be more portable.
Akos Kiss
Comment 4 2014-10-09 04:47:11 PDT
Created attachment 239527 [details] Proposed patch, v2 You're right, .previous works -- on my ARM64/Linux at least. (On that box, I've been experimenting with some corner cases to check that different compilers -- gcc & clang -- behave as expected, and found no issues, fortunately. I've also tried out some .pushsection/.popsection-based approaches but all turned out to need more and potentially harder to maintain changes.) I could not validate the patch for iOS. Interestingly, all the other defines use the same .text-based approach for defining the thunks with no .data/.previous/.pushsection-.popsection around and noone ran into this problem before. Still I guess that it would be safer to have .previous in those macros as well, but following the "if it's not broken, don't fix it" line, I've made no changes there.
Michael Saboff
Comment 5 2014-10-09 08:22:54 PDT
Comment on attachment 239527 [details] Proposed patch, v2 r=me
WebKit Commit Bot
Comment 6 2014-10-09 08:59:13 PDT
Comment on attachment 239527 [details] Proposed patch, v2 Clearing flags on attachment: 239527 Committed r174503: <http://trac.webkit.org/changeset/174503>
WebKit Commit Bot
Comment 7 2014-10-09 08:59:16 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.