This is the mail archive of the 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]

Re: [GOLD] Support --icf=safe with -pie for x86_64

Hi Alan and Cary,

Thanks for your feedback. Some comments below.

> I looked (maybe not carefully enough), but I didn't find anything in
> this call chain that restricts the check to text sections:
> gc_process_relocs
>  -> scan.[global|local]_reloc_may_be_function_pointer
>  -> possible_function_pointer_reloc
> You need to check the SHF_EXECINST flag before assuming you're looking
> at an opcode. (Look in for "is_executable".)

I'll take a closer look at this, add a check, and send an updated patch.

> I think you should also be checking that the target symbol is
> STT_FUNC; that should rule out most jump table cases. (I see a check
> for != STT_OBJECT in the local symbol path, but nothing in the global
> symbol path. It may be the case that STT_NOTYPE is used for some
> extern function symbols, but you want to be conservative, right?)

Ditto for this one.

> > x86 code editing/analysis is difficult.  If I had to do it, I think
> > I'd define a few more relocs for use in code so that you could find
> > the start of an instruction.  You could even do that without bloating
> > objects with extra relocs.  eg. instead of R_X86_64_32 in code you'd
> > use a series of relocs that all behave like R_X86_64_32 for relocation
> > but also say something about the insn opcode:
> >   R_X86_64_32_I1 insn opcode one byte before r_offset
> >   R_X86_64_32_I2 insn opcode two bytes before r_offset
> >   etc.
> Actually, I'd much prefer separate reloc types that just tells the
> linker whether it's a call or a materialization of a function pointer.
> In an ideal world, the linker should never have to load section
> contents until it's applying relocations in pass 2. I really dislike
> looking at opcodes. Everything we need to know in pass 1 should be
> available by scanning the relocations.

I agree completely. The need for this patch (and any related heuristics) comes
from the fact that the same relocation type is being used for both function
calls and taking the address of the function. The compiler already knows the
difference, and an ideal solution would be to simply use different relocation
types for different use cases.

What would it take to reach that solution? How involved is it to add a new
relocation type for something like this? I would guess that some coordination
is needed with gcc (and other compilers) for this to work. On the binutils
side, a new relocation type would need support in gas/objdump/ld/gold and maybe
other places as well. Can you point me to a recent example for adding a similar
new relocation type so I can get an idea of what it entails?


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]