]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
bpf: remove {update,get}_loop_entry functions
authorEduard Zingerman <eddyz87@gmail.com>
Wed, 11 Jun 2025 20:08:34 +0000 (13:08 -0700)
committerAlexei Starovoitov <ast@kernel.org>
Thu, 12 Jun 2025 23:52:43 +0000 (16:52 -0700)
The previous patch switched read and precision tracking for
iterator-based loops from state-graph-based loop tracking to
control-flow-graph-based loop tracking.

This patch removes the now-unused `update_loop_entry()` and
`get_loop_entry()` functions, which were part of the state-graph-based
logic.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20250611200836.4135542-9-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
include/linux/bpf_verifier.h
kernel/bpf/verifier.c

index b0273f759589c69bf63b52d9234cc29f64a0f0e8..1ae588679e20467d9be941fab0035cd367d27dc3 100644 (file)
@@ -449,16 +449,6 @@ struct bpf_verifier_state {
        /* first and last insn idx of this verifier state */
        u32 first_insn_idx;
        u32 last_insn_idx;
-       /* If this state is a part of states loop this field points to some
-        * parent of this state such that:
-        * - it is also a member of the same states loop;
-        * - DFS states traversal starting from initial state visits loop_entry
-        *   state before this state.
-        * Used to compute topmost loop entry for state loops.
-        * State loops might appear because of open coded iterators logic.
-        * See get_loop_entry() for more information.
-        */
-       struct bpf_verifier_state *loop_entry;
        /* if this state is a backedge state then equal_state
         * records cached state to which this state is equal.
         */
@@ -473,11 +463,6 @@ struct bpf_verifier_state {
        u32 dfs_depth;
        u32 callback_unroll_depth;
        u32 may_goto_depth;
-       /* If this state was ever pointed-to by other state's loop_entry field
-        * this flag would be set to true. Used to avoid freeing such states
-        * while they are still in use.
-        */
-       u32 used_as_loop_entry;
 };
 
 #define bpf_get_spilled_reg(slot, frame, mask)                         \
index aa1bb4be7b8b0cc74023506c6f5495142a07ce23..48847f8da5b1d06c24a5b55db7731b0e0b08f60a 100644 (file)
@@ -1682,7 +1682,7 @@ static void free_verifier_state(struct bpf_verifier_state *state,
                kfree(state);
 }
 
-/* struct bpf_verifier_state->{parent,loop_entry} refer to states
+/* struct bpf_verifier_state->parent refers to states
  * that are in either of env->{expored_states,free_list}.
  * In both cases the state is contained in struct bpf_verifier_state_list.
  */
@@ -1693,22 +1693,12 @@ static struct bpf_verifier_state_list *state_parent_as_list(struct bpf_verifier_
        return NULL;
 }
 
-static struct bpf_verifier_state_list *state_loop_entry_as_list(struct bpf_verifier_state *st)
-{
-       if (st->loop_entry)
-               return container_of(st->loop_entry, struct bpf_verifier_state_list, state);
-       return NULL;
-}
-
 static bool incomplete_read_marks(struct bpf_verifier_env *env,
                                  struct bpf_verifier_state *st);
 
 /* A state can be freed if it is no longer referenced:
  * - is in the env->free_list;
  * - has no children states;
- * - is not used as loop_entry.
- *
- * Freeing a state can make it's loop_entry free-able.
  */
 static void maybe_free_verifier_state(struct bpf_verifier_env *env,
                                      struct bpf_verifier_state_list *sl)
@@ -1765,9 +1755,7 @@ static int copy_verifier_state(struct bpf_verifier_state *dst_state,
        dst_state->last_insn_idx = src->last_insn_idx;
        dst_state->dfs_depth = src->dfs_depth;
        dst_state->callback_unroll_depth = src->callback_unroll_depth;
-       dst_state->used_as_loop_entry = src->used_as_loop_entry;
        dst_state->may_goto_depth = src->may_goto_depth;
-       dst_state->loop_entry = src->loop_entry;
        dst_state->equal_state = src->equal_state;
        for (i = 0; i <= src->curframe; i++) {
                dst = dst_state->frame[i];
@@ -1811,157 +1799,6 @@ static bool same_callsites(struct bpf_verifier_state *a, struct bpf_verifier_sta
        return true;
 }
 
-/* Open coded iterators allow back-edges in the state graph in order to
- * check unbounded loops that iterators.
- *
- * In is_state_visited() it is necessary to know if explored states are
- * part of some loops in order to decide whether non-exact states
- * comparison could be used:
- * - non-exact states comparison establishes sub-state relation and uses
- *   read and precision marks to do so, these marks are propagated from
- *   children states and thus are not guaranteed to be final in a loop;
- * - exact states comparison just checks if current and explored states
- *   are identical (and thus form a back-edge).
- *
- * Paper "A New Algorithm for Identifying Loops in Decompilation"
- * by Tao Wei, Jian Mao, Wei Zou and Yu Chen [1] presents a convenient
- * algorithm for loop structure detection and gives an overview of
- * relevant terminology. It also has helpful illustrations.
- *
- * [1] https://api.semanticscholar.org/CorpusID:15784067
- *
- * We use a similar algorithm but because loop nested structure is
- * irrelevant for verifier ours is significantly simpler and resembles
- * strongly connected components algorithm from Sedgewick's textbook.
- *
- * Define topmost loop entry as a first node of the loop traversed in a
- * depth first search starting from initial state. The goal of the loop
- * tracking algorithm is to associate topmost loop entries with states
- * derived from these entries.
- *
- * For each step in the DFS states traversal algorithm needs to identify
- * the following situations:
- *
- *          initial                     initial                   initial
- *            |                           |                         |
- *            V                           V                         V
- *           ...                         ...           .---------> hdr
- *            |                           |            |            |
- *            V                           V            |            V
- *           cur                     .-> succ          |    .------...
- *            |                      |    |            |    |       |
- *            V                      |    V            |    V       V
- *           succ                    '-- cur           |   ...     ...
- *                                                     |    |       |
- *                                                     |    V       V
- *                                                     |   succ <- cur
- *                                                     |    |
- *                                                     |    V
- *                                                     |   ...
- *                                                     |    |
- *                                                     '----'
- *
- *  (A) successor state of cur   (B) successor state of cur or it's entry
- *      not yet traversed            are in current DFS path, thus cur and succ
- *                                   are members of the same outermost loop
- *
- *                      initial                  initial
- *                        |                        |
- *                        V                        V
- *                       ...                      ...
- *                        |                        |
- *                        V                        V
- *                .------...               .------...
- *                |       |                |       |
- *                V       V                V       V
- *           .-> hdr     ...              ...     ...
- *           |    |       |                |       |
- *           |    V       V                V       V
- *           |   succ <- cur              succ <- cur
- *           |    |                        |
- *           |    V                        V
- *           |   ...                      ...
- *           |    |                        |
- *           '----'                       exit
- *
- * (C) successor state of cur is a part of some loop but this loop
- *     does not include cur or successor state is not in a loop at all.
- *
- * Algorithm could be described as the following python code:
- *
- *     traversed = set()   # Set of traversed nodes
- *     entries = {}        # Mapping from node to loop entry
- *     depths = {}         # Depth level assigned to graph node
- *     path = set()        # Current DFS path
- *
- *     # Find outermost loop entry known for n
- *     def get_loop_entry(n):
- *         h = entries.get(n, None)
- *         while h in entries:
- *             h = entries[h]
- *         return h
- *
- *     # Update n's loop entry if h comes before n in current DFS path.
- *     def update_loop_entry(n, h):
- *         if h in path and depths[entries.get(n, n)] < depths[n]:
- *             entries[n] = h1
- *
- *     def dfs(n, depth):
- *         traversed.add(n)
- *         path.add(n)
- *         depths[n] = depth
- *         for succ in G.successors(n):
- *             if succ not in traversed:
- *                 # Case A: explore succ and update cur's loop entry
- *                 #         only if succ's entry is in current DFS path.
- *                 dfs(succ, depth + 1)
- *                 h = entries.get(succ, None)
- *                 update_loop_entry(n, h)
- *             else:
- *                 # Case B or C depending on `h1 in path` check in update_loop_entry().
- *                 update_loop_entry(n, succ)
- *         path.remove(n)
- *
- * To adapt this algorithm for use with verifier:
- * - use st->branch == 0 as a signal that DFS of succ had been finished
- *   and cur's loop entry has to be updated (case A), handle this in
- *   update_branch_counts();
- * - use st->branch > 0 as a signal that st is in the current DFS path;
- * - handle cases B and C in is_state_visited().
- */
-static struct bpf_verifier_state *get_loop_entry(struct bpf_verifier_env *env,
-                                                struct bpf_verifier_state *st)
-{
-       struct bpf_verifier_state *topmost = st->loop_entry;
-       u32 steps = 0;
-
-       while (topmost && topmost->loop_entry) {
-               if (verifier_bug_if(steps++ > st->dfs_depth, env, "infinite loop"))
-                       return ERR_PTR(-EFAULT);
-               topmost = topmost->loop_entry;
-       }
-       return topmost;
-}
-
-static void update_loop_entry(struct bpf_verifier_env *env,
-                             struct bpf_verifier_state *cur, struct bpf_verifier_state *hdr)
-{
-       /* The hdr->branches check decides between cases B and C in
-        * comment for get_loop_entry(). If hdr->branches == 0 then
-        * head's topmost loop entry is not in current DFS path,
-        * hence 'cur' and 'hdr' are not in the same loop and there is
-        * no need to update cur->loop_entry.
-        */
-       if (hdr->branches && hdr->dfs_depth < (cur->loop_entry ?: cur)->dfs_depth) {
-               if (cur->loop_entry) {
-                       cur->loop_entry->used_as_loop_entry--;
-                       maybe_free_verifier_state(env, state_loop_entry_as_list(cur));
-               }
-               cur->loop_entry = hdr;
-               hdr->used_as_loop_entry++;
-       }
-}
-
 /* Return IP for a given frame in a call stack */
 static u32 frame_insn_idx(struct bpf_verifier_state *st, u32 frame)
 {