[P4Testgen] Set other headers also invalid when calling setInvalid on a union header.

backends/bmv2/pna_nic/midend.cpp

@@ -130,7 +130,7 @@ PnaNicMidEnd::PnaNicMidEnd(CompilerOptions &options, std::ostream *outStream)
             new P4::StrengthReduction(&typeMap),
             new P4::EliminateTuples(&typeMap),
             new P4::SimplifyComparisons(&typeMap),
-            new P4::CopyStructures(&typeMap),
+            new P4::CopyStructures(&typeMap, P4::CopyStructuresConfig()),
             new P4::NestedStructs(&typeMap),
             new P4::SimplifySelectList(&typeMap),
             new P4::RemoveSelectBooleans(&typeMap),

backends/bmv2/psa_switch/midend.cpp

@@ -131,7 +131,7 @@ PsaSwitchMidEnd::PsaSwitchMidEnd(CompilerOptions &options, std::ostream *outStre
             new P4::StrengthReduction(&typeMap),
             new P4::EliminateTuples(&typeMap),
             new P4::SimplifyComparisons(&typeMap),
-            new P4::CopyStructures(&typeMap),
+            new P4::CopyStructures(&typeMap, P4::CopyStructuresConfig()),
             new P4::NestedStructs(&typeMap),
             new P4::SimplifySelectList(&typeMap),
             new P4::RemoveSelectBooleans(&typeMap),

backends/bmv2/simple_switch/midend.cpp

@@ -98,7 +98,7 @@ SimpleSwitchMidEnd::SimpleSwitchMidEnd(CompilerOptions &options, std::ostream *o
              new P4::StrengthReduction(&typeMap),
              new P4::EliminateTuples(&typeMap),
              new P4::SimplifyComparisons(&typeMap),
-             new P4::CopyStructures(&typeMap),
+             new P4::CopyStructures(&typeMap, P4::CopyStructuresConfig()),
              new P4::NestedStructs(&typeMap),
              new P4::SimplifySelectList(&typeMap),
              new P4::RemoveSelectBooleans(&typeMap),

backends/dpdk/midend.cpp

@@ -194,7 +194,11 @@ DpdkMidEnd::DpdkMidEnd(CompilerOptions &options, std::ostream *outStream) {
             new P4::StrengthReduction(&typeMap),
             new P4::EliminateTuples(&typeMap),
             new P4::SimplifyComparisons(&typeMap),
-            new P4::CopyStructures(&typeMap, false /* errorOnMethodCall */),
+            new P4::CopyStructures(&typeMap, P4::CopyStructuresConfig{
+                                                 /*errorOnMethodCall*/ false,
+                                                 /*copyHeaders*/ false,
+                                                 /*expandUnions*/ true,
+                                             }),
             new P4::NestedStructs(&typeMap),
             new P4::SimplifySelectList(&typeMap),
             new P4::RemoveSelectBooleans(&typeMap),

backends/p4test/midend.cpp

@@ -102,7 +102,11 @@ MidEnd::MidEnd(CompilerOptions &options, std::ostream *outStream) {
          new P4::StrengthReduction(&typeMap),
          new P4::EliminateTuples(&typeMap),
          new P4::SimplifyComparisons(&typeMap),
-         new P4::CopyStructures(&typeMap, false),
+         new P4::CopyStructures(&typeMap, P4::CopyStructuresConfig{
+                                              /*errorOnMethodCall*/ false,
+                                              /*copyHeaders*/ false,
+                                              /*expandUnions*/ true,
+                                          }),
          new P4::NestedStructs(&typeMap),
          new P4::StrengthReduction(&typeMap),
          new P4::SimplifySelectList(&typeMap),

backends/p4tools/common/compiler/midend.cpp

@@ -121,7 +121,13 @@ void MidEnd::addDefaultPasses() {
         new P4::SimplifyComparisons(&typeMap),
         // Expand header and struct assignments into sequences of field assignments.
         new PassRepeated({
-            new P4::CopyStructures(&typeMap, false, true, nullptr),
+            new P4::CopyStructures(&typeMap,
+                                   {
+                                       /*errorOnMethodCall*/ false,
+                                       /*copyHeaders*/ true,
+                                       /*expandUnions*/ false,
+                                   },
+                                   nullptr),
         }),
         new P4::RemoveParserControlFlow(&typeMap),
         // Flatten nested list expressions.

backends/p4tools/modules/testgen/core/small_step/abstract_stepper.cpp

@@ -210,38 +210,48 @@ bool AbstractStepper::stepGetHeaderValidity(const IR::StateVariable &headerRef)
     return false;
 }
 
-void AbstractStepper::setHeaderValidity(const IR::StateVariable &headerRef, bool validity,
-                                        ExecutionState &nextState) {
+namespace {
+
+void setHeaderValidityHelper(const IR::StateVariable &headerRef, bool validity,
+                             const ProgramInfo &programInfo, ExecutionState &nextState) {
     const auto &headerRefValidity = ToolsVariables::getHeaderValidity(headerRef);
     nextState.set(headerRefValidity, IR::BoolLiteral::get(validity));
 
-    // In some cases, the header may be `part of a union.
-    if (validity) {
-        const auto *headerBaseMember = headerRef.ref->to<IR::Member>();
-        if (headerBaseMember == nullptr) {
-            return;
+    // The header is going to be invalid. Set all fields to taint constants.
+    // TODO: Should we make this target specific? Some targets set the header fields to 0...
+    if (!validity) {
+        std::vector<IR::StateVariable> validityVector;
+        auto fieldsVector = nextState.getFlatFields(headerRef, &validityVector);
+        for (const auto &field : validityVector) {
+            nextState.set(field, IR::BoolLiteral::get(false));
+        }
+        for (const auto &field : fieldsVector) {
+            nextState.set(field, programInfo.createTargetUninitialized(field->type, true));
         }
-        const auto *headerBase = headerBaseMember->expr;
+    }
+}
+
+}  // namespace
+
+void AbstractStepper::setHeaderValidity(const IR::StateVariable &headerRef, bool validity,
+                                        ExecutionState &nextState) {
+    setHeaderValidityHelper(headerRef, validity, programInfo, nextState);
+
+    // In some cases, the header may be nested.
+    if (const auto *headerBase = headerRef.ref->to<IR::Member>()) {
         // In the case of header unions, we need to set all other union members invalid.
-        if (const auto *hdrUnion = headerBase->type->to<IR::Type_HeaderUnion>()) {
+        CHECK_NULL(headerBase->expr->type);
+        if (const auto *hdrUnion = headerBase->expr->type->to<IR::Type_HeaderUnion>()) {
             for (const auto *field : hdrUnion->fields) {
-                auto *member = new IR::Member(field->type, headerBase, field->name);
-                // Ignore the member we are setting to valid.
-                if (headerRef->equiv(*member)) {
+                // Ignore the member we are setting.
+                if (headerBase->member == field->name) {
                     continue;
                 }
                 // Set all other members to invalid.
-                setHeaderValidity(member, false, nextState);
+                setHeaderValidityHelper(new IR::Member(field->type, headerBase->expr, field->name),
+                                        false, programInfo, nextState);
             }
         }
-        return;
-    }
-    std::vector<IR::StateVariable> validityVector;
-    auto fieldsVector = nextState.getFlatFields(headerRef, &validityVector);
-    // The header is going to be invalid. Set all fields to taint constants.
-    // TODO: Should we make this target specific? Some targets set the header fields to 0.
-    for (const auto &field : fieldsVector) {
-        nextState.set(field, programInfo.createTargetUninitialized(field->type, true));
     }
 }
 

backends/p4tools/modules/testgen/targets/bmv2/test/BMV2PTFXfail.cmake

@@ -77,6 +77,15 @@ p4tools_add_xfail_reason(
   v1model-digest-custom-type.p4
 )
 
+p4tools_add_xfail_reason(
+  "testgen-p4c-bmv2-ptf"
+  "Expected packet was not received on device"
+  # BMv2 has the incorrect HU invalidation implementation.
+  bmv2_invalid_hu_1.p4
+  bmv2_invalid_hu_2.p4
+  union4-bmv2.p4
+)
+
 ####################################################################################################
 # 2. P4Testgen Issues
 # These are failures in P4Testgen that need to be fixed.

backends/p4tools/modules/testgen/targets/bmv2/test/BMV2STFXfail.cmake

@@ -62,6 +62,15 @@ p4tools_add_xfail_reason(
   extract_for_header_union.p4
 )
 
+p4tools_add_xfail_reason(
+  "testgen-p4c-bmv2-stf"
+  "differs|Expected ([0-9]+) packets on port ([0-9]+) got ([0-9]+)"
+  # BMv2 has the incorrect HU invalidation implementation.
+  bmv2_invalid_hu_1.p4
+  bmv2_invalid_hu_2.p4
+  union4-bmv2.p4
+)
+
 ####################################################################################################
 # 2. P4Testgen Issues
 # These are failures in P4Testgen that need to be fixed.

backends/p4tools/modules/testgen/targets/bmv2/test/p4-programs/bmv2_invalid_hu_1.p4

@@ -0,0 +1,76 @@
+#include <v1model.p4>
+
+header opt_t {
+    bit<8> opt;
+}
+
+header ethernet_t {
+    bit<48> dst_addr;
+    bit<48> src_addr;
+    bit<16> eth_type;
+}
+
+header H {
+    bit<8> a;
+    bit<8> a1;
+    bit<8> b;
+}
+
+header_union U {
+    ethernet_t e;
+    H h;
+}
+
+struct Headers {
+    opt_t opt;
+    U u;
+}
+
+struct Meta {}
+
+parser p(packet_in pkt, out Headers h, inout Meta meta, inout standard_metadata_t stdmeta) {
+    state start {
+        pkt.extract(h.opt);
+        transition select(h.opt.opt) {
+            0x0: parse_e;
+            0x1: parse_h;
+            default: accept;
+        }
+    }
+
+    state parse_e {
+        pkt.extract(h.u.e);
+        transition accept;
+    }
+
+    state parse_h {
+        pkt.extract(h.u.h);
+        transition accept;
+    }
+}
+
+control vrfy(inout Headers h, inout Meta meta) {
+    apply { }
+}
+
+control ingress(inout Headers h, inout Meta m, inout standard_metadata_t s) {
+    apply {
+        h.u.e.setInvalid();
+    }
+}
+
+control egress(inout Headers h, inout Meta m, inout standard_metadata_t s) {
+    apply { }
+}
+
+control update(inout Headers h, inout Meta m) {
+    apply { }
+}
+
+control deparser(packet_out pkt, in Headers h) {
+    apply {
+        pkt.emit(h);
+    }
+}
+
+V1Switch(p(), vrfy(), ingress(), egress(), update(), deparser()) main;

backends/p4tools/modules/testgen/targets/bmv2/test/p4-programs/bmv2_invalid_hu_2.p4

@@ -0,0 +1,80 @@
+#include <v1model.p4>
+
+header opt_t {
+    bit<8> opt;
+}
+
+header ethernet_t {
+    bit<48> dst_addr;
+    bit<48> src_addr;
+    bit<16> eth_type;
+}
+
+header H {
+    bit<8> a;
+    bit<8> a1;
+    bit<8> b;
+}
+
+header_union U {
+    ethernet_t e;
+    H h;
+}
+
+struct Headers {
+    opt_t opt;
+    U u;
+}
+
+struct Meta {}
+
+parser p(packet_in pkt, out Headers h, inout Meta meta, inout standard_metadata_t stdmeta) {
+    state start {
+        pkt.extract(h.opt);
+        transition select(h.opt.opt) {
+            0x0: parse_e;
+            0x1: parse_h;
+            default: accept;
+        }
+    }
+
+    state parse_e {
+        pkt.extract(h.u.e);
+        transition invalidate;
+    }
+
+    state parse_h {
+        pkt.extract(h.u.h);
+        transition invalidate;
+    }
+
+    state invalidate {
+        h.u.e.setInvalid();
+        transition accept;
+    }
+}
+
+control vrfy(inout Headers h, inout Meta meta) {
+    apply { }
+}
+
+control ingress(inout Headers h, inout Meta m, inout standard_metadata_t s) {
+    apply {
+    }
+}
+
+control egress(inout Headers h, inout Meta m, inout standard_metadata_t s) {
+    apply { }
+}
+
+control update(inout Headers h, inout Meta m) {
+    apply { }
+}
+
+control deparser(packet_out pkt, in Headers h) {
+    apply {
+        pkt.emit(h);
+    }
+}
+
+V1Switch(p(), vrfy(), ingress(), egress(), update(), deparser()) main;

backends/ubpf/midend.cpp

@@ -93,7 +93,7 @@ const IR::ToplevelBlock *MidEnd::run(EbpfOptions &options, const IR::P4Program *
                 new P4::StrengthReduction(&typeMap),
             }),
             new P4::SimplifyComparisons(&typeMap),
-            new P4::CopyStructures(&typeMap),
+            new P4::CopyStructures(&typeMap, P4::CopyStructuresConfig()),
             new P4::LocalCopyPropagation(&typeMap),
             new P4::SimplifySelectList(&typeMap),
             new P4::MoveDeclarations(),  // more may have been introduced

midend/copyStructures.cpp

@@ -73,13 +73,14 @@ const IR::Node *DoCopyStructures::postorder(IR::AssignmentStatement *statement)
        FIXME: this is not correct for header unions and should be fixed.
        The fix bellow, commented-out, causes problems elsewhere.
        https://github.com/p4lang/p4c/issues/3842
-    if (ltype->is<IR::Type_HeaderUnion>())
-        return statement;
     */
+    if (!_config.expandUnions && ltype->is<IR::Type_HeaderUnion>()) {
+        return statement;
+    }
 
     // Do not copy structures for method calls.
     if (statement->right->is<IR::MethodCallExpression>()) {
-        if (errorOnMethodCall) {
+        if (_config.errorOnMethodCall) {
             ::P4::error(ErrorType::ERR_UNSUPPORTED_ON_TARGET,
                         "%1%: functions or methods returning structures "
                         "are not supported on this target",
@@ -98,7 +99,7 @@ const IR::Node *DoCopyStructures::postorder(IR::AssignmentStatement *statement)
             retval->push_back(
                 new IR::AssignmentStatement(statement->srcInfo, left, right->expression));
         }
-    } else if (copyHeaders && ltype->is<IR::Type_Header>()) {
+    } else if (_config.copyHeaders && ltype->is<IR::Type_Header>()) {
         const auto *header = ltype->checkedTo<IR::Type_Header>();
         // Build a "src.isValid()" call.
         const auto *isSrcValidCall = new IR::MethodCallExpression(