From 00b456f14ad28eaedf6ce31b962df82a337b0f63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20=C5=A0till?= Date: Tue, 14 Jan 2025 21:22:53 +0100 Subject: [PATCH 1/5] Add test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Vladimír Štill --- testdata/p4_16_errors/issue5094-bmv2.p4 | 38 +++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 testdata/p4_16_errors/issue5094-bmv2.p4 diff --git a/testdata/p4_16_errors/issue5094-bmv2.p4 b/testdata/p4_16_errors/issue5094-bmv2.p4 new file mode 100644 index 00000000000..5f74690e1e2 --- /dev/null +++ b/testdata/p4_16_errors/issue5094-bmv2.p4 @@ -0,0 +1,38 @@ +/* +Copyright 2013-present Barefoot Networks, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +#include +#include + +header hdr { + int<32> a; + bit<8> b; + int<64> c; +} + +#include "../p4_16_samples/arith-skeleton.p4" + +control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) { + action shift() + { h.h.c = (int<64>)(h.h.a >> "aaaa"); sm.egress_spec = 0; } + table t { + actions = { shift; } + const default_action = shift; + } + apply { t.apply(); } +} + +V1Switch(p(), vrfy(), ingress(), egress(), update(), deparser()) main; From 019568bb5552f81e4320ecebccd8c74367028aaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20=C5=A0till?= Date: Wed, 15 Jan 2025 09:10:44 +0100 Subject: [PATCH 2/5] Type check that RHS of shift is integral type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Vladimír Štill --- frontends/p4/typeChecking/typeCheckExpr.cpp | 5 ++++- .../p4_16_errors/{issue5094-bmv2.p4 => issue5094-a-bmv2.p4} | 0 2 files changed, 4 insertions(+), 1 deletion(-) rename testdata/p4_16_errors/{issue5094-bmv2.p4 => issue5094-a-bmv2.p4} (100%) diff --git a/frontends/p4/typeChecking/typeCheckExpr.cpp b/frontends/p4/typeChecking/typeCheckExpr.cpp index 851a5de79bb..ef2155b2c9f 100644 --- a/frontends/p4/typeChecking/typeCheckExpr.cpp +++ b/frontends/p4/typeChecking/typeCheckExpr.cpp @@ -945,9 +945,12 @@ const IR::Node *TypeInferenceBase::shift(const IR::Operation_Binary *expression) } } - if (rtype->is() && rtype->to()->isSigned) { + if (const auto *rbits = rtype->to(); rbits && rbits->isSigned) { typeError("%1%: Shift amount must be an unsigned number", expression->right); return expression; + } else if (!rbits && !rtype->is()) { + typeError("%1%: The right operand of shifts must be either an expression of type bit " + "or int, but is %2%", expression->right, rtype); } if (!lt && !ltype->is()) { diff --git a/testdata/p4_16_errors/issue5094-bmv2.p4 b/testdata/p4_16_errors/issue5094-a-bmv2.p4 similarity index 100% rename from testdata/p4_16_errors/issue5094-bmv2.p4 rename to testdata/p4_16_errors/issue5094-a-bmv2.p4 From d9d4688290bca8d5c85b7148ae362318ae7485bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20=C5=A0till?= Date: Wed, 15 Jan 2025 09:11:29 +0100 Subject: [PATCH 3/5] One more test, add expected outputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Vladimír Štill --- testdata/p4_16_errors/issue5094-b-bmv2.p4 | 38 ++++++++++++ .../p4_16_errors_outputs/issue5094-a-bmv2.p4 | 62 +++++++++++++++++++ .../issue5094-a-bmv2.p4-stderr | 3 + .../p4_16_errors_outputs/issue5094-b-bmv2.p4 | 62 +++++++++++++++++++ .../issue5094-b-bmv2.p4-stderr | 6 ++ 5 files changed, 171 insertions(+) create mode 100644 testdata/p4_16_errors/issue5094-b-bmv2.p4 create mode 100644 testdata/p4_16_errors_outputs/issue5094-a-bmv2.p4 create mode 100644 testdata/p4_16_errors_outputs/issue5094-a-bmv2.p4-stderr create mode 100644 testdata/p4_16_errors_outputs/issue5094-b-bmv2.p4 create mode 100644 testdata/p4_16_errors_outputs/issue5094-b-bmv2.p4-stderr diff --git a/testdata/p4_16_errors/issue5094-b-bmv2.p4 b/testdata/p4_16_errors/issue5094-b-bmv2.p4 new file mode 100644 index 00000000000..2e0a5393ba3 --- /dev/null +++ b/testdata/p4_16_errors/issue5094-b-bmv2.p4 @@ -0,0 +1,38 @@ +/* +Copyright 2013-present Barefoot Networks, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +#include +#include + +header hdr { + int<32> a; + bit<8> b; + int<64> c; +} + +#include "../p4_16_samples/arith-skeleton.p4" + +control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) { + action shift() + { h.h.c = (int<64>)(h.h.a << m); sm.egress_spec = 0; } + table t { + actions = { shift; } + const default_action = shift; + } + apply { t.apply(); } +} + +V1Switch(p(), vrfy(), ingress(), egress(), update(), deparser()) main; diff --git a/testdata/p4_16_errors_outputs/issue5094-a-bmv2.p4 b/testdata/p4_16_errors_outputs/issue5094-a-bmv2.p4 new file mode 100644 index 00000000000..10ccc11a64a --- /dev/null +++ b/testdata/p4_16_errors_outputs/issue5094-a-bmv2.p4 @@ -0,0 +1,62 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +header hdr { + int<32> a; + bit<8> b; + int<64> c; +} + +struct Headers { + hdr h; +} + +struct Meta { +} + +parser p(packet_in b, out Headers h, inout Meta m, inout standard_metadata_t sm) { + state start { + b.extract(h.h); + transition accept; + } +} + +control vrfy(inout Headers h, inout Meta m) { + apply { + } +} + +control update(inout Headers h, inout Meta m) { + apply { + } +} + +control egress(inout Headers h, inout Meta m, inout standard_metadata_t sm) { + apply { + } +} + +control deparser(packet_out b, in Headers h) { + apply { + b.emit(h.h); + } +} + +control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) { + action shift() { + h.h.c = (int<64>)(h.h.a >> "aaaa"); + sm.egress_spec = 0; + } + table t { + actions = { + shift; + } + const default_action = shift; + } + apply { + t.apply(); + } +} + +V1Switch(p(), vrfy(), ingress(), egress(), update(), deparser()) main; diff --git a/testdata/p4_16_errors_outputs/issue5094-a-bmv2.p4-stderr b/testdata/p4_16_errors_outputs/issue5094-a-bmv2.p4-stderr new file mode 100644 index 00000000000..9c105735e3d --- /dev/null +++ b/testdata/p4_16_errors_outputs/issue5094-a-bmv2.p4-stderr @@ -0,0 +1,3 @@ +issue5094-a-bmv2.p4(30): [--Werror=type-error] error: "aaaa": The right operand of shifts must be either an expression of type bit or int, but is string + { h.h.c = (int<64>)(h.h.a >> "aaaa"); sm.egress_spec = 0; } + ^ diff --git a/testdata/p4_16_errors_outputs/issue5094-b-bmv2.p4 b/testdata/p4_16_errors_outputs/issue5094-b-bmv2.p4 new file mode 100644 index 00000000000..a9dab6abc76 --- /dev/null +++ b/testdata/p4_16_errors_outputs/issue5094-b-bmv2.p4 @@ -0,0 +1,62 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +header hdr { + int<32> a; + bit<8> b; + int<64> c; +} + +struct Headers { + hdr h; +} + +struct Meta { +} + +parser p(packet_in b, out Headers h, inout Meta m, inout standard_metadata_t sm) { + state start { + b.extract(h.h); + transition accept; + } +} + +control vrfy(inout Headers h, inout Meta m) { + apply { + } +} + +control update(inout Headers h, inout Meta m) { + apply { + } +} + +control egress(inout Headers h, inout Meta m, inout standard_metadata_t sm) { + apply { + } +} + +control deparser(packet_out b, in Headers h) { + apply { + b.emit(h.h); + } +} + +control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) { + action shift() { + h.h.c = (int<64>)(h.h.a << m); + sm.egress_spec = 0; + } + table t { + actions = { + shift; + } + const default_action = shift; + } + apply { + t.apply(); + } +} + +V1Switch(p(), vrfy(), ingress(), egress(), update(), deparser()) main; diff --git a/testdata/p4_16_errors_outputs/issue5094-b-bmv2.p4-stderr b/testdata/p4_16_errors_outputs/issue5094-b-bmv2.p4-stderr new file mode 100644 index 00000000000..d4fc7724193 --- /dev/null +++ b/testdata/p4_16_errors_outputs/issue5094-b-bmv2.p4-stderr @@ -0,0 +1,6 @@ +issue5094-b-bmv2.p4(30): [--Werror=type-error] error: m: The right operand of shifts must be either an expression of type bit or int, but is struct Meta + { h.h.c = (int<64>)(h.h.a << m); sm.egress_spec = 0; } + ^ +arith-skeleton.p4(26) +struct Meta {} + ^^^^ From 1d3803592202bbb0b99e3db9ba2cc5621a07abec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20=C5=A0till?= Date: Wed, 15 Jan 2025 09:26:18 +0100 Subject: [PATCH 4/5] Fix formatting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Vladimír Štill --- frontends/p4/typeChecking/typeCheckExpr.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frontends/p4/typeChecking/typeCheckExpr.cpp b/frontends/p4/typeChecking/typeCheckExpr.cpp index ef2155b2c9f..9f705cd8846 100644 --- a/frontends/p4/typeChecking/typeCheckExpr.cpp +++ b/frontends/p4/typeChecking/typeCheckExpr.cpp @@ -949,8 +949,10 @@ const IR::Node *TypeInferenceBase::shift(const IR::Operation_Binary *expression) typeError("%1%: Shift amount must be an unsigned number", expression->right); return expression; } else if (!rbits && !rtype->is()) { - typeError("%1%: The right operand of shifts must be either an expression of type bit " - "or int, but is %2%", expression->right, rtype); + typeError( + "%1%: The right operand of shifts must be either an expression of type bit or int, " + "but is %2%", + expression->right, rtype); } if (!lt && !ltype->is()) { From 997f537d09dc387375469ee29d259d515e991b5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20=C5=A0till?= Date: Wed, 15 Jan 2025 12:39:43 +0100 Subject: [PATCH 5/5] Strip `type` wrapper from RHS type for now MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Vladimír Štill --- frontends/p4/typeChecking/typeCheckExpr.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frontends/p4/typeChecking/typeCheckExpr.cpp b/frontends/p4/typeChecking/typeCheckExpr.cpp index 9f705cd8846..b98163273a1 100644 --- a/frontends/p4/typeChecking/typeCheckExpr.cpp +++ b/frontends/p4/typeChecking/typeCheckExpr.cpp @@ -915,6 +915,12 @@ const IR::Node *TypeInferenceBase::shift(const IR::Operation_Binary *expression) // getTypeType should have already taken care of the error message return expression; } + // FIXME: #5100, strip the new-type introduced by `type` keyword for now. According to the spec, + // such code should be rejected, but before #5099 it was accepted, so keep it accepted for now, + // until we discuss what is the right approach here. + if (const auto *rt = rtype->to()) { + rtype = getTypeType(rt->type); + } auto lt = ltype->to(); if (auto cst = expression->right->to()) { if (!cst->fitsInt()) {