| Message ID | yddr3zmwcjd.fsf@lokon.CeBiTec.Uni-Bielefeld.DE |
|---|---|
| State | New |
| Headers | show |
Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes: > Hi Richard, >> Several targets define a function like i386's get_some_local_dynamic_name. >> The function looks through the current output function and returns the first >> (arbitrary) local-dynamic symbol that it finds. The result can be used in >> a call to __tls_get_addr, since all local-dynamic symbols have the same base. >> >> This patch replaces the various target functions with a single generic one. >> The only difference between the implementations was that s390 checked >> for constant pool references while the others didn't need to (because >> they don't allow TLS symbols to be forced into the pool). Checking for >> constant pool references is unnecessary but harmless for the other ports. >> Also, the walk is needed only once per TLS-referencing output function, >> so it's hardly critical in terms of compile time. >> >> All uses of this function are in final. In general it wouldn't be >> safe to call the function earlier than that, since the symbol reference >> could in principle be deleted by any rtl pass. I've therefore cached >> it in a variable local to final rather than in cfun (which is where >> the ports used to cache it). >> >> Also, i386 was robust against uses of %& in inline asm. The patch >> makes sure the other ports are too. Using %& in inline asm would >> often be a mistake, but it should at least trigger a proper error >> rather than an ICE. >> >> Tested on x86_64-linux-gnu. Also tested by building cross compilers >> before and after the change on: >> >> alpha-linux-gnu powerpc64-linux-gnu s390x-linux-gnu sparc64-linux-gnu >> >> OK to install? > > this patch broke Solaris/SPARC bootstrap: compiling libgo/runtime/proc.c > with -fPIC SEGVs: > > /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c: In function 'procresize': > /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:2727:1: internal compiler error: Segmentation Fault > } > ^ > 0x731c97 crash_signal > /vol/gcc/src/hg/trunk/local/gcc/toplev.c:339 > 0x45facc get_some_local_dynamic_name() > /vol/gcc/src/hg/trunk/local/gcc/final.c:1742 > 0xa0886f sparc_print_operand > /vol/gcc/src/hg/trunk/local/gcc/config/sparc/sparc.c:8789 > 0x4606a3 output_operand(rtx_def*, int) > /vol/gcc/src/hg/trunk/local/gcc/final.c:3848 > 0x460fdf output_asm_insn(char const*, rtx_def**) > /vol/gcc/src/hg/trunk/local/gcc/final.c:3778 > 0x46254b output_asm_insn(char const*, rtx_def**) > /vol/gcc/src/hg/trunk/local/gcc/recog.h:169 > 0x46254b final_scan_insn(rtx_insn*, __FILE*, int, int, int*) > /vol/gcc/src/hg/trunk/local/gcc/final.c:3027 > 0x462b53 final(rtx_insn*, __FILE*, int) > /vol/gcc/src/hg/trunk/local/gcc/final.c:2063 > 0x46470f rest_of_handle_final > /vol/gcc/src/hg/trunk/local/gcc/final.c:4470 > 0x46470f execute > /vol/gcc/src/hg/trunk/local/gcc/final.c:4545 > > Program received signal SIGSEGV, Segmentation fault. > [Switching to Thread 1 (LWP 1)] > get_some_local_dynamic_name () at /vol/gcc/src/hg/trunk/local/gcc/final.c:1742 > 1742 if (GET_CODE (x) == SYMBOL_REF) > > 1: x/i $pc > => 0x45facc <get_some_local_dynamic_name()+84>: lduh [ %o0 ], %g1 > (gdb) p $o0 > $1 = 0 > > (gdb) where > #0 get_some_local_dynamic_name () at /vol/gcc/src/hg/trunk/local/gcc/final.c:1742 > #1 0x00a08870 in sparc_print_operand (file=0x10ab0f0 <_iob+48>, x=0x0, code=<optimized out>) at /vol/gcc/src/hg/trunk/local/gcc/config/sparc/sparc.c:8789 > #2 0x004606a4 in output_operand (x=0x0, code=38) at /vol/gcc/src/hg/trunk/local/gcc/final.c:3848 > #3 0x00460fe0 in output_asm_insn (templ=templ@entry=0xf1e5c8 "sethi\t%%tldm_hi22(%&), %0", operands=0x107feb0 <recog_data>) at /vol/gcc/src/hg/trunk/local/gcc/final.c:3778 > #4 0x0046254c in output_asm_insn (operands=<optimized out>, templ=0xf1e5c8 "sethi\t%%tldm_hi22(%&), %0") at /vol/gcc/src/hg/trunk/local/gcc/recog.h:169 > #5 final_scan_insn (insn=insn@entry=0xfac70de8, file=file@entry=0x10ab0f0 <_iob+48>, optimize_p=optimize_p@entry=2, nopeepholes=nopeepholes@entry=0, seen=seen@entry=0xffbfef0c) at /vol/gcc/src/hg/trunk/local/gcc/final.c:3027 > #6 0x00462b54 in final (first=0xfac62020, file=0x10ab0f0 <_iob+48>, optimize_p=2) at /vol/gcc/src/hg/trunk/local/gcc/final.c:2063 > #7 0x00464710 in rest_of_handle_final () at /vol/gcc/src/hg/trunk/local/gcc/final.c:4470 > #8 (anonymous namespace)::pass_final::execute (this=0x10d9050) at /vol/gcc/src/hg/trunk/local/gcc/final.c:4545 > #9 0x00661cf4 in execute_one_pass (pass=pass@entry=0x10d9050) at /vol/gcc/src/hg/trunk/local/gcc/passes.c:2151 > #10 0x0066235c in execute_pass_list_1 (pass=0x10d9050) at /vol/gcc/src/hg/trunk/local/gcc/passes.c:2203 > #11 0x00662380 in execute_pass_list_1 (pass=0x10d8630) at /vol/gcc/src/hg/trunk/local/gcc/passes.c:2204 > #12 0x00662380 in execute_pass_list_1 (pass=0x10d7988, pass@entry=0x10d54a8) at /vol/gcc/src/hg/trunk/local/gcc/passes.c:2204 > #13 0x006623c4 in execute_pass_list (fn=0xfb5fc5b0, pass=0x10d54a8) at /vol/gcc/src/hg/trunk/local/gcc/passes.c:2214 > #14 0x0037d728 in cgraph_node::expand (this=this@entry=0xfb5f5180) at /vol/gcc/src/hg/trunk/local/gcc/cgraphunit.c:1744 > #15 0x0037f078 in expand_all_functions () at /vol/gcc/src/hg/trunk/local/gcc/cgraphunit.c:1880 > #16 symbol_table::compile (this=0xfb410000) at /vol/gcc/src/hg/trunk/local/gcc/cgraphunit.c:2209 > #17 0x00380ee4 in symbol_table::finalize_compilation_unit (this=0xfb410000) at /vol/gcc/src/hg/trunk/local/gcc/cgraphunit.c:2286 > #18 0x0020ee34 in c_write_global_declarations () at /vol/gcc/src/hg/trunk/local/gcc/c/c-decl.c:10431 > #19 0x00731d58 in compile_file () at /vol/gcc/src/hg/trunk/local/gcc/toplev.c:564 > #20 0x00734604 in do_compile () at /vol/gcc/src/hg/trunk/local/gcc/toplev.c:1945 > #21 toplev_main (argc=10, argv=0xffbff424) at /vol/gcc/src/hg/trunk/local/gcc/toplev.c:2021 > #22 0x001f1184 in _start () > Backtrace stopped: previous frame identical to this frame (corrupt stack?) > > It seems the new get_some_local_dynamic_name implementation in > function.c lost the non-NULL check the sparc.c version had. I'm > currently testing the following patch: Could you do a: call debug_rtx (...) on the insn that contains a null pointer? Normally insn patterns shouldn't contain nulls, so I was wondering whether this was some SPARC-specific construct. Thanks, Richard
Hi Richard, >> It seems the new get_some_local_dynamic_name implementation in >> function.c lost the non-NULL check the sparc.c version had. I'm >> currently testing the following patch: > > Could you do a: > > call debug_rtx (...) > > on the insn that contains a null pointer? Normally insn patterns > shouldn't contain nulls, so I was wondering whether this was some > SPARC-specific construct. proved a bit difficult to do: at the default -O2, insn was optimized away, at -g3 -O0, I only got can't compute CFA for this frame with gdb 7.8 even after recompiling all of the gcc dir with -g3 -O0. Here's what I find after inserting the call in the source: (insn 30 6 28 (sequence [ (call_insn:TI 8 6 7 (parallel [ (set (reg:SI 8 %o0) (call (mem:SI (unspec:SI [ (symbol_ref:SI ("__tls_get_addr")) ] UNSPEC_TLSLDM) [0 S4 A32]) (const_int 1 [0x1]))) (clobber (reg:SI 15 %o7)) ]) /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:936 390 {tldm_call32} (expr_list:REG_EH_REGION (const_int -2147483648 [0xffffffff80000000]) (nil)) (expr_list (use (reg:SI 8 %o0)) (nil))) (insn 7 8 28 (set (reg:SI 8 %o0) (plus:SI (reg:SI 23 %l7) (unspec:SI [ (reg:SI 8 %o0 [112]) ] UNSPEC_TLSLDM))) 388 {tldm_add32} (nil)) ]) /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:936 -1 (nil)) Rainer
Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes: > Hi Richard, >>> It seems the new get_some_local_dynamic_name implementation in >>> function.c lost the non-NULL check the sparc.c version had. I'm >>> currently testing the following patch: >> >> Could you do a: >> >> call debug_rtx (...) >> >> on the insn that contains a null pointer? Normally insn patterns >> shouldn't contain nulls, so I was wondering whether this was some >> SPARC-specific construct. > > proved a bit difficult to do: at the default -O2, insn was optimized > away, at -g3 -O0, I only got > > can't compute CFA for this frame > > with gdb 7.8 even after recompiling all of the gcc dir with -g3 -O0. > > Here's what I find after inserting the call in the source: > > (insn 30 6 28 (sequence [ > (call_insn:TI 8 6 7 (parallel [ > (set (reg:SI 8 %o0) > (call (mem:SI (unspec:SI [ > (symbol_ref:SI ("__tls_get_addr")) > ] UNSPEC_TLSLDM) [0 S4 A32]) > (const_int 1 [0x1]))) > (clobber (reg:SI 15 %o7)) > ]) /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:936 390 {tldm_call32} > (expr_list:REG_EH_REGION (const_int -2147483648 [0xffffffff80000000]) > (nil)) > (expr_list (use (reg:SI 8 %o0)) > (nil))) > (insn 7 8 28 (set (reg:SI 8 %o0) > (plus:SI (reg:SI 23 %l7) > (unspec:SI [ > (reg:SI 8 %o0 [112]) > ] UNSPEC_TLSLDM))) 388 {tldm_add32} > (nil)) > ]) /vol/gcc/src/hg/trunk/local/libgo/runtime/proc.c:936 -1 > (nil)) Bah, a sequence. Hadn't thought of that. IMO it's a bug for a walk on a PATTERN to pull in non-PATTERN parts of an insn. We should really be looking at the patterns of the two subinsns instead and ignore the other stuff. Let me have a think about it. Now that we know the underlying cause, I personally wouldn't mind the null check being committed as a workaround. If you do though, please could you add a comment saying that this is for SEQUENCEs? Thanks, Richard
diff --git a/gcc/final.c b/gcc/final.c --- a/gcc/final.c +++ b/gcc/final.c @@ -1739,7 +1739,7 @@ get_some_local_dynamic_name () FOR_EACH_SUBRTX (iter, array, PATTERN (insn), ALL) { const_rtx x = *iter; - if (GET_CODE (x) == SYMBOL_REF) + if (x && GET_CODE (x) == SYMBOL_REF) { if (SYMBOL_REF_TLS_MODEL (x) == TLS_MODEL_LOCAL_DYNAMIC) return some_local_dynamic_name = XSTR (x, 0);