Add duplicate action control plane name check pass

control-plane/p4RuntimeSerializer.cpp

@@ -1584,7 +1584,7 @@ void P4RuntimeSerializer::serializeP4RuntimeIfRequired(const IR::P4Program *prog
     std::vector<cstring> files;
     std::vector<P4::P4RuntimeFormat> formats;
 
-    // only generate P4Info is required by use-provided options
+    // only generate P4Info is required by user-provided options
     if (options.p4RuntimeFile.isNullOrEmpty() && options.p4RuntimeFiles.isNullOrEmpty() &&
         options.p4RuntimeEntriesFile.isNullOrEmpty() &&
         options.p4RuntimeEntriesFiles.isNullOrEmpty()) {

frontends/CMakeLists.txt

@@ -33,6 +33,7 @@ set (P4_FRONTEND_SRCS
   p4/frontend.cpp
   p4/functionsInlining.cpp
   p4/hierarchicalNames.cpp
+  p4/duplicateHierarchicalNameCheck.cpp
   p4/inlining.cpp
   p4/localizeActions.cpp
   p4/methodInstance.cpp
@@ -110,6 +111,7 @@ set (P4_FRONTEND_HDRS
   p4/frontend.h
   p4/functionsInlining.h
   p4/hierarchicalNames.h
+  p4/duplicateHierarchicalNameCheck.h
   p4/inlining.h
   p4/localizeActions.h
   p4/methodInstance.h

frontends/p4/duplicateHierarchicalNameCheck.cpp

@@ -0,0 +1,81 @@
+/*
+Copyright 2024 Cisco Systems, 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 "duplicateHierarchicalNameCheck.h"
+
+#include "lib/log.h"
+
+namespace P4 {
+
+cstring DuplicateHierarchicalNameCheck::getName(const IR::IDeclaration *decl) {
+    return decl->getName();
+}
+
+const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::Annotation *annotation) {
+    if (annotation->name != IR::Annotation::nameAnnotation) return annotation;
+
+    cstring name = annotation->getName();
+    if (!name.startsWith(".")) {
+        if (!stack.empty()) {
+            name = absl::StrCat(absl::StrJoin(stack, ".",
+                                              [](std::string *out, cstring s) {
+                                                  absl::StrAppend(out, s.string_view());
+                                              }),
+                                ".", name.string_view());
+        }
+    }
+    // The node the annotation belongs to
+    CHECK_NULL(getContext()->parent);
+    auto *annotatedNode = getContext()->parent->node;
+    CHECK_NULL(annotatedNode);
+    // variable and struct declarations are fine if they have identical
+    // name annotations, and such name annotations can be synthesized by
+    // p4c before this pass.  Ignore them.
+    if (annotatedNode->is<IR::Declaration_Variable>() || annotatedNode->is<IR::Type_Struct>()) {
+        return annotation;
+    }
+    bool foundDuplicate = false;
+    auto *otherNode = annotatedNode;
+    if (annotatedNode->is<IR::P4Table>()) {
+        if (annotatedTables.count(name)) {
+            foundDuplicate = true;
+            otherNode = annotatedTables[name];
+        } else {
+            annotatedTables[name] = annotatedNode;
+        }
+    } else if (annotatedNode->is<IR::P4Action>()) {
+        if (annotatedActions.count(name)) {
+            foundDuplicate = true;
+            otherNode = annotatedActions[name];
+        } else {
+            annotatedActions[name] = annotatedNode;
+        }
+    } else {
+        if (annotatedOthers.count(name)) {
+            foundDuplicate = true;
+            otherNode = annotatedOthers[name];
+        } else {
+            annotatedOthers[name] = annotatedNode;
+        }
+    }
+    if (foundDuplicate) {
+        error(ErrorType::ERR_DUPLICATE, "%1%: conflicting control plane name: %2%", annotatedNode,
+              otherNode);
+    }
+    return annotation;
+}
+
+}  // namespace P4

frontends/p4/duplicateHierarchicalNameCheck.h

@@ -0,0 +1,92 @@
+/*
+Copyright 2024 Cisco Systems, 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.
+*/
+
+#ifndef FRONTENDS_P4_DUPLICATEHIERARCHICALNAMECHECK_H_
+#define FRONTENDS_P4_DUPLICATEHIERARCHICALNAMECHECK_H_
+
+#include "ir/ir.h"
+#include "ir/visitor.h"
+
+namespace P4 {
+
+/**
+ * This pass does not change anything in the IR.  It only gives an
+ * error if it finds two objects with the same hierarchical name.  I
+ * am not certain, but it might be that at this point in the front
+ * end, the only way such a duplicate can happen is due to @name
+ * annotations.  @name annotations are definitely taken into account
+ * in this duplicate check.
+ *
+ * See also the pass HierarchicalNames, on which the implementation of
+ * this pass was based.
+ *
+ * This pass should be run before pass LocalizeAllActions, because
+ * LocalizeAllActions can create actions with duplicate names, by
+ * design, that were not created by the P4 developer.  We should not
+ * issue an error if that pass creates duplicate hierarchical names.
+ */
+class DuplicateHierarchicalNameCheck : public Transform {
+    std::vector<cstring> stack;
+    /// Used for detection of conflicting control plane names among actions.
+    string_map<const IR::Node *> annotatedActions;
+    /// Used for detection of conflicting control plane names among tables.
+    string_map<const IR::Node *> annotatedTables;
+    /// Used for detection of conflicting control plane names among
+    /// objects other than actions and tables.
+    string_map<const IR::Node *> annotatedOthers;
+
+ public:
+    cstring getName(const IR::IDeclaration *decl);
+
+    DuplicateHierarchicalNameCheck() {
+        setName("DuplicateHierarchicalNameCheck");
+        visitDagOnce = false;
+    }
+    const IR::Node *preorder(IR::P4Parser *parser) override {
+        stack.push_back(getName(parser));
+        return parser;
+    }
+    const IR::Node *postorder(IR::P4Parser *parser) override {
+        stack.pop_back();
+        return parser;
+    }
+
+    const IR::Node *preorder(IR::P4Control *control) override {
+        stack.push_back(getName(control));
+        return control;
+    }
+    const IR::Node *postorder(IR::P4Control *control) override {
+        stack.pop_back();
+        return control;
+    }
+
+    const IR::Node *preorder(IR::P4Table *table) override {
+        visit(table->annotations);
+        prune();
+        return table;
+    }
+
+    const IR::Node *postorder(IR::Annotation *annotation) override;
+    // Do not change name annotations on parameters
+    const IR::Node *preorder(IR::Parameter *parameter) override {
+        prune();
+        return parameter;
+    }
+};
+
+}  // namespace P4
+
+#endif /* FRONTENDS_P4_DUPLICATEHIERARCHICALNAMECHECK_H_ */

frontends/p4/frontend.cpp

@@ -35,6 +35,7 @@ limitations under the License.
 #include "deprecated.h"
 #include "directCalls.h"
 #include "dontcareArgs.h"
+#include "duplicateHierarchicalNameCheck.h"
 #include "entryPriorities.h"
 #include "evaluator/evaluator.h"
 #include "frontends/common/constantFolding.h"
@@ -234,6 +235,15 @@ const IR::P4Program *FrontEnd::run(const CompilerOptions &options, const IR::P4P
     if (policy->optimize(options)) {
         passes.addPasses({
             new Inline(&typeMap, *policy, options.optimizeParserInlining),
+        });
+    }
+    if (policy->controlPlaneAPIGenEnabled(options)) {
+        passes.addPasses({
+            new DuplicateHierarchicalNameCheck(),
+        });
+    }
+    if (policy->optimize(options)) {
+        passes.addPasses({
             new InlineActions(&typeMap, *policy),
             new LocalizeAllActions(*policy),
             new UniqueNames(),

frontends/p4/frontend.h

@@ -60,6 +60,18 @@ class FrontEndPolicy : public RemoveUnusedPolicy {
         return options.optimizationLevel > 0;
     }
 
+    /// Indicates whether control plane API generation is enabled.
+    /// @returns default to false unless a command line option was
+    /// given explicitly enabling control plane API generation.
+    virtual bool controlPlaneAPIGenEnabled(const CompilerOptions &options) const {
+        if (options.p4RuntimeFile.isNullOrEmpty() && options.p4RuntimeFiles.isNullOrEmpty() &&
+            options.p4RuntimeEntriesFile.isNullOrEmpty() &&
+            options.p4RuntimeEntriesFiles.isNullOrEmpty()) {
+            return false;
+        }
+        return true;
+    }
+
     /// Get policy for the constant folding pass. @see ConstantFoldingPolicy
     /// @returns Defaults to nullptr, which causes constant folding to use the default policy, which
     /// does not modify the pass defaults in any way.

testdata/p4_16_errors/issue1949-bmv2.p4

@@ -0,0 +1,84 @@
+/*
+Copyright 2019 Cisco Systems, 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 h1_t {
+    bit<8> f1;
+    bit<8> f2;
+}
+
+struct headers_t {
+    h1_t h1;
+}
+
+struct metadata_t {
+}
+
+parser parserImpl(packet_in packet,
+                  out headers_t hdr,
+                  inout metadata_t meta,
+                  inout standard_metadata_t stdmeta)
+{
+    state start {
+        packet.extract(hdr.h1);
+        transition accept;
+    }
+}
+
+control ingressImpl(inout headers_t hdr,
+                    inout metadata_t meta,
+                    inout standard_metadata_t stdmeta)
+{
+    @name(".foo") action act1() {
+        hdr.h1.f1 = hdr.h1.f1 >> 2;
+    }
+    @name(".foo") action act2() {
+        hdr.h1.f1 = hdr.h1.f1 << 3;
+    }
+    table t1 {
+        key = { hdr.h1.f1 : exact; }
+        actions = { act1; act2; NoAction; }
+        const default_action = NoAction;
+    }
+    apply {
+        t1.apply();
+    }
+}
+
+control verifyChecksum(inout headers_t hdr, inout metadata_t meta) {
+    apply {
+    }
+}
+
+control egressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) {
+    apply {
+    }
+}
+
+control updateChecksum(inout headers_t hdr, inout metadata_t meta) {
+    apply {
+    }
+}
+
+control deparserImpl(packet_out packet, in headers_t hdr) {
+    apply {
+    }
+}
+
+V1Switch<headers_t, metadata_t>(parserImpl(), verifyChecksum(), ingressImpl(), egressImpl(), updateChecksum(), deparserImpl()) main;
+