On 5/14/2025 4:45 AM, Josh Poimboeuf wrote:
On Tue, May 13, 2025 at 10:49:59PM +0800, laokz wrote:
On 5/10/2025 4:17 AM, Josh Poimboeuf wrote:
+
+#define sym_for_each_reloc(elf, sym, reloc)                            \
+       for (reloc = find_reloc_by_dest_range(elf, sym->sec,         \
+                                             sym->offset, sym->len);     \
+            reloc && reloc_offset(reloc) <  sym->offset + sym->len;   \
+            reloc = rsec_next_reloc(sym->sec->rsec, reloc))

This macro intents to walk through ALL relocations for the 'sym'. It seems
we have the assumption that, there is at most one single relocation for the
same offset and find_reloc_by_dest_range only needs to do 'less than' offset
comparison:

        elf_hash_for_each_possible(reloc, reloc, hash,
                                   sec_offset_hash(rsec, o)) {
                if (reloc->sec != rsec)
                        continue;
                if (reloc_offset(reloc) >= offset &&
                    reloc_offset(reloc) < offset + len) {
less than ==>                if (!r || reloc_offset(reloc) < reloc_offset(r))
                                        r = reloc;

Because if there were multiple relocations for the same offset, the returned
one would be the last one in section entry order(hash list has reverse order
against section order), then broken the intention.

Right.  Is that a problem?  I don't believe I've ever seen two
relocations for the same offset.


Thanks for the clarification. I asked this because I noticed the patchset have done some code refactoring, so guess if we could make it more general to other architectures which not support objtool yet. Such as RISC-V, it is not unusual having multiple relocs for same offset, like vmlinux.o might have:

000c 0000010a00000017 R_RISCV_PCREL_HI20 0000000000000000 .LANCHOR0 + 48
000c 0000000000000033 R_RISCV_RELAX                         48

0044 0000061700000023 R_RISCV_ADD32   0000000000000048 pe_head_start + 0
0044 000dd5b900000027 R_RISCV_SUB32   0000000000000002 _start + 0

But it is a bit off-topic:/

Regards,
laokz


Reply via email to