objtool: Assume only ELF functions do sibling calls
authorJosh Poimboeuf <jpoimboe@redhat.com>
Thu, 21 Jan 2021 21:29:22 +0000 (15:29 -0600)
committerJosh Poimboeuf <jpoimboe@redhat.com>
Tue, 26 Jan 2021 17:12:00 +0000 (11:12 -0600)
There's an inconsistency in how sibling calls are detected in
non-function asm code, depending on the scope of the object.  If the
target code is external to the object, objtool considers it a sibling
call.  If the target code is internal but not a function, objtool
*doesn't* consider it a sibling call.

This can cause some inconsistencies between per-object and vmlinux.o
validation.

Instead, assume only ELF functions can do sibling calls.  This generally
matches existing reality, and makes sibling call validation consistent
between vmlinux.o and per-object.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Link: https://lore.kernel.org/r/0e9ab6f3628cc7bf3bde7aa6762d54d7df19ad78.1611263461.git.jpoimboe@redhat.com
tools/objtool/check.c

index 5f5949951ca76400aef96e74c41765e7f2f10a40..b4e1655017de4284484b12d6505add63f44624f1 100644 (file)
@@ -110,15 +110,20 @@ static struct instruction *prev_insn_same_sym(struct objtool_file *file,
 
 static bool is_sibling_call(struct instruction *insn)
 {
+       /*
+        * Assume only ELF functions can make sibling calls.  This ensures
+        * sibling call detection consistency between vmlinux.o and individual
+        * objects.
+        */
+       if (!insn->func)
+               return false;
+
        /* An indirect jump is either a sibling call or a jump to a table. */
        if (insn->type == INSN_JUMP_DYNAMIC)
                return list_empty(&insn->alts);
 
-       if (!is_static_jump(insn))
-               return false;
-
        /* add_jump_destinations() sets insn->call_dest for sibling calls. */
-       return !!insn->call_dest;
+       return (is_static_jump(insn) && insn->call_dest);
 }
 
 /*
@@ -774,7 +779,7 @@ static int add_jump_destinations(struct objtool_file *file)
                        continue;
 
                reloc = find_reloc_by_dest_range(file->elf, insn->sec,
-                                              insn->offset, insn->len);
+                                                insn->offset, insn->len);
                if (!reloc) {
                        dest_sec = insn->sec;
                        dest_off = arch_jump_destination(insn);
@@ -794,18 +799,21 @@ static int add_jump_destinations(struct objtool_file *file)
 
                        insn->retpoline_safe = true;
                        continue;
-               } else if (reloc->sym->sec->idx) {
-                       dest_sec = reloc->sym->sec;
-                       dest_off = reloc->sym->sym.st_value +
-                                  arch_dest_reloc_offset(reloc->addend);
-               } else {
-                       /* external sibling call */
+               } else if (insn->func) {
+                       /* internal or external sibling call (with reloc) */
                        insn->call_dest = reloc->sym;
                        if (insn->call_dest->static_call_tramp) {
                                list_add_tail(&insn->static_call_node,
                                              &file->static_call_list);
                        }
                        continue;
+               } else if (reloc->sym->sec->idx) {
+                       dest_sec = reloc->sym->sec;
+                       dest_off = reloc->sym->sym.st_value +
+                                  arch_dest_reloc_offset(reloc->addend);
+               } else {
+                       /* non-func asm code jumping to another file */
+                       continue;
                }
 
                insn->jump_dest = find_insn(file, dest_sec, dest_off);
@@ -854,7 +862,7 @@ static int add_jump_destinations(struct objtool_file *file)
                        } else if (insn->jump_dest->func->pfunc != insn->func->pfunc &&
                                   insn->jump_dest->offset == insn->jump_dest->func->offset) {
 
-                               /* internal sibling call */
+                               /* internal sibling call (without reloc) */
                                insn->call_dest = insn->jump_dest->func;
                                if (insn->call_dest->static_call_tramp) {
                                        list_add_tail(&insn->static_call_node,
@@ -2587,7 +2595,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 
                case INSN_JUMP_CONDITIONAL:
                case INSN_JUMP_UNCONDITIONAL:
-                       if (func && is_sibling_call(insn)) {
+                       if (is_sibling_call(insn)) {
                                ret = validate_sibling_call(insn, &state);
                                if (ret)
                                        return ret;
@@ -2609,7 +2617,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 
                case INSN_JUMP_DYNAMIC:
                case INSN_JUMP_DYNAMIC_CONDITIONAL:
-                       if (func && is_sibling_call(insn)) {
+                       if (is_sibling_call(insn)) {
                                ret = validate_sibling_call(insn, &state);
                                if (ret)
                                        return ret;