From ef83d4cec6ec02203c8d51e7e5cb1f749ee94bef Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 21 Aug 2019 08:56:44 -0700 Subject: [PATCH 1/7] Fix segfault due to b64 encoding Prior to this patch, bconst.b64 encoded its instruction with a 32-bit immediate that caused improper decoding of the MOV instruction; fixes #911. --- cranelift-codegen/meta/src/isa/x86/encodings.rs | 3 ++- cranelift-codegen/meta/src/isa/x86/recipes.rs | 13 +++++++++++++ filetests/isa/x86/bconst-run.clif | 9 +++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 filetests/isa/x86/bconst-run.clif diff --git a/cranelift-codegen/meta/src/isa/x86/encodings.rs b/cranelift-codegen/meta/src/isa/x86/encodings.rs index c5bbfe1a8..551637585 100644 --- a/cranelift-codegen/meta/src/isa/x86/encodings.rs +++ b/cranelift-codegen/meta/src/isa/x86/encodings.rs @@ -520,6 +520,7 @@ pub fn define( let rec_pu_id_bool = r.template("pu_id_bool"); let rec_pu_id_ref = r.template("pu_id_ref"); let rec_pu_iq = r.template("pu_iq"); + let rec_pu_iq_bool = r.template("pu_iq_bool"); let rec_pushq = r.template("pushq"); let rec_ret = r.template("ret"); let rec_r_ib = r.template("r_ib"); @@ -685,7 +686,7 @@ pub fn define( } e.enc64( bconst.bind(B64), - rec_pu_id_bool.opcodes(vec![0xb8]).rex().w(), + rec_pu_iq_bool.opcodes(vec![0xb8]).rex().w(), ); // Shifts and rotates. diff --git a/cranelift-codegen/meta/src/isa/x86/recipes.rs b/cranelift-codegen/meta/src/isa/x86/recipes.rs index 3e773bc34..308fcd1c1 100644 --- a/cranelift-codegen/meta/src/isa/x86/recipes.rs +++ b/cranelift-codegen/meta/src/isa/x86/recipes.rs @@ -936,6 +936,19 @@ pub fn define<'shared>( ), ); + // XX+rd iq unary with 64-bit immediate. + recipes.add_template_recipe( + EncodingRecipeBuilder::new("pu_iq_bool", f_unary_bool, 8) + .operands_out(vec![gpr]) + .emit( + r#" + {{PUT_OP}}(bits | (out_reg0 & 7), rex1(out_reg0), sink); + let imm: i64 = if imm { 1 } else { 0 }; + sink.put8(imm as u64); + "#, + ), + ); + // XX /n Unary with floating point 32-bit immediate equal to zero. { let format = formats.get(f_unary_ieee32); diff --git a/filetests/isa/x86/bconst-run.clif b/filetests/isa/x86/bconst-run.clif new file mode 100644 index 000000000..63236a954 --- /dev/null +++ b/filetests/isa/x86/bconst-run.clif @@ -0,0 +1,9 @@ +test run + +; this verifies that returning b64 immediates does not result in a segmentation fault, see https://github.com/CraneStation/cranelift/issues/911 +function %test_b64() -> b64 { +ebb0: + v0 = bconst.b64 true + return v0 +} +; run From 56f07338629b8580392266e7aea0220c902a9a79 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 21 Aug 2019 14:16:11 -0700 Subject: [PATCH 2/7] Use 32-bit encoding for bconst.b64 to avoid new recipe --- cranelift-codegen/meta/src/isa/x86/encodings.rs | 6 +----- cranelift-codegen/meta/src/isa/x86/recipes.rs | 13 ------------- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/cranelift-codegen/meta/src/isa/x86/encodings.rs b/cranelift-codegen/meta/src/isa/x86/encodings.rs index 551637585..0cc9565d4 100644 --- a/cranelift-codegen/meta/src/isa/x86/encodings.rs +++ b/cranelift-codegen/meta/src/isa/x86/encodings.rs @@ -520,7 +520,6 @@ pub fn define( let rec_pu_id_bool = r.template("pu_id_bool"); let rec_pu_id_ref = r.template("pu_id_ref"); let rec_pu_iq = r.template("pu_iq"); - let rec_pu_iq_bool = r.template("pu_iq_bool"); let rec_pushq = r.template("pushq"); let rec_ret = r.template("ret"); let rec_r_ib = r.template("r_ib"); @@ -684,10 +683,7 @@ pub fn define( for &ty in &[B1, B8, B16, B32] { e.enc_both(bconst.bind(ty), rec_pu_id_bool.opcodes(vec![0xb8])); } - e.enc64( - bconst.bind(B64), - rec_pu_iq_bool.opcodes(vec![0xb8]).rex().w(), - ); + e.enc64(bconst.bind(B64), rec_pu_id_bool.opcodes(vec![0xb8])); // Shifts and rotates. // Note that the dynamic shift amount is only masked by 5 or 6 bits; the 8-bit diff --git a/cranelift-codegen/meta/src/isa/x86/recipes.rs b/cranelift-codegen/meta/src/isa/x86/recipes.rs index 308fcd1c1..3e773bc34 100644 --- a/cranelift-codegen/meta/src/isa/x86/recipes.rs +++ b/cranelift-codegen/meta/src/isa/x86/recipes.rs @@ -936,19 +936,6 @@ pub fn define<'shared>( ), ); - // XX+rd iq unary with 64-bit immediate. - recipes.add_template_recipe( - EncodingRecipeBuilder::new("pu_iq_bool", f_unary_bool, 8) - .operands_out(vec![gpr]) - .emit( - r#" - {{PUT_OP}}(bits | (out_reg0 & 7), rex1(out_reg0), sink); - let imm: i64 = if imm { 1 } else { 0 }; - sink.put8(imm as u64); - "#, - ), - ); - // XX /n Unary with floating point 32-bit immediate equal to zero. { let format = formats.get(f_unary_ieee32); From 36823370eafe4b28758a9f01f2bd10d46df4a23a Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Thu, 22 Aug 2019 09:34:33 -0700 Subject: [PATCH 3/7] Revert "Use 32-bit encoding for bconst.b64 to avoid new recipe" This reverts commit 56f07338629b8580392266e7aea0220c902a9a79. --- cranelift-codegen/meta/src/isa/x86/encodings.rs | 6 +++++- cranelift-codegen/meta/src/isa/x86/recipes.rs | 13 +++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/cranelift-codegen/meta/src/isa/x86/encodings.rs b/cranelift-codegen/meta/src/isa/x86/encodings.rs index 0cc9565d4..551637585 100644 --- a/cranelift-codegen/meta/src/isa/x86/encodings.rs +++ b/cranelift-codegen/meta/src/isa/x86/encodings.rs @@ -520,6 +520,7 @@ pub fn define( let rec_pu_id_bool = r.template("pu_id_bool"); let rec_pu_id_ref = r.template("pu_id_ref"); let rec_pu_iq = r.template("pu_iq"); + let rec_pu_iq_bool = r.template("pu_iq_bool"); let rec_pushq = r.template("pushq"); let rec_ret = r.template("ret"); let rec_r_ib = r.template("r_ib"); @@ -683,7 +684,10 @@ pub fn define( for &ty in &[B1, B8, B16, B32] { e.enc_both(bconst.bind(ty), rec_pu_id_bool.opcodes(vec![0xb8])); } - e.enc64(bconst.bind(B64), rec_pu_id_bool.opcodes(vec![0xb8])); + e.enc64( + bconst.bind(B64), + rec_pu_iq_bool.opcodes(vec![0xb8]).rex().w(), + ); // Shifts and rotates. // Note that the dynamic shift amount is only masked by 5 or 6 bits; the 8-bit diff --git a/cranelift-codegen/meta/src/isa/x86/recipes.rs b/cranelift-codegen/meta/src/isa/x86/recipes.rs index 3e773bc34..308fcd1c1 100644 --- a/cranelift-codegen/meta/src/isa/x86/recipes.rs +++ b/cranelift-codegen/meta/src/isa/x86/recipes.rs @@ -936,6 +936,19 @@ pub fn define<'shared>( ), ); + // XX+rd iq unary with 64-bit immediate. + recipes.add_template_recipe( + EncodingRecipeBuilder::new("pu_iq_bool", f_unary_bool, 8) + .operands_out(vec![gpr]) + .emit( + r#" + {{PUT_OP}}(bits | (out_reg0 & 7), rex1(out_reg0), sink); + let imm: i64 = if imm { 1 } else { 0 }; + sink.put8(imm as u64); + "#, + ), + ); + // XX /n Unary with floating point 32-bit immediate equal to zero. { let format = formats.get(f_unary_ieee32); From 47fd9725187fa569874e01dae40ed78e2163f5b8 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Thu, 22 Aug 2019 09:43:56 -0700 Subject: [PATCH 4/7] Add 32-bit encoding as an alternate encoding for bconst.b64 --- cranelift-codegen/meta/src/isa/x86/encodings.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cranelift-codegen/meta/src/isa/x86/encodings.rs b/cranelift-codegen/meta/src/isa/x86/encodings.rs index 551637585..789c2fb50 100644 --- a/cranelift-codegen/meta/src/isa/x86/encodings.rs +++ b/cranelift-codegen/meta/src/isa/x86/encodings.rs @@ -688,6 +688,8 @@ pub fn define( bconst.bind(B64), rec_pu_iq_bool.opcodes(vec![0xb8]).rex().w(), ); + // relaxation may replace the REX.W version above with this 32-bit version; it relies on MOV to zero-fill the undefined upper bits + e.enc64(bconst.bind(B64), rec_pu_id_bool.opcodes(vec![0xb8])); // Shifts and rotates. // Note that the dynamic shift amount is only masked by 5 or 6 bits; the 8-bit From cf84e2e60e27e4136af9fcae151aa62cb6c1c9b3 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Thu, 22 Aug 2019 11:06:33 -0700 Subject: [PATCH 5/7] Add binemit test --- filetests/isa/x86/bconst-binemit.clif | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 filetests/isa/x86/bconst-binemit.clif diff --git a/filetests/isa/x86/bconst-binemit.clif b/filetests/isa/x86/bconst-binemit.clif new file mode 100644 index 000000000..6438ba0e9 --- /dev/null +++ b/filetests/isa/x86/bconst-binemit.clif @@ -0,0 +1,8 @@ +test binemit +target x86_64 + +function %test_b64() -> b64 { +ebb0: +[-, %r10] v0 = bconst.b64 true ; bin: 49 ba 0000000000000001 + return v0 +} From d0593816ae7f6411b56a56602169d235ca563879 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Thu, 22 Aug 2019 11:48:18 -0700 Subject: [PATCH 6/7] Use smaller REX.B encoding for bconst.b64 MOV --- cranelift-codegen/meta/src/isa/x86/encodings.rs | 8 +------- cranelift-codegen/meta/src/isa/x86/recipes.rs | 13 ------------- filetests/isa/x86/bconst-binemit.clif | 2 +- filetests/isa/x86/bconst-run.clif | 5 +++-- 4 files changed, 5 insertions(+), 23 deletions(-) diff --git a/cranelift-codegen/meta/src/isa/x86/encodings.rs b/cranelift-codegen/meta/src/isa/x86/encodings.rs index 789c2fb50..a71b2cc68 100644 --- a/cranelift-codegen/meta/src/isa/x86/encodings.rs +++ b/cranelift-codegen/meta/src/isa/x86/encodings.rs @@ -520,7 +520,6 @@ pub fn define( let rec_pu_id_bool = r.template("pu_id_bool"); let rec_pu_id_ref = r.template("pu_id_ref"); let rec_pu_iq = r.template("pu_iq"); - let rec_pu_iq_bool = r.template("pu_iq_bool"); let rec_pushq = r.template("pushq"); let rec_ret = r.template("ret"); let rec_r_ib = r.template("r_ib"); @@ -684,12 +683,7 @@ pub fn define( for &ty in &[B1, B8, B16, B32] { e.enc_both(bconst.bind(ty), rec_pu_id_bool.opcodes(vec![0xb8])); } - e.enc64( - bconst.bind(B64), - rec_pu_iq_bool.opcodes(vec![0xb8]).rex().w(), - ); - // relaxation may replace the REX.W version above with this 32-bit version; it relies on MOV to zero-fill the undefined upper bits - e.enc64(bconst.bind(B64), rec_pu_id_bool.opcodes(vec![0xb8])); + e.enc64(bconst.bind(B64), rec_pu_id_bool.opcodes(vec![0xb8]).rex()); // Shifts and rotates. // Note that the dynamic shift amount is only masked by 5 or 6 bits; the 8-bit diff --git a/cranelift-codegen/meta/src/isa/x86/recipes.rs b/cranelift-codegen/meta/src/isa/x86/recipes.rs index 308fcd1c1..3e773bc34 100644 --- a/cranelift-codegen/meta/src/isa/x86/recipes.rs +++ b/cranelift-codegen/meta/src/isa/x86/recipes.rs @@ -936,19 +936,6 @@ pub fn define<'shared>( ), ); - // XX+rd iq unary with 64-bit immediate. - recipes.add_template_recipe( - EncodingRecipeBuilder::new("pu_iq_bool", f_unary_bool, 8) - .operands_out(vec![gpr]) - .emit( - r#" - {{PUT_OP}}(bits | (out_reg0 & 7), rex1(out_reg0), sink); - let imm: i64 = if imm { 1 } else { 0 }; - sink.put8(imm as u64); - "#, - ), - ); - // XX /n Unary with floating point 32-bit immediate equal to zero. { let format = formats.get(f_unary_ieee32); diff --git a/filetests/isa/x86/bconst-binemit.clif b/filetests/isa/x86/bconst-binemit.clif index 6438ba0e9..55974e564 100644 --- a/filetests/isa/x86/bconst-binemit.clif +++ b/filetests/isa/x86/bconst-binemit.clif @@ -3,6 +3,6 @@ target x86_64 function %test_b64() -> b64 { ebb0: -[-, %r10] v0 = bconst.b64 true ; bin: 49 ba 0000000000000001 +[-, %r10] v0 = bconst.b64 true ; bin: 41 ba 00000001 return v0 } diff --git a/filetests/isa/x86/bconst-run.clif b/filetests/isa/x86/bconst-run.clif index 63236a954..b255770c1 100644 --- a/filetests/isa/x86/bconst-run.clif +++ b/filetests/isa/x86/bconst-run.clif @@ -1,9 +1,10 @@ test run +target x86_64 ; this verifies that returning b64 immediates does not result in a segmentation fault, see https://github.com/CraneStation/cranelift/issues/911 function %test_b64() -> b64 { ebb0: - v0 = bconst.b64 true - return v0 +[-, %r10] v0 = bconst.b64 true + return v0 } ; run From 484bf395632bcaacf3e0273a4c26bfd6d7a8fb5b Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Fri, 23 Aug 2019 08:32:41 -0700 Subject: [PATCH 7/7] Rename tests --- filetests/isa/x86/bconst-binemit.clif | 8 -------- filetests/isa/x86/{bconst-run.clif => binary64-run.clif} | 0 filetests/isa/x86/binary64.clif | 7 +++++++ 3 files changed, 7 insertions(+), 8 deletions(-) delete mode 100644 filetests/isa/x86/bconst-binemit.clif rename filetests/isa/x86/{bconst-run.clif => binary64-run.clif} (100%) diff --git a/filetests/isa/x86/bconst-binemit.clif b/filetests/isa/x86/bconst-binemit.clif deleted file mode 100644 index 55974e564..000000000 --- a/filetests/isa/x86/bconst-binemit.clif +++ /dev/null @@ -1,8 +0,0 @@ -test binemit -target x86_64 - -function %test_b64() -> b64 { -ebb0: -[-, %r10] v0 = bconst.b64 true ; bin: 41 ba 00000001 - return v0 -} diff --git a/filetests/isa/x86/bconst-run.clif b/filetests/isa/x86/binary64-run.clif similarity index 100% rename from filetests/isa/x86/bconst-run.clif rename to filetests/isa/x86/binary64-run.clif diff --git a/filetests/isa/x86/binary64.clif b/filetests/isa/x86/binary64.clif index a65b3d3d1..7742a24ee 100644 --- a/filetests/isa/x86/binary64.clif +++ b/filetests/isa/x86/binary64.clif @@ -1642,3 +1642,10 @@ ebb0: return } + +function %B64() { +ebb0: + [-, %rax] v1 = bconst.b64 true ; bin: 40 b8 00000001 + [-, %r10] v0 = bconst.b64 true ; bin: 41 ba 00000001 + return +}