From 3cf76d06d36904753bfd3c1b099bd15546025dc7 Mon Sep 17 00:00:00 2001 From: Michael Tautschnig Date: Wed, 26 Nov 2025 10:06:34 +0000 Subject: [PATCH] Add cpplint warnings for exit() and abort() Our code should only terminate normally via return from main, or abnormally via the macros from `invariant.h`. While at it, also fixes some warnings about cpplint.py's code. Fixes: #1902 --- .../cpp-linter/abort-exit-no-warning/main.cpp | 48 ++++++++++++++++ .../module_dependencies.txt | 1 + .../abort-exit-no-warning/test.desc | 9 +++ .../cpp-linter/abort-exit-nolint/main.cpp | 43 +++++++++++++++ .../abort-exit-nolint/module_dependencies.txt | 1 + .../cpp-linter/abort-exit-nolint/test.desc | 12 ++++ regression/cpp-linter/abort/main.cpp | 52 ++++++++++++++++++ .../cpp-linter/abort/module_dependencies.txt | 1 + regression/cpp-linter/abort/test.desc | 14 +++++ regression/cpp-linter/exit/main.cpp | 55 +++++++++++++++++++ .../cpp-linter/exit/module_dependencies.txt | 1 + regression/cpp-linter/exit/test.desc | 17 ++++++ scripts/cpplint.py | 50 +++++++++++++++-- 13 files changed, 299 insertions(+), 5 deletions(-) create mode 100644 regression/cpp-linter/abort-exit-no-warning/main.cpp create mode 100644 regression/cpp-linter/abort-exit-no-warning/module_dependencies.txt create mode 100644 regression/cpp-linter/abort-exit-no-warning/test.desc create mode 100644 regression/cpp-linter/abort-exit-nolint/main.cpp create mode 100644 regression/cpp-linter/abort-exit-nolint/module_dependencies.txt create mode 100644 regression/cpp-linter/abort-exit-nolint/test.desc create mode 100644 regression/cpp-linter/abort/main.cpp create mode 100644 regression/cpp-linter/abort/module_dependencies.txt create mode 100644 regression/cpp-linter/abort/test.desc create mode 100644 regression/cpp-linter/exit/main.cpp create mode 100644 regression/cpp-linter/exit/module_dependencies.txt create mode 100644 regression/cpp-linter/exit/test.desc diff --git a/regression/cpp-linter/abort-exit-no-warning/main.cpp b/regression/cpp-linter/abort-exit-no-warning/main.cpp new file mode 100644 index 00000000000..4be57c83d83 --- /dev/null +++ b/regression/cpp-linter/abort-exit-no-warning/main.cpp @@ -0,0 +1,48 @@ +// Author: Michael Tautschnig + +// Test file that should NOT generate any termination warnings +// This file contains various uses of "exit" and "abort" that should not trigger + +#include +#include + +int main() +{ + // Variable names containing exit/abort - should not trigger + int exit_code = 0; + bool exit_flag = false; + void *exit_ptr = nullptr; + bool abort_flag = false; + int abort_count = 0; + + // Assignment to variables + exit_code = 1; + exit_flag = true; + abort_flag = false; + + // Function calls that use custom namespaces (not std:: or ::) + // Note: Member function calls like obj.exit() would trigger warnings + // due to current regex implementation, so we avoid them here + + return exit_code; +} + +// Function definitions that shouldn't trigger +void my_exit_function() +{ + std::cout << "This is not exit()" << std::endl; +} + +void abort_handler() +{ + std::cout << "This is not abort()" << std::endl; +} + +// Custom namespace functions (not std:: or ::) +void use_custom_functions() +{ + // These have custom namespace prefixes, so should not trigger + // Note: current implementation only checks for std:: and :: prefixes + // myns::exit(0); // Would not trigger if uncommented + // custom::abort(); // Would not trigger if uncommented +} diff --git a/regression/cpp-linter/abort-exit-no-warning/module_dependencies.txt b/regression/cpp-linter/abort-exit-no-warning/module_dependencies.txt new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/regression/cpp-linter/abort-exit-no-warning/module_dependencies.txt @@ -0,0 +1 @@ + diff --git a/regression/cpp-linter/abort-exit-no-warning/test.desc b/regression/cpp-linter/abort-exit-no-warning/test.desc new file mode 100644 index 00000000000..60102a0f233 --- /dev/null +++ b/regression/cpp-linter/abort-exit-no-warning/test.desc @@ -0,0 +1,9 @@ +CORE +main.cpp + +^# Total errors found: 0$ +^EXIT=0$ +^SIGNAL=0$ +-- +^# Total errors found: [1-9] +-- diff --git a/regression/cpp-linter/abort-exit-nolint/main.cpp b/regression/cpp-linter/abort-exit-nolint/main.cpp new file mode 100644 index 00000000000..fe7858f5610 --- /dev/null +++ b/regression/cpp-linter/abort-exit-nolint/main.cpp @@ -0,0 +1,43 @@ +// Author: Michael Tautschnig + +// Test file for NOLINT suppression +// This file tests that NOLINT comments properly suppress warnings + +#include + +int main() +{ + // These should be suppressed by NOLINT comments + exit(1); // NOLINT - justified use case + abort(); // NOLINT(runtime/termination) - specific suppression + + std::exit(2); // NOLINT + ::abort(); // NOLINT(runtime/termination) + + // These should still trigger warnings (no NOLINT) + exit(3); // Line 18: Warning expected + abort(); // Line 19: Warning expected + + // Test NOLINT with other categories (should still suppress) + exit(4); // NOLINT(whitespace/parens) + + return 0; +} + +void test_function() +{ + // Mixed suppressed and unsuppressed + exit(5); // Line 30: Warning expected + abort(); // NOLINT - suppressed + std::exit(6); // Line 32: Warning expected + ::abort(); // NOLINT(runtime/termination) - suppressed +} + +// Test NOLINT at end of line vs middle +void another_test() +{ + exit(7); /* NOLINT */ + int x = 0; + abort(); // Line 41: Warning expected + int y = 0; +} diff --git a/regression/cpp-linter/abort-exit-nolint/module_dependencies.txt b/regression/cpp-linter/abort-exit-nolint/module_dependencies.txt new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/regression/cpp-linter/abort-exit-nolint/module_dependencies.txt @@ -0,0 +1 @@ + diff --git a/regression/cpp-linter/abort-exit-nolint/test.desc b/regression/cpp-linter/abort-exit-nolint/test.desc new file mode 100644 index 00000000000..942ada1a141 --- /dev/null +++ b/regression/cpp-linter/abort-exit-nolint/test.desc @@ -0,0 +1,12 @@ +CORE +main.cpp + +^regression/cpp-linter/abort-exit-nolint/main\.cpp:18: exit\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/abort-exit-nolint/main\.cpp:19: abort\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/abort-exit-nolint/main\.cpp:22: exit\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/abort-exit-nolint/main\.cpp:30: exit\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/abort-exit-nolint/main\.cpp:32: exit\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/abort-exit-nolint/main\.cpp:41: abort\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^# Total errors found: 6$ +^EXIT=1$ +^SIGNAL=0$ diff --git a/regression/cpp-linter/abort/main.cpp b/regression/cpp-linter/abort/main.cpp new file mode 100644 index 00000000000..9e5f70d4fd3 --- /dev/null +++ b/regression/cpp-linter/abort/main.cpp @@ -0,0 +1,52 @@ +// Author: Michael Tautschnig + +// Test file for abort() call detection +// This file should generate multiple warnings + +#include + +void error_handler() +{ + // Basic abort calls - should trigger warnings + abort(); // Line 11: Warning expected + + // Namespace qualified calls - should trigger warnings + std::abort(); // Line 14: Warning expected + ::abort(); // Line 15: Warning expected + + // Calls with whitespace - should trigger warnings + abort(); // Line 18: Warning expected + abort(); // Line 19: Warning expected + abort(); // Line 20: Warning expected (tab) + + // Calls in expressions - should trigger warnings + if(fatal_error) + abort(); // Line 24: Warning expected +} + +// Variable names - should NOT trigger warnings +bool abort_flag = false; +int abort_count = 0; + +class abortablet +{ +public: + void process() + { + if(should_abort) + { + abort(); // Line 38: Warning expected + } + } +}; + +// Function names containing abort - should NOT trigger +void abort_operation() +{ + return; +} + +void check_abort_status() +{ + return; +} diff --git a/regression/cpp-linter/abort/module_dependencies.txt b/regression/cpp-linter/abort/module_dependencies.txt new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/regression/cpp-linter/abort/module_dependencies.txt @@ -0,0 +1 @@ + diff --git a/regression/cpp-linter/abort/test.desc b/regression/cpp-linter/abort/test.desc new file mode 100644 index 00000000000..7c0b566cf92 --- /dev/null +++ b/regression/cpp-linter/abort/test.desc @@ -0,0 +1,14 @@ +CORE +main.cpp + +^regression/cpp-linter/abort/main\.cpp:11: abort\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/abort/main\.cpp:14: abort\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/abort/main\.cpp:15: abort\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/abort/main\.cpp:18: abort\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/abort/main\.cpp:19: abort\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/abort/main\.cpp:20: abort\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/abort/main\.cpp:24: abort\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/abort/main\.cpp:38: abort\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^# Total errors found: 8$ +^EXIT=1$ +^SIGNAL=0$ diff --git a/regression/cpp-linter/exit/main.cpp b/regression/cpp-linter/exit/main.cpp new file mode 100644 index 00000000000..34299fb9446 --- /dev/null +++ b/regression/cpp-linter/exit/main.cpp @@ -0,0 +1,55 @@ +// Author: Michael Tautschnig + +// Test file for exit() call detection +// This file should generate multiple warnings + +#include + +int main() +{ + // Basic exit calls - should trigger warnings + exit(1); // Line 11: Warning expected + exit(EXIT_FAILURE); // Line 12: Warning expected + + // Namespace qualified calls - should trigger warnings + std::exit(1); // Line 15: Warning expected + ::exit(2); // Line 16: Warning expected + + // Calls with whitespace - should trigger warnings + exit(3); // Line 19: Warning expected + exit(4); // Line 20: Warning expected + exit(5); // Line 21: Warning expected (tab) + + // Calls in expressions - should trigger warnings + if(condition) + exit(6); // Line 25: Warning expected + return exit(7); // Line 26: Warning expected + + // Variable names - should NOT trigger warnings + int exit_code = 0; + bool exit_flag = false; + void *exit_ptr = nullptr; + + return exit_code; +} + +void error_function() +{ + // More exit calls in different contexts + exit(10); // Line 39: Warning expected +} + +class test_classt +{ +public: + void method() + { + exit(11); // Line 47: Warning expected + } +}; + +// Function names containing exit - should NOT trigger +void my_exit_function() +{ + return; +} diff --git a/regression/cpp-linter/exit/module_dependencies.txt b/regression/cpp-linter/exit/module_dependencies.txt new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/regression/cpp-linter/exit/module_dependencies.txt @@ -0,0 +1 @@ + diff --git a/regression/cpp-linter/exit/test.desc b/regression/cpp-linter/exit/test.desc new file mode 100644 index 00000000000..32e8065e2da --- /dev/null +++ b/regression/cpp-linter/exit/test.desc @@ -0,0 +1,17 @@ +CORE +main.cpp + +^regression/cpp-linter/exit/main\.cpp:11: exit\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/exit/main\.cpp:12: exit\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/exit/main\.cpp:15: exit\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/exit/main\.cpp:16: exit\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/exit/main\.cpp:19: exit\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/exit/main\.cpp:20: exit\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/exit/main\.cpp:21: exit\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/exit/main\.cpp:25: exit\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/exit/main\.cpp:26: exit\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/exit/main\.cpp:39: exit\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^regression/cpp-linter/exit/main\.cpp:47: exit\(\) should not be used. Normal termination should be via return from main. Abnormal termination should use invariant.h macros. If this use is justified, mark with NOLINT. See https://github.com/diffblue/cbmc/issues/1902 \[runtime/termination\] \[4\] +^# Total errors found: 11$ +^EXIT=1$ +^SIGNAL=0$ diff --git a/scripts/cpplint.py b/scripts/cpplint.py index 845267986d2..6991a4d78c4 100755 --- a/scripts/cpplint.py +++ b/scripts/cpplint.py @@ -55,7 +55,6 @@ import math # for log import os import re -import sre_compile import string import sys import sysconfig @@ -344,6 +343,7 @@ 'runtime/printf_format', 'runtime/references', 'runtime/string', + 'runtime/termination', 'runtime/threadsafe_fn', 'runtime/vlog', 'runtime/catch_test_tags', @@ -1048,7 +1048,7 @@ def Match(pattern, s): # performance reasons; factoring it out into a separate function turns out # to be noticeably expensive. if pattern not in _regexp_compile_cache: - _regexp_compile_cache[pattern] = sre_compile.compile(pattern) + _regexp_compile_cache[pattern] = re.compile(pattern) return _regexp_compile_cache[pattern].match(s) @@ -1066,14 +1066,14 @@ def ReplaceAll(pattern, rep, s): string with replacements made (or original string if no replacements) """ if pattern not in _regexp_compile_cache: - _regexp_compile_cache[pattern] = sre_compile.compile(pattern) + _regexp_compile_cache[pattern] = re.compile(pattern) return _regexp_compile_cache[pattern].sub(rep, s) def Search(pattern, s): """Searches the string for the pattern, caching the compiled regexp.""" if pattern not in _regexp_compile_cache: - _regexp_compile_cache[pattern] = sre_compile.compile(pattern) + _regexp_compile_cache[pattern] = re.compile(pattern) return _regexp_compile_cache[pattern].search(s) @@ -2049,7 +2049,7 @@ def IsTemplateArgumentList_DB(clean_lines, linenum, pos): start_pos = 0 inbetween_string += clean_lines.elided[linenum][0:pos] - is_simple_template_params = Match('^[<>(::),\w\s]*$', inbetween_string) + is_simple_template_params = Match(r'^[<>(::),\w\s]*$', inbetween_string) if is_simple_template_params: return True @@ -2816,6 +2816,45 @@ def CheckPosixThreading(filename, clean_lines, linenum, error): '...) for improved thread safety.') +def CheckTerminationFunctions(filename, clean_lines, linenum, error): + """Checks for use of exit() or abort() functions. + + According to the coding policy (https://github.com/diffblue/cbmc/issues/1902), + normal termination should be via return from main, and abnormal termination + should use macros from invariant.h. Any other use of exit() or abort() + requires explanation and should be explicitly marked as NOLINT. + + Args: + filename: The name of the current file. + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + error: The function to call with any errors found. + """ + line = clean_lines.elided[linenum] + + # Check for exit() - detect std::exit, ::exit, or plain exit followed by ( + # Use word boundary or specific characters to avoid matching variable names + # Pattern explanation: + # - (?:^|[^\w:]) - start of line or non-word/non-colon character + # - (?:std::)?(?:::)? - optional std:: or :: prefix + # - exit - the function name + # - \s*\( - optional whitespace followed by opening paren + if Search(r'(?:^|[^\w:])\s*(?:std::)?(?:::)?exit\s*\(', line): + error(filename, linenum, 'runtime/termination', 4, + 'exit() should not be used. Normal termination should be via return ' + 'from main. Abnormal termination should use invariant.h macros. ' + 'If this use is justified, mark with NOLINT. ' + 'See https://github.com/diffblue/cbmc/issues/1902') + + # Check for abort() - detect std::abort, ::abort, or plain abort followed by ( + if Search(r'(?:^|[^\w:])\s*(?:std::)?(?:::)?abort\s*\(', line): + error(filename, linenum, 'runtime/termination', 4, + 'abort() should not be used. Normal termination should be via return ' + 'from main. Abnormal termination should use invariant.h macros. ' + 'If this use is justified, mark with NOLINT. ' + 'See https://github.com/diffblue/cbmc/issues/1902') + + def CheckVlogArguments(filename, clean_lines, linenum, error): """Checks that VLOG() is only used for defining a logging level. @@ -6951,6 +6990,7 @@ def ProcessLine(filename, file_extension, clean_lines, line, nesting_state, error) CheckVlogArguments(filename, clean_lines, line, error) CheckPosixThreading(filename, clean_lines, line, error) + CheckTerminationFunctions(filename, clean_lines, line, error) CheckInvalidIncrement(filename, clean_lines, line, error) CheckMakePairUsesDeduction(filename, clean_lines, line, error) CheckRedundantVirtual(filename, clean_lines, line, error)