re #440: fix all scan-build warnings

changelog/current.md

@@ -1,7 +1,16 @@
 ## Fixes
 
+- (WIP) Fix [#440](https://github.com/biojppm/rapidyaml/issues/440) Some tests failing with gcc -O2 (due to undefined behavior)
+  - Fix all warnings from `scan-build`
+- Use malloc.h instead of alloca.h on MinGW ([PR#447](https://github.com/biojppm/rapidyaml/pull/447))
 - Fix [#442](https://github.com/biojppm/rapidyaml/issues/442) ([PR#443](https://github.com/biojppm/rapidyaml/pull/443)):
   - Ensure leading `+` is accepted when deserializing numbers.
   - Ensure numbers are not quoted by fixing the heuristics in `scalar_style_query_plain()` and `scalar_style_choose()`.
   - Add quickstart sample for overflow detection (only of integral types).
 - Parse engine: cleanup unused macros
+
+
+## Thanks
+
+- @toge
+- @musicinmybrain

src/c4/yml/event_handler_tree.hpp

@@ -104,11 +104,13 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
 
     void start_parse(const char* filename, detail::pfn_relocate_arena relocate_arena, void *relocate_arena_data)
     {
+        _RYML_CB_ASSERT(m_stack.m_callbacks, m_tree != nullptr);
         this->_stack_start_parse(filename, relocate_arena, relocate_arena_data);
     }
 
     void finish_parse()
     {
+        _RYML_CB_ASSERT(m_stack.m_callbacks, m_tree != nullptr);
         if(m_num_directives && !m_tree->is_stream(m_tree->root_id()))
             _RYML_CB_ERR_(m_stack.m_callbacks, "directives cannot be used without a document", {});
         this->_stack_finish_parse();
@@ -307,6 +309,7 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
 
     void add_sibling()
     {
+        _RYML_CB_ASSERT(m_stack.m_callbacks, m_tree);
         _RYML_CB_ASSERT(m_stack.m_callbacks, m_parent);
         _RYML_CB_ASSERT(m_stack.m_callbacks, m_tree->has_children(m_parent->node_id));
         NodeData const* prev = m_tree->m_buf; // watchout against relocation of the tree nodes
@@ -445,6 +448,7 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
     void set_key_anchor(csubstr anchor)
     {
         _c4dbgpf("node[{}]: set key anchor: [{}]~~~{}~~~", m_curr->node_id, anchor.len, anchor);
+        _RYML_CB_ASSERT(m_stack.m_callbacks, m_tree);
         if(C4_UNLIKELY(_has_any_(KEYREF)))
             _RYML_CB_ERR_(m_tree->callbacks(), "key cannot have both anchor and ref", m_curr->pos);
         _RYML_CB_ASSERT(m_tree->callbacks(), !anchor.begins_with('&'));
@@ -454,6 +458,7 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
     void set_val_anchor(csubstr anchor)
     {
         _c4dbgpf("node[{}]: set val anchor: [{}]~~~{}~~~", m_curr->node_id, anchor.len, anchor);
+        _RYML_CB_ASSERT(m_stack.m_callbacks, m_tree);
         if(C4_UNLIKELY(_has_any_(VALREF)))
             _RYML_CB_ERR_(m_tree->callbacks(), "val cannot have both anchor and ref", m_curr->pos);
         _RYML_CB_ASSERT(m_tree->callbacks(), !anchor.begins_with('&'));
@@ -464,6 +469,7 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
     void set_key_ref(csubstr ref)
     {
         _c4dbgpf("node[{}]: set key ref: [{}]~~~{}~~~", m_curr->node_id, ref.len, ref);
+        _RYML_CB_ASSERT(m_stack.m_callbacks, m_tree);
         if(C4_UNLIKELY(_has_any_(KEYANCH)))
             _RYML_CB_ERR_(m_tree->callbacks(), "key cannot have both anchor and ref", m_curr->pos);
         _RYML_CB_ASSERT(m_tree->callbacks(), ref.begins_with('*'));
@@ -474,6 +480,7 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
     void set_val_ref(csubstr ref)
     {
         _c4dbgpf("node[{}]: set val ref: [{}]~~~{}~~~", m_curr->node_id, ref.len, ref);
+        _RYML_CB_ASSERT(m_stack.m_callbacks, m_tree);
         if(C4_UNLIKELY(_has_any_(VALANCH)))
             _RYML_CB_ERR_(m_tree->callbacks(), "val cannot have both anchor and ref", m_curr->pos);
         _RYML_CB_ASSERT(m_tree->callbacks(), ref.begins_with('*'));
@@ -541,6 +548,7 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
 
     substr alloc_arena(size_t len)
     {
+        _RYML_CB_ASSERT(m_stack.m_callbacks, m_tree);
         csubstr prev = m_tree->arena();
         substr out = m_tree->alloc_arena(len);
         substr curr = m_tree->arena();
@@ -551,6 +559,7 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
 
     substr alloc_arena(size_t len, substr *relocated)
     {
+        _RYML_CB_ASSERT(m_stack.m_callbacks, m_tree);
         csubstr prev = m_tree->arena();
         if(!prev.is_super(*relocated))
             return alloc_arena(len);
@@ -568,12 +577,13 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
     /** @cond dev */
     void _reset_parser_state(state* st, id_type parse_root, id_type node)
     {
+        _RYML_CB_ASSERT(m_stack.m_callbacks, m_tree);
         _set_state_(st, node);
         const NodeType type = m_tree->type(node);
         #ifdef RYML_DBG
         char flagbuf[80];
-        #endif
         _c4dbgpf("resetting state: initial flags={}", detail::_parser_flags_to_str(flagbuf, st->flags));
+        #endif
         if(type == NOTYPE)
         {
             _c4dbgpf("node[{}] is notype", node);
@@ -612,7 +622,9 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
             _c4dbgpf("node[{}] is doc", node);
             st->flags |= RDOC;
         }
+        #ifdef RYML_DBG
         _c4dbgpf("resetting state: final flags={}", detail::_parser_flags_to_str(flagbuf, st->flags));
+        #endif
     }
 
     /** push a new parent, add a child to the new parent, and set the
@@ -697,6 +709,7 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
     void _remove_speculative()
     {
         _c4dbgp("remove speculative node");
+        _RYML_CB_ASSERT(m_stack.m_callbacks, m_tree);
         _RYML_CB_ASSERT(m_tree->callbacks(), m_tree->size() > 0);
         const id_type last_added = m_tree->size() - 1;
         if(m_tree->has_parent(last_added))
@@ -706,6 +719,7 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
 
     void _remove_speculative_with_parent()
     {
+        _RYML_CB_ASSERT(m_stack.m_callbacks, m_tree);
         _RYML_CB_ASSERT(m_tree->callbacks(), m_tree->size() > 0);
         const id_type last_added = m_tree->size() - 1;
         _RYML_CB_ASSERT(m_tree->callbacks(), m_tree->has_parent(last_added));
@@ -718,6 +732,7 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
 
     C4_ALWAYS_INLINE void _save_loc()
     {
+        _RYML_CB_ASSERT(m_stack.m_callbacks, m_tree);
         _RYML_CB_ASSERT(m_tree->callbacks(), m_tree->_p(m_curr->node_id)->m_val.scalar.len == 0);
         m_tree->_p(m_curr->node_id)->m_val.scalar.str = m_curr->line_contents.rem.str;
     }

src/c4/yml/parse.cpp

@@ -28,7 +28,7 @@ inline void _reset_tree_handler(Parser *parser, Tree *t, id_type node_id)
     RYML_ASSERT(parser);
     RYML_ASSERT(t);
     if(!parser->m_evt_handler)
-        _RYML_CB_ERR(parser->m_evt_handler->m_stack.m_callbacks, "event handler is not set");
+        _RYML_CB_ERR(t->m_callbacks, "event handler is not set");
     parser->m_evt_handler->reset(t, node_id);
     RYML_ASSERT(parser->m_evt_handler->m_tree == t);
 }

src/c4/yml/parse_engine.def.hpp

@@ -3222,7 +3222,7 @@ void ParseEngine<EventHandler>::_filter_block_folded_newlines(FilterProcessor &C
             // In the end, moving this block to a separate function
             // was the only way to bury the problem. But it may
             // resurface again, as The Undead, rising to from the
-            // grave to haunt us with his terrible
+            // grave to haunt us with his terrible presence.
             //
             // We may have to revisit this. With a stake, and lots of
             // garlic.

src/c4/yml/reference_resolver.cpp

@@ -208,16 +208,10 @@ void ReferenceResolver::resolve(Tree *t_)
             _c4dbgpf("ref {} has parent: {}", i, refdata.parent_ref);
             _RYML_CB_ASSERT(m_tree->m_callbacks, m_tree->is_seq(refdata.parent_ref));
             const id_type p = m_tree->parent(refdata.parent_ref);
-            id_type after;
-            if(prev_parent_ref != refdata.parent_ref)
-            {
-                after = refdata.parent_ref;//prev_sibling(rd.parent_ref_sibling);
-                prev_parent_ref_after = after;
-            }
-            else
-            {
-                after = prev_parent_ref_after;
-            }
+            const id_type after = (prev_parent_ref != refdata.parent_ref) ?
+                refdata.parent_ref//prev_sibling(rd.parent_ref_sibling)
+                :
+                prev_parent_ref_after;
             prev_parent_ref = refdata.parent_ref;
             prev_parent_ref_after = m_tree->duplicate_children_no_rep(refdata.target, p, after);
             m_tree->remove(refdata.node);

test/test_noderef.cpp

@@ -1118,14 +1118,12 @@ TEST(NodeRef, vsConstNodeRef)
     // mseq = seq; // deliberate compilation error
     seq = mseq; // ok
     {
-        NodeData *nd = mseq.get();
-        // nd = seq.get(); // deliberate compile error
-        C4_UNUSED(nd);
+        (void)mseq.get();
+        //(void)seq.get(); // deliberate compile error
     }
     {
-        NodeData const* nd = seq.get();
-        nd = seq.get(); // ok
-        C4_UNUSED(nd);
+        (void)seq.get();
+        (void)seq.get(); // ok
     }
     test_invariants(t);
 }

test/test_serialize.cpp

@@ -565,11 +565,11 @@ void test_deserialize_trailing_434(csubstr yaml, csubstr val, csubstr first, dou
     ConstNodeRef cnode = tree["val"];
     NodeRef node = tree["val"];
     {
-        int value;
+        int value = {};
         EXPECT_FALSE(read(cnode, &value));
     }
     {
-        int value;
+        int value = {};
         EXPECT_FALSE(read(node, &value));
     }
     ExpectError::do_check(&tree, [&]{
@@ -582,22 +582,22 @@ void test_deserialize_trailing_434(csubstr yaml, csubstr val, csubstr first, dou
     });
     float fval = (float)dval;
     {
-        float value;
+        float value = {};
         EXPECT_TRUE(read(cnode, &value));
         EXPECT_EQ(memcmp(&value, &fval, sizeof(fval)), 0);
     }
     {
-        float value;
+        float value = {};
         EXPECT_TRUE(read(node, &value));
         EXPECT_EQ(memcmp(&value, &fval, sizeof(fval)), 0);
     }
     {
-        double value;
+        double value = {};
         EXPECT_TRUE(read(cnode, &value));
         EXPECT_EQ(memcmp(&value, &dval, sizeof(dval)), 0);
     }
     {
-        double value;
+        double value = {};
         EXPECT_TRUE(read(node, &value));
         EXPECT_EQ(memcmp(&value, &dval, sizeof(dval)), 0);
     }

test/test_tree.cpp

@@ -719,9 +719,8 @@ TEST(Tree, move_ctor)
     test_compare(save, src);
     {
         Tree dst(std::move(src));
-        EXPECT_EQ(src.empty(), true);
-        EXPECT_EQ(src.size(), 0u);
-        EXPECT_EQ(src.arena().empty(), true);
+        EXPECT_EQ(src.m_size, 0u);
+        EXPECT_EQ(src.m_arena_pos, 0u);
         EXPECT_EQ(dst.size(), save.size());
         EXPECT_EQ(dst.arena(), save.arena());
         test_invariants(src);
@@ -789,10 +788,9 @@ TEST(Tree, move_assign_same_callbacks)
         EXPECT_NE(dst.size(), 0u);
         EXPECT_NE(dst.arena().empty(), true);
         dst = std::move(src);
-        EXPECT_EQ(src.empty(), true);
-        EXPECT_EQ(src.size(), 0u);
-        EXPECT_EQ(src.arena().empty(), true);
-        EXPECT_EQ(src.callbacks(), cbt.callbacks());
+        EXPECT_EQ(src.m_size, 0u);
+        EXPECT_EQ(src.m_arena_pos, 0u);
+        EXPECT_EQ(src.m_callbacks, cbt.callbacks());
         EXPECT_EQ(dst.size(), save.size());
         EXPECT_EQ(dst.arena(), save.arena());
         EXPECT_EQ(dst.callbacks(), save.callbacks());
@@ -820,10 +818,9 @@ TEST(Tree, move_assign_diff_callbacks)
         EXPECT_NE(dst.arena().empty(), true);
         EXPECT_EQ(dst.callbacks(), cbdst.callbacks());
         dst = std::move(src);
-        EXPECT_EQ(src.empty(), true);
-        EXPECT_EQ(src.size(), 0u);
-        EXPECT_EQ(src.arena().empty(), true);
-        EXPECT_EQ(src.callbacks(), cbsrc.callbacks());
+        EXPECT_EQ(src.m_size, 0u);
+        EXPECT_EQ(src.m_arena_pos, 0u);
+        EXPECT_EQ(src.m_callbacks, cbsrc.callbacks());
         EXPECT_EQ(dst.size(), save.size());
         EXPECT_EQ(dst.arena(), save.arena());
         EXPECT_NE(dst.callbacks(), cbdst.callbacks());
@@ -3714,16 +3711,16 @@ TEST(Tree, lookup_path_or_modify)
         EXPECT_EQ(t["newmap2"]["newseq2"][2]["newmap2"]["newseq2"][2].val(), nullptr);
         EXPECT_EQ(t["newmap2"]["newseq2"][2]["newmap2"]["newseq2"][3].is_map(), true);
         EXPECT_EQ(t["newmap2"]["newseq2"][2]["newmap2"]["newseq2"][3]["first2"].val(), "y");
-        sz2 = t.lookup_path_or_modify("z", "newmap2.newseq2[2].newmap2.newseq2[3].second2");
+        (void)t.lookup_path_or_modify("z", "newmap2.newseq2[2].newmap2.newseq2[3].second2");
         EXPECT_EQ  (t["newmap2"]["newseq2"][2]["newmap2"]["newseq2"][3]["second2"].val(), "z");
 
-        sz = t.lookup_path_or_modify("foo", "newmap.newseq1[1]");
+        (void)t.lookup_path_or_modify("foo", "newmap.newseq1[1]");
         EXPECT_EQ(t["newmap"].is_map(), true);
         EXPECT_EQ(t["newmap"]["newseq1"].is_seq(), true);
         EXPECT_EQ(t["newmap"]["newseq1"].num_children(), 2u);
         EXPECT_EQ(t["newmap"]["newseq1"][0].val(), nullptr);
         EXPECT_EQ(t["newmap"]["newseq1"][1].val(), "foo");
-        sz = t.lookup_path_or_modify("bar", "newmap.newseq1[2][1]");
+        (void)t.lookup_path_or_modify("bar", "newmap.newseq1[2][1]");
         EXPECT_EQ(t["newmap"]["newseq1"].is_seq(), true);
         EXPECT_EQ(t["newmap"]["newseq1"].num_children(), 3u);
         EXPECT_EQ(t["newmap"]["newseq1"][0].val(), nullptr);
@@ -3732,12 +3729,12 @@ TEST(Tree, lookup_path_or_modify)
         EXPECT_EQ(t["newmap"]["newseq1"][2].num_children(), 2u);
         EXPECT_EQ(t["newmap"]["newseq1"][2][0].val(), nullptr);
         EXPECT_EQ(t["newmap"]["newseq1"][2][1].val(), "bar");
-        sz = t.lookup_path_or_modify("Foo?"   , "newmap.newseq1[0]");
-        sz = t.lookup_path_or_modify("Bar?"   , "newmap.newseq1[2][0]");
-        sz = t.lookup_path_or_modify("happy"  , "newmap.newseq1[2][2][3]");
-        sz = t.lookup_path_or_modify("trigger", "newmap.newseq1[2][2][2]");
-        sz = t.lookup_path_or_modify("Arnold" , "newmap.newseq1[2][2][0]");
-        sz = t.lookup_path_or_modify("is"     , "newmap.newseq1[2][2][1]");
+        (void)t.lookup_path_or_modify("Foo?"   , "newmap.newseq1[0]");
+        (void)t.lookup_path_or_modify("Bar?"   , "newmap.newseq1[2][0]");
+        (void)t.lookup_path_or_modify("happy"  , "newmap.newseq1[2][2][3]");
+        (void)t.lookup_path_or_modify("trigger", "newmap.newseq1[2][2][2]");
+        (void)t.lookup_path_or_modify("Arnold" , "newmap.newseq1[2][2][0]");
+        (void)t.lookup_path_or_modify("is"     , "newmap.newseq1[2][2][1]");
         EXPECT_EQ(t["newmap"]["newseq1"].is_seq(), true);
         EXPECT_EQ(t["newmap"]["newseq1"].num_children(), 3u);
         EXPECT_EQ(t["newmap"]["newseq1"][0].val(), "Foo?");