Fix #14150 (Add unionzeroinit check)

addons/test/unionzeroinit.py

@@ -0,0 +1,87 @@
+# Running the test with Python 3:
+# Command in cppcheck directory:
+# PYTHONPATH=./addons python3 -m pytest addons/test/unionzeroinit.py
+
+import contextlib
+import sys
+
+from addons import cppcheckdata
+from addons import unionzeroinit
+from . import util
+
+
+@contextlib.contextmanager
+def run(path):
+    sys.argv.append("--cli")
+    util.dump_create(path)
+    data = cppcheckdata.CppcheckData(f"{path}.dump")
+    for cfg in data.iterconfigurations():
+        yield cfg, data
+    sys.argv.remove("--cli")
+    util.dump_remove(path)
+
+
+def test_basic(capsys):
+    with run("./addons/test/unionzeroinit/basic.c") as (cfg, data):
+        unionzeroinit.union_zero_init(cfg, data)
+        captured = capsys.readouterr()
+        captured = captured.out.splitlines()
+        json_output = util.convert_json_output(captured)
+        assert len(json_output["unionzeroinit"]) == 3
+        assert json_output["unionzeroinit"][0]["linenr"] == 22
+        assert json_output["unionzeroinit"][1]["linenr"] == 23
+        assert json_output["unionzeroinit"][2]["linenr"] == 25
+
+
+def test_array_member(capsys):
+    with run("./addons/test/unionzeroinit/array.c") as (cfg, data):
+        unionzeroinit.union_zero_init(cfg, data)
+        captured = capsys.readouterr()
+        captured = captured.out.splitlines()
+        json_output = util.convert_json_output(captured)
+        assert len(json_output) == 0
+
+
+def test_struct_member(capsys):
+    with run("./addons/test/unionzeroinit/struct.c") as (cfg, data):
+        unionzeroinit.union_zero_init(cfg, data, True)
+        captured = capsys.readouterr()
+        captured = captured.out.splitlines()
+        json_output = util.convert_json_output(captured)
+        assert len(json_output) == 0
+
+
+def test_struct_cyclic_member(capsys):
+    with run("./addons/test/unionzeroinit/struct-cyclic.c") as (cfg, data):
+        unionzeroinit.union_zero_init(cfg, data, True)
+        captured = capsys.readouterr()
+        captured = captured.out.splitlines()
+        json_output = util.convert_json_output(captured)
+        assert len(json_output) == 0
+
+
+def test_unknown_type(capsys):
+    with run("./addons/test/unionzeroinit/unknown-type.c") as (cfg, data):
+        unionzeroinit.union_zero_init(cfg, data, True)
+        captured = capsys.readouterr()
+        captured = captured.out.splitlines()
+        json_output = util.convert_json_output(captured)
+        assert len(json_output) == 0
+
+
+def test_long_long(capsys):
+    with run("./addons/test/unionzeroinit/long-long.c") as (cfg, data):
+        unionzeroinit.union_zero_init(cfg, data, True)
+        captured = capsys.readouterr()
+        captured = captured.out.splitlines()
+        json_output = util.convert_json_output(captured)
+        assert len(json_output) == 0
+
+
+def test_bitfields(capsys):
+    with run("./addons/test/unionzeroinit/bitfields.c") as (cfg, data):
+        unionzeroinit.union_zero_init(cfg, data, True)
+        captured = capsys.readouterr()
+        captured = captured.out.splitlines()
+        json_output = util.convert_json_output(captured)
+        assert len(json_output) == 0

addons/test/unionzeroinit/array.c

@@ -0,0 +1,3 @@
+void foo(void) {
+    union { int c; char s8[2]; } u = {0};
+}

addons/test/unionzeroinit/basic.c

@@ -0,0 +1,26 @@
+#include <stdint.h>
+
+union bad_union_0 {
+    char c;
+    int64_t i64;
+    void *p;
+};
+
+typedef union {
+    char c;
+    int i;
+} bad_union_1;
+
+extern void e(union bad_union_0 *);
+
+void
+foo(void)
+{
+    union { int i; char c; } good0 = {0};
+    union { int i; char c; } good1 = {};
+
+    union { char c; int i; } bad0 = {0};
+    union bad_union_0 bad1 = {0};
+    e(&bad1);
+    bad_union_1 bad2 = {0};
+}

addons/test/unionzeroinit/bitfields.c

@@ -0,0 +1,15 @@
+typedef union Evex {
+	int u32;
+	struct {
+		char	mmm:3,
+			b4:1,
+			r4:1,
+			b3:1,
+			x3:1,
+			r3:1;
+	} extended;
+} Evex;
+
+void foo(void) {
+    Evex evex = {0};
+}

addons/test/unionzeroinit/long-long.c

@@ -0,0 +1,3 @@
+union u {
+    long long x;
+};

addons/test/unionzeroinit/struct-cyclic.c

@@ -0,0 +1,4 @@
+oid foo(void) {
+    struct s { struct s *s; }
+    union { struct s s; } u = {0};
+}

addons/test/unionzeroinit/struct.c

@@ -0,0 +1,11 @@
+void foo(void) {
+    union {
+        int c;
+        struct {
+            char x;
+            struct {
+                char y;
+            } s1;
+        } s0;
+    } u = {0};
+}

addons/test/unionzeroinit/unknown-type.c

@@ -0,0 +1,3 @@
+union u {
+    Unknown x;
+};

addons/unionzeroinit.py

@@ -0,0 +1,185 @@
+#!/usr/bin/env python3
+#
+# Detect error-prone zero initialization of unions.
+
+import dataclasses
+import cppcheck
+import cppcheckdata
+from typing import List, Tuple
+
+
+@dataclasses.dataclass
+class Member:
+    name: str
+    size: int
+
+
+@dataclasses.dataclass
+class Union:
+    name: str
+    members: List[Member] = dataclasses.field(default_factory=list)
+
+    def largest_member(self):
+        return sorted(self.members, key=lambda m: -m.size)[0]
+
+    def is_largest_member_first(self):
+        sizes = [m.size for m in self.members]
+
+        has_unknown_sizes = 0 in sizes
+        if has_unknown_sizes:
+            return True
+
+        return sizes[0] == sorted(sizes, key=lambda s: -s)[0]
+
+
+def estimate_size_of_type(
+    platform: cppcheckdata.Platform, type: str, pointer: bool
+) -> int:
+    bits = 0
+    if pointer:
+        bits = platform.pointer_bit
+    elif type == "char":
+        bits = platform.char_bit
+    elif type == "short":
+        bits = platform.short_bit
+    elif type == "int":
+        bits = platform.int_bit
+    elif type == "long":
+        bits = platform.long_bit
+    elif type == "long_long":
+        bits = platform.long_long_bit
+    else:
+        # Fair estimate...
+        bits = platform.int_bit
+    return bits
+
+
+def tokat(token: cppcheckdata.Token, offset) -> cppcheckdata.Token:
+    at = token.tokAt(offset)
+    if at:
+        return at
+
+    empty = {"str": ""}
+    return cppcheckdata.Token(empty)
+
+
+def parse_array_length(token) -> int:
+    if not tokat(token, 1).str == "[":
+        return 1
+
+    nelements = 0
+    try:
+        nelements = int(tokat(token, 2).str)
+    except ValueError:
+        return 1
+
+    if not tokat(token, 3).str == "]":
+        return 1
+
+    return nelements
+
+
+def is_zero_initialized(token):
+    return (
+        tokat(token, 1).str == "="
+        and tokat(token, 2).str == "{"
+        and (
+            tokat(token, 3).str == "}"
+            or (tokat(token, 3).str == "0" and tokat(token, 4).str == "}")
+        )
+    )
+
+
+def is_pointer(variable: cppcheckdata.Variable) -> bool:
+    return variable.nameToken.valueType.pointer and not variable.isArray
+
+
+def accumulated_member_size(
+    data: cppcheckdata.CppcheckData, variable: cppcheckdata.Variable
+) -> Tuple[str, int]:
+    # Note that cppcheck might not be able to observe all types due to
+    # inaccessible include(s).
+    if not variable.nameToken.valueType:
+        return (None, 0)
+
+    if variable.nameToken.valueType.type == "record":
+        if not variable.nameToken.valueType.typeScope:
+            return (None, 0)
+
+        nested_variables = variable.nameToken.valueType.typeScope.varlist
+
+        # Circumvent what seems to be a bug in which only the last bitfield has
+        # its bits properly assigned.
+        has_bitfields = any([v.nameToken.valueType.bits for v in nested_variables])
+        if has_bitfields:
+            return (variable.nameToken.str, len(nested_variables))
+
+        total_size = 0
+        for nested in nested_variables:
+            # Avoid potential cyclic members referring to the type currently
+            # being traversed.
+            if is_pointer(nested):
+                total_size += data.platform.pointer_bit
+            else:
+                _, size = accumulated_member_size(data, nested.nameToken.variable)
+                total_size += size
+        return (variable.nameToken.str, total_size)
+
+    vt = variable.nameToken.valueType
+    if vt.bits:
+        size = vt.bits
+    else:
+        size = estimate_size_of_type(
+            data.platform,
+            variable.nameToken.valueType.type,
+            is_pointer(variable),
+        )
+    if variable.isArray:
+        size *= parse_array_length(variable.nameToken)
+    return (variable.nameToken.str, size)
+
+
+def error_message(u: Union):
+    return (
+        f"Zero initializing union {u.name} does not guarantee its complete "
+        "storage to be zero initialized as its largest member is not declared "
+        f"as the first member. Consider making {u.largest_member().name} the "
+        "first member or favor memset()."
+    )
+
+
+@cppcheck.checker
+def union_zero_init(cfg, data, debug=False):
+    unions_by_id = {}
+
+    # Detect union declarations.
+    for scope in cfg.scopes:
+        if not scope.type == "Union":
+            continue
+
+        union = Union(name=scope.className)
+        for variable in scope.varlist:
+            name, size = accumulated_member_size(data, variable)
+            union.members.append(Member(name=name, size=size))
+        unions_by_id[scope.Id] = union
+
+    if debug:
+        for id, u in unions_by_id.items():
+            print(id, u, u.is_largest_member_first(), u.largest_member())
+
+    # Detect problematic union variables.
+    for token in cfg.tokenlist:
+        if (
+            token.valueType
+            and token.valueType.typeScopeId in unions_by_id
+            and token.isName
+            and is_zero_initialized(token)
+        ):
+            id = token.valueType.typeScopeId
+            if not unions_by_id[id].is_largest_member_first():
+                cppcheck.reportError(
+                    token,
+                    "portability",
+                    error_message(unions_by_id[id]),
+                    "unionzeroinit",
+                )