eBPF ISA specification requires high 32-bit cleared when low 32-bit sub-register is written. This applies to destination register of ALU32 etc. JIT back-ends must guarantee this semantic when doing code-gen. x86_64 and AArch64 ISA has the same semantics, so the corresponding JIT back-end doesn't need to do extra work. However, 32-bit arches (arm, x86, nfp etc.) and some other 64-bit arches (PowerPC, SPARC etc) need to do explicit zero extension to meet this requirement, otherwise code like the following will fail. u64_value = (u64) u32_value ... other uses of u64_value This is because compiler could exploit the semantic described above and save those zero extensions for extending u32_value to u64_value, these JIT back-ends are expected to guarantee this through inserting extra zero extensions which however could be a significant increase on the code size. Some benchmarks show there could be ~40% sub-register writes out of total insns, meaning at least ~40% extra code-gen. One observation is these extra zero extensions are not always necessary. Take above code snippet for example, it is possible u32_value will never be casted into a u64, the value of high 32-bit of u32_value then could be ignored and extra zero extension could be eliminated. This patch implements this idea, insns defining sub-registers will be marked when the high 32-bit of the defined sub-register matters. For those unmarked insns, it is safe to eliminate high 32-bit clearnace for them. Algo: - Split read flags into READ32 and READ64. - Record index of insn that does sub-register write. Keep the index inside reg state and update it during verifier insn walking. - A full register read on a sub-register marks its definition insn as needing zero extension on dst register. A new sub-register write overrides the old one. - When propagating read64 during path pruning, also mark any insn defining a sub-register that is read in the pruned path as full-register.
Unified · Split
| @@ -36,9 +36,11 @@ · 35 unmodified lines | ||
| 36 | 36 | */ |
| 37 | 37 | enum bpf_reg_liveness { |
| 38 | 38 | REG_LIVE_NONE = 0, /* reg hasn't been read or written this branch */ |
| 39 | − REG_LIVE_READ, /* reg was read, so we're sensitive to initial value */ | |
| 40 | − REG_LIVE_WRITTEN, /* reg was written first, screening off later reads */ | |
| 41 | − REG_LIVE_DONE = 4, /* liveness won't be updating this register anymore */ | |
| 39 | + REG_LIVE_READ32 = 0x1, /* reg was read, so we're sensitive to initial value */ | |
| 40 | + REG_LIVE_READ64 = 0x2, /* likewise, but full 64-bit content matters */ | |
| 41 | + REG_LIVE_READ = REG_LIVE_READ32 | REG_LIVE_READ64, | |
| 42 | + REG_LIVE_WRITTEN = 0x4, /* reg was written first, screening off later reads */ | |
| 43 | + REG_LIVE_DONE = 0x8, /* liveness won't be updating this register anymore */ | |
| 42 | 44 | }; |
| 43 | 45 | |
| 44 | 46 | struct bpf_reg_state { |
| @@ -131,6 +133,11 @@ struct bpf_reg_state { · 86 unmodified lines | ||
| 131 | 133 | * pointing to bpf_func_state. |
| 132 | 134 | */ |
| 133 | 135 | u32 frameno; |
| 136 | + /* Tracks subreg definition. The stored value is the insn_idx of the | |
| 137 | + * writing insn. This is safe because subreg_def is used before any insn | |
| 138 | + * patching which only happens after main verification finished. | |
| 139 | + */ | |
| 140 | + s32 subreg_def; | |
| 134 | 141 | enum bpf_reg_liveness live; |
| 135 | 142 | }; |
| 136 | 143 | |
| @@ -233,6 +240,7 @@ struct bpf_insn_aux_data { · 96 unmodified lines | ||
| 233 | 240 | int ctx_field_size; /* the ctx field size for load insn, maybe 0 */ |
| 234 | 241 | int sanitize_stack_off; /* stack slot to be cleared */ |
| 235 | 242 | bool seen; /* this insn was processed by the verifier */ |
| 243 | + bool zext_dst; /* this insn zero extends dst reg */ | |
| 236 | 244 | u8 alu_state; /* used in combination with alu_limit */ |
| 237 | 245 | bool prune_point; |
| 238 | 246 | unsigned int orig_idx; /* original instruction index */ |
| @@ -982,6 +982,7 @@ static void mark_reg_not_init(struct bpf_verifier_env *env, · 981 unmodified lines | ||
| 982 | 982 | __mark_reg_not_init(regs + regno); |
| 983 | 983 | } |
| 984 | 984 | |
| 985 | +#define DEF_NOT_SUBREG (0) | |
| 985 | 986 | static void init_reg_state(struct bpf_verifier_env *env, |
| 986 | 987 | struct bpf_func_state *state) |
| 987 | 988 | { |
| @@ -992,6 +993,7 @@ static void init_reg_state(struct bpf_verifier_env *env, · 4 unmodified lines | ||
| 992 | 993 | mark_reg_not_init(env, regs, i); |
| 993 | 994 | regs[i].live = REG_LIVE_NONE; |
| 994 | 995 | regs[i].parent = NULL; |
| 996 | + regs[i].subreg_def = DEF_NOT_SUBREG; | |
| 995 | 997 | } |
| 996 | 998 | |
| 997 | 999 | /* frame pointer */ |
| @@ -1137,7 +1139,7 @@ next: · 139 unmodified lines | ||
| 1137 | 1139 | */ |
| 1138 | 1140 | static int mark_reg_read(struct bpf_verifier_env *env, |
| 1139 | 1141 | const struct bpf_reg_state *state, |
| 1140 | − struct bpf_reg_state *parent) | |
| 1142 | + struct bpf_reg_state *parent, u8 flag) | |
| 1141 | 1143 | { |
| 1142 | 1144 | bool writes = parent == state->parent; /* Observe write marks */ |
| 1143 | 1145 | int cnt = 0; |
| @@ -1152,17 +1154,26 @@ static int mark_reg_read(struct bpf_verifier_env *env, · 8 unmodified lines | ||
| 1152 | 1154 | parent->var_off.value, parent->off); |
| 1153 | 1155 | return -EFAULT; |
| 1154 | 1156 | } |
| 1155 | − if (parent->live & REG_LIVE_READ) | |
| 1157 | + /* The first condition is more likely to be true than the | |
| 1158 | + * second, checked it first. | |
| 1159 | + */ | |
| 1160 | + if ((parent->live & REG_LIVE_READ) == flag || | |
| 1161 | + parent->live & REG_LIVE_READ64) | |
| 1156 | 1162 | /* The parentage chain never changes and |
| 1157 | 1163 | * this parent was already marked as LIVE_READ. |
| 1158 | 1164 | * There is no need to keep walking the chain again and |
| 1159 | 1165 | * keep re-marking all parents as LIVE_READ. |
| 1160 | 1166 | * This case happens when the same register is read |
| 1161 | 1167 | * multiple times without writes into it in-between. |
| 1168 | + * Also, if parent has the stronger REG_LIVE_READ64 set, | |
| 1169 | + * then no need to set the weak REG_LIVE_READ32. | |
| 1162 | 1170 | */ |
| 1163 | 1171 | break; |
| 1164 | 1172 | /* ... then we depend on parent's value */ |
| 1165 | − parent->live |= REG_LIVE_READ; | |
| 1173 | + parent->live |= flag; | |
| 1174 | + /* REG_LIVE_READ64 overrides REG_LIVE_READ32. */ | |
| 1175 | + if (flag == REG_LIVE_READ64) | |
| 1176 | + parent->live &= ~REG_LIVE_READ32; | |
| 1166 | 1177 | state = parent; |
| 1167 | 1178 | parent = state->parent; |
| 1168 | 1179 | writes = true; |
| @@ -1174,12 +1185,111 @@ static int mark_reg_read(struct bpf_verifier_env *env, · 5 unmodified lines | ||
| 1174 | 1185 | return 0; |
| 1175 | 1186 | } |
| 1176 | 1187 | |
| 1188 | +/* This function is supposed to be used by the following 32-bit optimization | |
| 1189 | + * code only. It returns TRUE if the source or destination register operates | |
| 1190 | + * on 64-bit, otherwise return FALSE. | |
| 1191 | + */ | |
| 1192 | +static bool is_reg64(struct bpf_verifier_env *env, struct bpf_insn *insn, | |
| 1193 | + u32 regno, struct bpf_reg_state *reg, enum reg_arg_type t) | |
| 1194 | +{ | |
| 1195 | + u8 code, class, op; | |
| 1196 | + | |
| 1197 | + code = insn->code; | |
| 1198 | + class = BPF_CLASS(code); | |
| 1199 | + op = BPF_OP(code); | |
| 1200 | + if (class == BPF_JMP) { | |
| 1201 | + /* BPF_EXIT for "main" will reach here. Return TRUE | |
| 1202 | + * conservatively. | |
| 1203 | + */ | |
| 1204 | + if (op == BPF_EXIT) | |
| 1205 | + return true; | |
| 1206 | + if (op == BPF_CALL) { | |
| 1207 | + /* BPF to BPF call will reach here because of marking | |
| 1208 | + * caller saved clobber with DST_OP_NO_MARK for which we | |
| 1209 | + * don't care the register def because they are anyway | |
| 1210 | + * marked as NOT_INIT already. | |
| 1211 | + */ | |
| 1212 | + if (insn->src_reg == BPF_PSEUDO_CALL) | |
| 1213 | + return false; | |
| 1214 | + /* Helper call will reach here because of arg type | |
| 1215 | + * check, conservatively return TRUE. | |
| 1216 | + */ | |
| 1217 | + if (t == SRC_OP) | |
| 1218 | + return true; | |
| 1219 | + | |
| 1220 | + return false; | |
| 1221 | + } | |
| 1222 | + } | |
| 1223 | + | |
| 1224 | + if (class == BPF_ALU64 || class == BPF_JMP || | |
| 1225 | + /* BPF_END always use BPF_ALU class. */ | |
| 1226 | + (class == BPF_ALU && op == BPF_END && insn->imm == 64)) | |
| 1227 | + return true; | |
| 1228 | + | |
| 1229 | + if (class == BPF_ALU || class == BPF_JMP32) | |
| 1230 | + return false; | |
| 1231 | + | |
| 1232 | + if (class == BPF_LDX) { | |
| 1233 | + if (t != SRC_OP) | |
| 1234 | + return BPF_SIZE(code) == BPF_DW; | |
| 1235 | + /* LDX source must be ptr. */ | |
| 1236 | + return true; | |
| 1237 | + } | |
| 1238 | + | |
| 1239 | + if (class == BPF_STX) { | |
| 1240 | + if (reg->type != SCALAR_VALUE) | |
| 1241 | + return true; | |
| 1242 | + return BPF_SIZE(code) == BPF_DW; | |
| 1243 | + } | |
| 1244 | + | |
| 1245 | + if (class == BPF_LD) { | |
| 1246 | + u8 mode = BPF_MODE(code); | |
| 1247 | + | |
| 1248 | + /* LD_IMM64 */ | |
| 1249 | + if (mode == BPF_IMM) | |
| 1250 | + return true; | |
| 1251 | + | |
| 1252 | + /* Both LD_IND and LD_ABS return 32-bit data. */ | |
| 1253 | + if (t != SRC_OP) | |
| 1254 | + return false; | |
| 1255 | + | |
| 1256 | + /* Implicit ctx ptr. */ | |
| 1257 | + if (regno == BPF_REG_6) | |
| 1258 | + return true; | |
| 1259 | + | |
| 1260 | + /* Explicit source could be any width. */ | |
| 1261 | + return true; | |
| 1262 | + } | |
| 1263 | + | |
| 1264 | + if (class == BPF_ST) | |
| 1265 | + /* The only source register for BPF_ST is a ptr. */ | |
| 1266 | + return true; | |
| 1267 | + | |
| 1268 | + /* Conservatively return true at default. */ | |
| 1269 | + return true; | |
| 1270 | +} | |
| 1271 | + | |
| 1272 | +static void mark_insn_zext(struct bpf_verifier_env *env, | |
| 1273 | + struct bpf_reg_state *reg) | |
| 1274 | +{ | |
| 1275 | + s32 def_idx = reg->subreg_def; | |
| 1276 | + | |
| 1277 | + if (def_idx == DEF_NOT_SUBREG) | |
| 1278 | + return; | |
| 1279 | + | |
| 1280 | + env->insn_aux_data[def_idx - 1].zext_dst = true; | |
| 1281 | + /* The dst will be zero extended, so won't be sub-register anymore. */ | |
| 1282 | + reg->subreg_def = DEF_NOT_SUBREG; | |
| 1283 | +} | |
| 1284 | + | |
| 1177 | 1285 | static int check_reg_arg(struct bpf_verifier_env *env, u32 regno, |
| 1178 | 1286 | enum reg_arg_type t) |
| 1179 | 1287 | { |
| 1180 | 1288 | struct bpf_verifier_state *vstate = env->cur_state; |
| 1181 | 1289 | struct bpf_func_state *state = vstate->frame[vstate->curframe]; |
| 1290 | + struct bpf_insn *insn = env->prog->insnsi + env->insn_idx; | |
| 1182 | 1291 | struct bpf_reg_state *reg, *regs = state->regs; |
| 1292 | + bool rw64; | |
| 1183 | 1293 | |
| 1184 | 1294 | if (regno >= MAX_BPF_REG) { |
| 1185 | 1295 | verbose(env, "R%d is invalid\n", regno); |
| @@ -1187,6 +1297,7 @@ static int check_reg_arg(struct bpf_verifier_env *env, u32 regno, · 1 unmodified line | ||
| 1187 | 1297 | } |
| 1188 | 1298 | |
| 1189 | 1299 | reg = ®s[regno]; |
| 1300 | + rw64 = is_reg64(env, insn, regno, reg, t); | |
| 1190 | 1301 | if (t == SRC_OP) { |
| 1191 | 1302 | /* check whether register used as source operand can be read */ |
| 1192 | 1303 | if (reg->type == NOT_INIT) { |
| @@ -1197,7 +1308,11 @@ static int check_reg_arg(struct bpf_verifier_env *env, u32 regno, · 4 unmodified lines | ||
| 1197 | 1308 | if (regno == BPF_REG_FP) |
| 1198 | 1309 | return 0; |
| 1199 | 1310 | |
| 1200 | − return mark_reg_read(env, reg, reg->parent); | |
| 1311 | + if (rw64) | |
| 1312 | + mark_insn_zext(env, reg); | |
| 1313 | + | |
| 1314 | + return mark_reg_read(env, reg, reg->parent, | |
| 1315 | + rw64 ? REG_LIVE_READ64 : REG_LIVE_READ32); | |
| 1201 | 1316 | } else { |
| 1202 | 1317 | /* check whether register used as dest operand can be written to */ |
| 1203 | 1318 | if (regno == BPF_REG_FP) { |
| @@ -1205,6 +1320,7 @@ static int check_reg_arg(struct bpf_verifier_env *env, u32 regno, · 1 unmodified line | ||
| 1205 | 1320 | return -EACCES; |
| 1206 | 1321 | } |
| 1207 | 1322 | reg->live |= REG_LIVE_WRITTEN; |
| 1323 | + reg->subreg_def = rw64 ? DEF_NOT_SUBREG : env->insn_idx + 1; | |
| 1208 | 1324 | if (t == DST_OP) |
| 1209 | 1325 | mark_reg_unknown(env, regs, regno); |
| 1210 | 1326 | } |
| @@ -1384,7 +1500,8 @@ static int check_stack_read(struct bpf_verifier_env *env, · 173 unmodified lines | ||
| 1384 | 1500 | state->regs[value_regno].live |= REG_LIVE_WRITTEN; |
| 1385 | 1501 | } |
| 1386 | 1502 | mark_reg_read(env, ®_state->stack[spi].spilled_ptr, |
| 1387 | − reg_state->stack[spi].spilled_ptr.parent); | |
| 1503 | + reg_state->stack[spi].spilled_ptr.parent, | |
| 1504 | + REG_LIVE_READ64); | |
| 1388 | 1505 | return 0; |
| 1389 | 1506 | } else { |
| 1390 | 1507 | int zeros = 0; |
| @@ -1401,7 +1518,8 @@ static int check_stack_read(struct bpf_verifier_env *env, · 10 unmodified lines | ||
| 1401 | 1518 | return -EACCES; |
| 1402 | 1519 | } |
| 1403 | 1520 | mark_reg_read(env, ®_state->stack[spi].spilled_ptr, |
| 1404 | − reg_state->stack[spi].spilled_ptr.parent); | |
| 1521 | + reg_state->stack[spi].spilled_ptr.parent, | |
| 1522 | + REG_LIVE_READ64); | |
| 1405 | 1523 | if (value_regno >= 0) { |
| 1406 | 1524 | if (zeros == size) { |
| 1407 | 1525 | /* any size read into register is zero extended, |
| @@ -2110,6 +2228,12 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn · 702 unmodified lines | ||
| 2110 | 2228 | value_regno); |
| 2111 | 2229 | if (reg_type_may_be_null(reg_type)) |
| 2112 | 2230 | regs[value_regno].id = ++env->id_gen; |
| 2231 | + /* A load of ctx field could have different | |
| 2232 | + * actual load size with the one encoded in the | |
| 2233 | + * insn. When the dst is PTR, it is for sure not | |
| 2234 | + * a sub-register. | |
| 2235 | + */ | |
| 2236 | + regs[value_regno].subreg_def = DEF_NOT_SUBREG; | |
| 2113 | 2237 | } |
| 2114 | 2238 | regs[value_regno].type = reg_type; |
| 2115 | 2239 | } |
| @@ -2369,7 +2493,8 @@ mark: · 253 unmodified lines | ||
| 2369 | 2493 | * the whole slot to be marked as 'read' |
| 2370 | 2494 | */ |
| 2371 | 2495 | mark_reg_read(env, &state->stack[spi].spilled_ptr, |
| 2372 | − state->stack[spi].spilled_ptr.parent); | |
| 2496 | + state->stack[spi].spilled_ptr.parent, | |
| 2497 | + REG_LIVE_READ64); | |
| 2373 | 2498 | } |
| 2374 | 2499 | return update_stack_depth(env, state, min_off); |
| 2375 | 2500 | } |
| @@ -3333,6 +3458,9 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn · 957 unmodified lines | ||
| 3333 | 3458 | check_reg_arg(env, caller_saved[i], DST_OP_NO_MARK); |
| 3334 | 3459 | } |
| 3335 | 3460 | |
| 3461 | + /* helper call returns 64-bit value. */ | |
| 3462 | + regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG; | |
| 3463 | + | |
| 3336 | 3464 | /* update return register (already marked as written above) */ |
| 3337 | 3465 | if (fn->ret_type == RET_INTEGER) { |
| 3338 | 3466 | /* sets type to SCALAR_VALUE */ |
| @@ -4264,6 +4392,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn) · 925 unmodified lines | ||
| 4264 | 4392 | */ |
| 4265 | 4393 | *dst_reg = *src_reg; |
| 4266 | 4394 | dst_reg->live |= REG_LIVE_WRITTEN; |
| 4395 | + dst_reg->subreg_def = DEF_NOT_SUBREG; | |
| 4267 | 4396 | } else { |
| 4268 | 4397 | /* R1 = (u32) R2 */ |
| 4269 | 4398 | if (is_pointer_value(env, insn->src_reg)) { |
| @@ -4274,6 +4403,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn) · 4 unmodified lines | ||
| 4274 | 4403 | } else if (src_reg->type == SCALAR_VALUE) { |
| 4275 | 4404 | *dst_reg = *src_reg; |
| 4276 | 4405 | dst_reg->live |= REG_LIVE_WRITTEN; |
| 4406 | + dst_reg->subreg_def = env->insn_idx + 1; | |
| 4277 | 4407 | } else { |
| 4278 | 4408 | mark_reg_unknown(env, regs, |
| 4279 | 4409 | insn->dst_reg); |
| @@ -5353,6 +5483,8 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn) · 1073 unmodified lines | ||
| 5353 | 5483 | * Already marked as written above. |
| 5354 | 5484 | */ |
| 5355 | 5485 | mark_reg_unknown(env, regs, BPF_REG_0); |
| 5486 | + /* ld_abs load up to 32-bit skb data. */ | |
| 5487 | + regs[BPF_REG_0].subreg_def = env->insn_idx + 1; | |
| 5356 | 5488 | return 0; |
| 5357 | 5489 | } |
| 5358 | 5490 | |
| @@ -6309,20 +6441,33 @@ static bool states_equal(struct bpf_verifier_env *env, · 950 unmodified lines | ||
| 6309 | 6441 | return true; |
| 6310 | 6442 | } |
| 6311 | 6443 | |
| 6444 | +/* Return 0 if no propagation happened. Return negative error code if error | |
| 6445 | + * happened. Otherwise, return the propagated bit. | |
| 6446 | + */ | |
| 6312 | 6447 | static int propagate_liveness_reg(struct bpf_verifier_env *env, |
| 6313 | 6448 | struct bpf_reg_state *reg, |
| 6314 | 6449 | struct bpf_reg_state *parent_reg) |
| 6315 | 6450 | { |
| 6451 | + u8 parent_flag = parent_reg->live & REG_LIVE_READ; | |
| 6452 | + u8 flag = reg->live & REG_LIVE_READ; | |
| 6316 | 6453 | int err; |
| 6317 | 6454 | |
| 6318 | − if (parent_reg->live & REG_LIVE_READ || !(reg->live & REG_LIVE_READ)) | |
| 6455 | + /* When comes here, read flags of PARENT_REG or REG could be any of | |
| 6456 | + * REG_LIVE_READ64, REG_LIVE_READ32, REG_LIVE_NONE. There is no need | |
| 6457 | + * of propagation if PARENT_REG has strongest REG_LIVE_READ64. | |
| 6458 | + */ | |
| 6459 | + if (parent_flag == REG_LIVE_READ64 || | |
| 6460 | + /* Or if there is no read flag from REG. */ | |
| 6461 | + !flag || | |
| 6462 | + /* Or if the read flag from REG is the same as PARENT_REG. */ | |
| 6463 | + parent_flag == flag) | |
| 6319 | 6464 | return 0; |
| 6320 | 6465 | |
| 6321 | − err = mark_reg_read(env, reg, parent_reg); | |
| 6466 | + err = mark_reg_read(env, reg, parent_reg, flag); | |
| 6322 | 6467 | if (err) |
| 6323 | 6468 | return err; |
| 6324 | 6469 | |
| 6325 | − return 0; | |
| 6470 | + return flag; | |
| 6326 | 6471 | } |
| 6327 | 6472 | |
| 6328 | 6473 | /* A write screens off any subsequent reads; but write marks come from the |
| @@ -6356,8 +6501,10 @@ static int propagate_liveness(struct bpf_verifier_env *env, · 27 unmodified lines | ||
| 6356 | 6501 | for (i = frame < vstate->curframe ? BPF_REG_6 : 0; i < BPF_REG_FP; i++) { |
| 6357 | 6502 | err = propagate_liveness_reg(env, &state_reg[i], |
| 6358 | 6503 | &parent_reg[i]); |
| 6359 | − if (err) | |
| 6504 | + if (err < 0) | |
| 6360 | 6505 | return err; |
| 6506 | + if (err == REG_LIVE_READ64) | |
| 6507 | + mark_insn_zext(env, &parent_reg[i]); | |
| 6361 | 6508 | } |
| 6362 | 6509 | |
| 6363 | 6510 | /* Propagate stack slots. */ |
| @@ -6367,11 +6514,11 @@ static int propagate_liveness(struct bpf_verifier_env *env, · 3 unmodified lines | ||
| 6367 | 6514 | state_reg = &state->stack[i].spilled_ptr; |
| 6368 | 6515 | err = propagate_liveness_reg(env, state_reg, |
| 6369 | 6516 | parent_reg); |
| 6370 | − if (err) | |
| 6517 | + if (err < 0) | |
| 6371 | 6518 | return err; |
| 6372 | 6519 | } |
| 6373 | 6520 | } |
| 6374 | − return err; | |
| 6521 | + return 0; | |
| 6375 | 6522 | } |
| 6376 | 6523 | |
| 6377 | 6524 | static int is_state_visited(struct bpf_verifier_env *env, int insn_idx) |