Fix some issues found by static analysis (#4213)

* Fix use of end() iterator
* Fix GlobalRef::validate
* testgen-ebpf: Avoid overwritting short output packet
* Add initializers
* Fix comment
* Avoid null dereference in Visitor::findContext for visitor without context

backends/p4tools/modules/testgen/core/program_info.h

@@ -53,7 +53,7 @@ class ProgramInfo : public ICastable {
     const IR::P4Program *program;
 
     /// The generated dcg.
-    const NodesCallGraph *dcg;
+    const NodesCallGraph *dcg = nullptr;
 
     /// @returns the series of nodes that has been computed by this particular target.
     [[nodiscard]] const std::vector<Continuation::Command> *getPipelineSequence() const;

backends/p4tools/modules/testgen/core/symbolic_executor/selected_branches.cpp

@@ -48,7 +48,7 @@ void SelectedBranches::runImpl(const Callback &callBack, ExecutionStateReference
             executionState = *next;
         }
         if (executionState.get().isTerminal()) {
-            // We've reached the end of the program. Call back and (if desired) end execution.
+            // We've reached the end of the program. Call back and end execution.
             handleTerminalState(callBack, executionState);
             if (!selectedBranches.empty()) {
                 ::warning(

backends/p4tools/modules/testgen/lib/execution_state.cpp

@@ -223,9 +223,8 @@ TestObjectMap ExecutionState::getTestObjectCategory(cstring category) const {
 void ExecutionState::deleteTestObject(cstring category, cstring objectLabel) {
     auto it = testObjects.find(category);
     if (it != testObjects.end()) {
-        return;
+        it->second.erase(objectLabel);
     }
-    it->second.erase(objectLabel);
 }
 
 void ExecutionState::deleteTestObjectCategory(cstring category) { testObjects.erase(category); }

backends/p4tools/modules/testgen/targets/ebpf/test_backend.cpp

@@ -66,12 +66,13 @@ TestBackEnd::TestInfo EBPFTestBackend::produceTestInfo(
             IR::getConstant(IR::getBitType(outPktSize), EBPFTestBackend::ZERO_PKT_VAL);
         testInfo.packetTaintMask =
             IR::getConstant(IR::getBitType(outPktSize), EBPFTestBackend::ZERO_PKT_MAX);
+    } else {
+        // eBPF actually can not modify the input packet. It can only filter. Thus we reuse our
+        // input packet here.
+        testInfo.outputPacket = testInfo.inputPacket;
+        testInfo.packetTaintMask = IR::getConstant(testInfo.inputPacket->type,
+                                                   IR::getMaxBvVal(testInfo.inputPacket->type));
     }
-    // eBPF actually can not modify the input packet. It can only filter. Thus we reuse our input
-    // packet here.
-    testInfo.outputPacket = testInfo.inputPacket;
-    testInfo.packetTaintMask =
-        IR::getConstant(testInfo.inputPacket->type, IR::getMaxBvVal(testInfo.inputPacket->type));
     return testInfo;
 }
 

frontends/p4/simplifyDefUse.cpp

@@ -365,15 +365,15 @@ class FindUninitialized : public Inspector {
     ReferenceMap *refMap;
     TypeMap *typeMap;
     AllDefinitions *definitions;
-    bool lhs;                   // checking the lhs of an assignment
+    bool lhs = false;           // checking the lhs of an assignment
     ProgramPoint currentPoint;  // context of the current expression/statement
     /// For some simple expresssions keep here the read location sets.
     /// This does not include location sets read by subexpressions.
     std::map<const IR::Expression *, const LocationSet *> readLocations;
     HasUses *hasUses;  // output
     /// If true the current statement is unreachable
-    bool unreachable;
-    bool virtualMethod;
+    bool unreachable = false;
+    bool virtualMethod = false;
 
     HeaderDefinitions *headerDefs;
     bool reportInvalidHeaders = true;
@@ -407,10 +407,8 @@ class FindUninitialized : public Inspector {
           refMap(parent->definitions->storageMap->refMap),
           typeMap(parent->definitions->storageMap->typeMap),
           definitions(parent->definitions),
-          lhs(false),
           currentPoint(context),
           hasUses(parent->hasUses),
-          virtualMethod(false),
           headerDefs(parent->headerDefs),
           reportInvalidHeaders(parent->reportInvalidHeaders) {
         visitDagOnce = false;
@@ -421,10 +419,8 @@ class FindUninitialized : public Inspector {
         : refMap(definitions->storageMap->refMap),
           typeMap(definitions->storageMap->typeMap),
           definitions(definitions),
-          lhs(false),
           currentPoint(),
           hasUses(hasUses),
-          virtualMethod(false),
           headerDefs(new HeaderDefinitions(refMap, typeMap, definitions->storageMap)) {
         CHECK_NULL(refMap);
         CHECK_NULL(typeMap);

frontends/p4/toP4/toP4.h

@@ -51,9 +51,9 @@ class ToP4 : public Inspector {
 
     // maintained as stacks
     std::vector<VecPrint> vectorSeparator;
-    size_t vectorSeparator_init_apply_size;
+    size_t vectorSeparator_init_apply_size = 0;
     std::vector<ListPrint> listTerminators;
-    size_t listTerminators_init_apply_size;
+    size_t listTerminators_init_apply_size = 0;
 
     void setVecSep(const char *sep, const char *term = nullptr) {
         vectorSeparator.push_back(VecPrint(sep, term));

ir/v1.def

@@ -227,10 +227,10 @@ class CalculatedField : IAnnotated {
     optional NullOK Expression  field;
     class update_or_verify {
         Util::SourceInfo        srcInfo;
-        bool                    update;
+        optional bool           update = false;
         ID                      name;
         Expression              cond;
-        update_or_verify() {}  // FIXME -- needed by umpack_json(safe_vector) -- should not be
+        update_or_verify() { }  // FIXME -- needed by umpack_json(safe_vector) -- should not be
     }
     safe_vector<update_or_verify> specs = {};
     Annotations                 annotations;
@@ -448,7 +448,7 @@ class GlobalRef : Expression {
     Node        obj;            // FIXME -- should be IInstance, but IRgen doesn't allow
                                 // FIXME -- interface references directly in the IR
     GlobalRef { type = obj->to<IInstance>()->getType(); }
-    validate { obj->is<IInstance>(); }
+    validate { BUG_CHECK(obj->is<IInstance>(), "Invalid object %1%", obj); }
     toString { return obj->to<IInstance>()->toString(); }
     ID Name() const { return obj->to<IInstance>()->Name(); }
     dbprint { out << obj->to<IInstance>()->Name(); }

ir/visitor.h

@@ -245,6 +245,7 @@ class Visitor {
     template <class T>
     inline const T *findContext(const Context *&c) const {
         if (!c) c = ctxt;
+        if (!c) return nullptr;
         while ((c = c->parent))
             if (auto *rv = dynamic_cast<const T *>(c->node)) return rv;
         return nullptr;
@@ -257,6 +258,7 @@ class Visitor {
     template <class T>
     inline const T *findOrigCtxt(const Context *&c) const {
         if (!c) c = ctxt;
+        if (!c) return nullptr;
         while ((c = c->parent))
             if (auto *rv = dynamic_cast<const T *>(c->original)) return rv;
         return nullptr;

midend/simplifyKey.h

@@ -40,7 +40,7 @@ class KeyIsSimple {
  */
 class IsLikeLeftValue : public KeyIsSimple, public Inspector {
  protected:
-    bool simple;
+    bool simple = false;
 
  public:
     IsLikeLeftValue() { setName("IsLikeLeftValue"); }