Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type check that RHS of shift is integral type #5099

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion frontends/p4/typeChecking/typeCheckExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<IR::Type_Newtype>()) {
rtype = getTypeType(rt->type);
}
auto lt = ltype->to<IR::Type_Bits>();
if (auto cst = expression->right->to<IR::Constant>()) {
if (!cst->fitsInt()) {
Expand Down Expand Up @@ -945,9 +951,14 @@ const IR::Node *TypeInferenceBase::shift(const IR::Operation_Binary *expression)
}
}

if (rtype->is<IR::Type_Bits>() && rtype->to<IR::Type_Bits>()->isSigned) {
if (const auto *rbits = rtype->to<IR::Type_Bits>(); rbits && rbits->isSigned) {
typeError("%1%: Shift amount must be an unsigned number", expression->right);
return expression;
} else if (!rbits && !rtype->is<IR::Type_InfInt>()) {
typeError(
"%1%: The right operand of shifts must be either an expression of type bit<S> or int, "
"but is %2%",
expression->right, rtype);
}

if (!lt && !ltype->is<IR::Type_InfInt>()) {
Expand Down
38 changes: 38 additions & 0 deletions testdata/p4_16_errors/issue5094-a-bmv2.p4
Original file line number Diff line number Diff line change
@@ -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 <core.p4>
#include <v1model.p4>

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;
38 changes: 38 additions & 0 deletions testdata/p4_16_errors/issue5094-b-bmv2.p4
Original file line number Diff line number Diff line change
@@ -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 <core.p4>
#include <v1model.p4>

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;
62 changes: 62 additions & 0 deletions testdata/p4_16_errors_outputs/issue5094-a-bmv2.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
#include <core.p4>
#define V1MODEL_VERSION 20180101
#include <v1model.p4>

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;
3 changes: 3 additions & 0 deletions testdata/p4_16_errors_outputs/issue5094-a-bmv2.p4-stderr
Original file line number Diff line number Diff line change
@@ -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<S> or int, but is string
{ h.h.c = (int<64>)(h.h.a >> "aaaa"); sm.egress_spec = 0; }
^
62 changes: 62 additions & 0 deletions testdata/p4_16_errors_outputs/issue5094-b-bmv2.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
#include <core.p4>
#define V1MODEL_VERSION 20180101
#include <v1model.p4>

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;
6 changes: 6 additions & 0 deletions testdata/p4_16_errors_outputs/issue5094-b-bmv2.p4-stderr
Original file line number Diff line number Diff line change
@@ -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<S> 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 {}
^^^^
Loading