This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Hi Maciej, I'm standing by the patch since I believe it does fix a real assembler bug. However, if you still think it should be reverted after reading this message, I'll do so. "Maciej W. Rozycki" <macro at ds2 dot pg dot gda dot pl> writes: > I object. > > Essentially you kill lazy binding for code as currently emitted by gcc > entirely this way. True. I'm getting used to mips-rewrite, so I forgot what older gccs (including trunk ;) do. But more below. > And I don't think you have to. While you have > incorrect CALL relocations in the object file, indeed, they will be > converted to regular GOT references upon the final link as the function > will not get a stub (as this is an undefined weak symbol as I understand; > otherwise it wouldn't be zero or there would be other relocations related > to taking the function's address). If the function gets a stub > regardless, there is a bug elsewhere, probably in BFD -- try to narrow it > down then. Yes, the problem I came across was with undefined weak symbols. I think the same sort of thing could happen for defined weak symbols as well. Example: if (foo != dont_run_this) foo (); I've attached a runnable test case, in the form of a shell script. I tried both: CROSS=mipsel-linux-gnu- REG=24 ./tc and CROSS=mipsel-linux-gnu- REG=25 ./tc The ./main produced by the first command runs OK. The second one segfaults. In the second case, the weak symbol (foo) has a stub in lib.so. But this seems correct to me. The only reloc that refers to the symbol is an R_MIPS_CALL16. And I can't find anything in the ABI which says that undefined weak symbols are not allowed to have stubs. FWIW, I tried the same experiment on irix (with -32). I used gas to assemble lib.s but used the irix ld and cc to do the rest. The irix linker also uses a stub for foo. So again, the test case works if you use REG=24, but if you use REG=25, you get: 15021780:./main: rld: Fatal Error: attempted access to unresolvable symbol in ./lib.so: foo As I understand it, the idea behind the old behaviour was that gcc generated: la $25,foo jal $31,$25 and we wanted GAS to treat "la $25,foo" as part of a call sequence. But the question (which I think you asked then) is: why does gcc split the call in the first place? I see from the archives that one answer was: to allow better scheduling. But I don't think it does. Quoting from trunk mips.md: (define_insn "call_internal2" [(call (mem (match_operand 0 "call_insn_operand" "ri")) (match_operand 1 "" "i")) (clobber (match_operand:SI 2 "register_operand" "=d"))] "TARGET_ABICALLS && !TARGET_LONG_CALLS" "* { register rtx target = operands[0]; if (GET_CODE (target) == CONST_INT) return \"li\\t%^,%0\\n\\tjal\\t%2,%^\"; else if (CONSTANT_ADDRESS_P (target)) { if (GET_MODE (target) == SImode) return \"la\\t%^,%0\\n\\tjal\\t%2,%^\"; else return \"dla\\t%^,%0\\n\\tjal\\t%2,%^\"; } else if (REGNO (target) != PIC_FUNCTION_ADDR_REGNUM) return \"move\\t%^,%0\\n\\tjal\\t%2,%^\"; else return \"jal\\t%2,%0\"; }" [(set_attr "type" "call") (set_attr "mode" "none") (set_attr "length" "8")]) So gcc is actually emitting: (call (mem (symbol_ref "foo")) ...) as a fixed-form assembler string: la $25,foo jal $25,$31 There's no scheduling benefit here. The only reason I can think of doing it is to avoid a nop between the load and the jump when assembling with -mips2. If that is indeed the reason, it doesn't seem to be a very good one. gas inserts all sorts of other nops (even for -mips2) when expanding PIC macros. IMO we should fix the assembler instead. Note that gcc's call expanders don't force the address into a register, so SYMBOL_REF calls are the norm. When we do have indirect calls, the 'r' constraint will allow any GPR, not just $25. For example, try compiling: void foo (void) __attribute__ ((weak)); void bar (void) { if (foo) foo (); } I get: la $2,foo .set noreorder .set nomacro beq $2,$0,$L1 sw $28,24($sp) .set macro .set reorder move $25,$2 jal $31,$25 $L1: [I was wrong in my original mail when I said that gcc didn't copy the GPR to $25. Doh! Serves me right for not checking.] So in summary, gcc seems to be going out of its way not to emit "jal foo" when that's exactly what the assembler wants to see. I think the correct fix is to make historical[*] gccs use "jal %0" for the CONSTANT_ADDRESS_P case. I don't think we should hack bfd to prevent lazy binding in the test case below. Why should conforming code be penalised because of a strange gccism? [*] this has already been "fixed" on mips-rewrite branch. > One could argue the whole heuristics is fragile, to say at least, and I > would wholeheartedly agree. It should be replaced with explicit setting > of relocation attributes using "%call_hi", "%call_lo", "%got_hi" and > "%got_lo" (I think there should be two additional operations like "%call" > and "%got"[*] for the "la" and related macros themselves). I discussed it > at this list a few years ago and IIRC there was an agreement that is a > good idea with the heuristics being a temporary solution until gcc gets > improved. Well, the code is still raw, but mips-rewrite does what you suggest, at least for n32 and n64 PIC. There's a patch (pending review ;) to extend it to o32 PIC as well. But gcc will probably need to have an "assembler macro" mode for some time (for example, to support o32 on irix). Richard PS. I see I forgot to change the -xgot case. I'll do that as an obvious follow-up if the current patch stays.
Attachment:
tc
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |