Cranelift: x64: fix user-controlled recursion in cmp emission. (#12333) (#12338)

* Cranelift: x64: fix user-controlled recursion in cmp emission.

We had a set of rules introduced in #11097 that attempted to optimize
the case of testing the result of an `icmp` for a nonzero value. This
allowed optimization of, for example, `(((x == 0) == 0) == 0 ...)` to
a single level, either `x == 0` or `x != 0` depending on even/odd
nesting depth.

Unfortunately this kind of recursion in the backend has a depth
bounded only by the user input, hence creates a DoS vulnerability: the
wrong kind of compiler input can cause a stack overflow in Cranelift
at compilation time. This case is reachable from Wasmtime's Wasm
frontend via the `i32.eqz` operator (for example) as well.

Ideally, this kind of deep rewrite is best done in our mid-end
optimizer, where we think carefully about bounds for recursive
rewrites. The left-hand sides for the backend rules should really be
fixed shapes that correspond to machine instructions, rather than
ad-hoc peephole optimizations in their own right.

This fix thus simply removes the recursion case that causes the
blowup. The patch includes two tests: one with optimizations disabled,
showing correct compilation (without the fix, this case fails to
compile with a stack overflow), and one with optimizations enabled,
showing that the mid-end properly cleans up the nested expression and
we get the expected one-level result anyway.

* Preserve codegen on branches.

This change works by splitting a rule so that the entry point used by
`brif` lowering can still peel off one layer of `icmp` and emit it
directly, without entering the unbounded structural recursion.

It also adds a mid-end rule to catch one case that we were previously
catching in the backend only: `fcmp(...) != 0`.

Author: cfallin

cranelift/codegen/src/isa/x64/inst.isle

@@ -3403,14 +3403,14 @@
 ;; Note that this is used as the base entry case for instruction lowering such
 ;; as `select` and `brif`. The `Value` here is expected to, via CLIF validation,
 ;; have an integer type (and it can be I128)
-(decl is_nonzero_cmp (Value) CondResult)
+(decl is_nonzero (Value) CondResult)
 
 ;; Base case: fits in one GPR, use `x64_test`
-(rule (is_nonzero_cmp val @ (value_type (is_single_register_gpr_type ty)))
+(rule (is_nonzero val @ (value_type (is_single_register_gpr_type ty)))
   (let ((gpr Gpr val)) (CondResult.CC (x64_test ty gpr gpr) (CC.NZ))))
 
 ;; Base case: i128
-(rule 1 (is_nonzero_cmp val @ (value_type $I128))
+(rule 1 (is_nonzero val @ (value_type $I128))
       (let ((lo Gpr (value_regs_get_gpr val 0))
             (hi Gpr (value_regs_get_gpr val 1)))
           (CondResult.CC
@@ -3419,13 +3419,24 @@
 
 ;; Special case some instructions where lowerings directly produce condition
 ;; codes.
-(rule 2 (is_nonzero_cmp (fcmp cc a b)) (emit_fcmp cc a b))
-(rule 2 (is_nonzero_cmp (icmp cc a b)) (emit_cmp cc a b))
-(rule 2 (is_nonzero_cmp (vall_true vec)) (is_vall_true vec))
-(rule 2 (is_nonzero_cmp (vany_true vec)) (is_vany_true vec))
-(rule 2 (is_nonzero_cmp (uextend val)) (is_nonzero_cmp val))
-(rule 2 (is_nonzero_cmp (band a @ (value_type (ty_int (fits_in_64 ty))) b))
-  (is_nonzero_band ty a b))
+(rule 2 (is_nonzero (vall_true vec)) (is_vall_true vec))
+(rule 2 (is_nonzero (vany_true vec)) (is_vany_true vec))
+(rule 2 (is_nonzero (uextend (vall_true vec))) (is_vall_true vec))
+(rule 2 (is_nonzero (uextend (vany_true vec))) (is_vany_true vec))
+(rule 2 (is_nonzero (band a @ (value_type (ty_int (fits_in_64 ty))) b))
+      (is_nonzero_band ty a b))
+
+
+;; Like `is_nonzero` but with additional specializations for compare
+;; operators. We break this out from `is_nonzero` because we want to
+;; avoid unbounded recursion.
+(decl is_nonzero_cmp (Value) CondResult)
+
+(rule 1 (is_nonzero_cmp (fcmp cc a b)) (emit_fcmp cc a b))
+(rule 1 (is_nonzero_cmp (icmp cc a b)) (emit_cmp cc a b))
+(rule 1 (is_nonzero_cmp (uextend (fcmp cc a b))) (emit_fcmp cc a b))
+(rule 1 (is_nonzero_cmp (uextend (icmp cc a b))) (emit_cmp cc a b))
+(rule 0 (is_nonzero_cmp val) (is_nonzero val))
 
 (decl is_nonzero_band (Type Value Value) CondResult)
 (rule 0 (is_nonzero_band ty a b) (CondResult.CC (x64_test ty a b) (CC.NZ)))
@@ -3510,7 +3521,14 @@
             (a_hi Gpr (value_regs_get_gpr a 1))
             (b_lo Gpr (value_regs_get_gpr b 0))
             (b_hi Gpr (value_regs_get_gpr b 1)))
-      (emit_cmp_i128 cc a_hi a_lo b_hi b_lo)))
+        (emit_cmp_i128 cc a_hi a_lo b_hi b_lo)))
+
+;; For direct equality comparisons to zero transform the other operand into a
+;; nonzero comparison and then invert the whole conditional to test for zero.
+(rule 5 (emit_cmp (IntCC.Equal) a (u64_from_iconst 0)) (cond_invert (is_nonzero a)))
+(rule 6 (emit_cmp (IntCC.Equal) (u64_from_iconst 0) a) (cond_invert (is_nonzero a)))
+(rule 5 (emit_cmp (IntCC.NotEqual) a (u64_from_iconst 0)) (is_nonzero a))
+(rule 6 (emit_cmp (IntCC.NotEqual) (u64_from_iconst 0) a) (is_nonzero a))
 
 (decl emit_cmp_i128 (CC Gpr Gpr Gpr Gpr) CondResult)
 ;; Eliminate cases which compare something "or equal" by swapping arguments.
@@ -3523,13 +3541,6 @@
 (rule 2 (emit_cmp_i128 (CC.BE)  a_hi a_lo b_hi b_lo)
         (emit_cmp_i128 (CC.NB)  b_hi b_lo a_hi a_lo))
 
-;; For direct equality comparisons to zero transform the other operand into a
-;; nonzero comparison and then invert the whole conditional to test for zero.
-(rule 5 (emit_cmp (IntCC.Equal) a (u64_from_iconst 0)) (cond_invert (is_nonzero_cmp a)))
-(rule 6 (emit_cmp (IntCC.Equal) (u64_from_iconst 0) a) (cond_invert (is_nonzero_cmp a)))
-(rule 5 (emit_cmp (IntCC.NotEqual) a (u64_from_iconst 0)) (is_nonzero_cmp a))
-(rule 6 (emit_cmp (IntCC.NotEqual) (u64_from_iconst 0) a) (is_nonzero_cmp a))
-
 ;; 128-bit strict equality/inequality can't be easily tested using subtraction
 ;; but we can quickly determine whether any bits are different instead.
 (rule 1 (emit_cmp_i128 (cc_nz_or_z cc) a_hi a_lo b_hi b_lo)

cranelift/codegen/src/opts/icmp.isle

@@ -51,6 +51,13 @@
                       (iconst_u _ 0)))
       (subsume inner))
 
+;; Likewise for icmp-of-fcmp.
+;; ne(fcmp(ty, cc, x, y), 0) == fcmp(ty, cc, x, y)
+(rule (simplify (ne ty
+                      (uextend_maybe _ inner @ (fcmp ty _ _ _))
+                      (iconst_u _ 0)))
+      (subsume inner))
+
 ;; eq(icmp(ty, cc, x, y), 0) == icmp(ty, cc_complement, x, y)
 ;; e.g. eq(ugt(x, y), 0) == ule(x, y)
 (rule (simplify (eq ty