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

[Cary Coutant <ccoutant at gmail dot com>]

>> Also, might you have an R_X86_64_PC32 in data and so be looking at the
>> high byte of the previous word?
>
> Not sure what you mean here, but this code is only called for text sections, so
> we can be sure that the relocation offset is part of an instruction. Did you
> mean an instruction with an encoding containning opcode bytes, followed by
> immediate operand, which is followed by a R_X86_64_PC32 relocation?

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 x86_64.cc for "is_executable".)


>> For a jump table containing relocations, it should be fine to treat all of them
>> as function calls/jumps, right?
>
> I don't know.  I could look at gold source and figure out the answer,
> but it's really your job to convince Cary and others that what you're
> doing is safe.  ie. Prove to some level of confidence that comparing
> the byte before r_offset of an R_X86_64_PC32 against 0xe8 really is a
> valid test for a call.  Note that I'm *not* saying your patch was
> wrong.  It may in fact be the case that all insns that could use an
> R_X86_64_PC32 besides a call will have modrm or sib bytes different
> to 0xe8.  Ditto for the other opcodes you test.  It might also be the
> case that gcc won't put jump tables using R_X86_64_PC32 in code
> sections.  (What crazy assembly programmers do is another story.)

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?)


> 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.

-cary