Diff
Modified: trunk/Tools/ChangeLog (287788 => 287789)
--- trunk/Tools/ChangeLog 2022-01-07 22:56:47 UTC (rev 287788)
+++ trunk/Tools/ChangeLog 2022-01-07 22:56:53 UTC (rev 287789)
@@ -1,3 +1,41 @@
+2022-01-07 David Kilzer <ddkil...@apple.com>
+
+ check-webkit-style: add checker for unexpected fall through after ASSERT_NOT_REACHED() statements
+ <https://webkit.org/b/234932>
+ <rdar://87213520>
+
+ Reviewed by Brent Fulgham.
+
+ This checker only returns a confidence level of 4 (out of 5)
+ since there are too many different code patterns to check them
+ all perfectly using regular expressions.
+
+ Run new checker tests like this:
+ $ test-webkitpy webkitpy.style.checkers.cpp_unittest.CppStyleTest.test_debug_not_reached_assertion
+
+ Run this checker against a file or folder like this:
+ $ check-webkit-style --filter "-,+security/assertion_fallthrough" (file|folder)
+
+ * Scripts/webkitpy/style/checkers/cpp.py:
+ (SingleLineView.__init__):
+ - Expose `trimmed_lines` as a property on SingleLineView.
+ (_FunctionState.body_view): Add.
+ - Add method to return a SingleLineView object containing the
+ body of the function.
+ (check_function_body): Add.
+ - New function to check for fall through after
+ ASSERT_NOT_REACHED() statements.
+ (process_line):
+ - Call check_function_body() to implement the new check.
+ (CppChecker.categories):
+ - Add 'security/assertion_fallthrough' to the list of known
+ checkers.
+ * Scripts/webkitpy/style/checkers/cpp_unittest.py:
+ (CppStyleTestBase.perform_function_body_check): Add.
+ - Add method to call check_function_body() for testing.
+ (CppStyleTest.test_debug_not_reached_assertion): Add.
+ - Add tests for the new checker.
+
2022-01-07 Wenson Hsieh <wenson_hs...@apple.com>
Teach modal container observer to make the body element scrollable if necessary
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py (287788 => 287789)
--- trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py 2022-01-07 22:56:47 UTC (rev 287788)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py 2022-01-07 22:56:53 UTC (rev 287789)
@@ -438,21 +438,21 @@
end_position: just after where to end (like a slice operation).
"""
# Get the rows of interest.
- trimmed_lines = lines[start_position.row:end_position.row + 1]
+ self.trimmed_lines = lines[start_position.row:end_position.row + 1]
# Remove the columns on the last line that aren't included.
- trimmed_lines[-1] = trimmed_lines[-1][:end_position.column]
+ self.trimmed_lines[-1] = self.trimmed_lines[-1][:end_position.column]
# Remove the columns on the first line that aren't included.
- trimmed_lines[0] = trimmed_lines[0][start_position.column:]
+ self.trimmed_lines[0] = self.trimmed_lines[0][start_position.column:]
# Create a single line with all of the parameters.
- self.single_line = ' '.join(trimmed_lines)
+ self.single_line = ' '.join(self.trimmed_lines)
# Keep the row lengths, so we can calculate the original row number
# given a column in the single line (adding 1 due to the space added
# during the join).
- self._row_lengths = [len(line) + 1 for line in trimmed_lines]
+ self._row_lengths = [len(line) + 1 for line in self.trimmed_lines]
self._starting_row = start_position.row
def convert_column_to_row(self, single_line_column_number):
@@ -600,6 +600,11 @@
elided = self._clean_lines.elided
return SingleLineView(elided, self.parameter_end_position, self.body_start_position).single_line.strip()
+ def body_view(self):
+ """Returns the function body."""
+ elided = self._clean_lines.elided
+ return SingleLineView(elided, self.body_start_position, self.end_position)
+
def attributes_after_definition(self, attribute_regex):
return re.findall(attribute_regex, self.post_modifiers())
@@ -2044,6 +2049,44 @@
'%s should only appear in directories matching %s.' % (export_macro, path))
+def check_function_body(filename, file_extension, clean_lines, line_number, class_state, function_state, error):
+ """Check function bodies for style issues.
+
+ Args:
+ filename: Filename of the file that is being processed.
+ file_extension: The current file extension, without the leading dot.
+ clean_lines: A CleansedLines instance containing the file.
+ line_number: The number of the line to check.
+ function_state: Current function name and lines in body so far.
+ error: The function to call with any errors found.
+ """
+ if line_number != function_state.end_position.row: # last line
+ return
+
+ function_body_view = function_state.body_view()
+ function_line_count = len(function_body_view.trimmed_lines)
+
+ # Check for uncontrolled fall-through after ASSERT_NOT_REACHED() statement.
+ for i in range(0, function_line_count):
+ current_line = function_body_view.trimmed_lines[i]
+ if not re.search(r'[^_]ASSERT_NOT_REACHED\(\);', current_line):
+ continue
+
+ min_index = max(0, i - 1)
+ max_index = min(function_line_count, i + 4)
+ partial_function_body = ' '.join(function_body_view.trimmed_lines[min_index:max_index])
+
+ if search(r'[^_]ASSERT_NOT_REACHED\(\);\s*(continue|return(\s+[^;]+)?);', partial_function_body) \
+ or search(r'[^_]ASSERT_NOT_REACHED\(\);(\s*#endif)?(\s*})+\s*$', partial_function_body) \
+ or search(r'[^_]ASSERT_NOT_REACHED\(\);(\s*completionHandler[^;]+;)?(\s*})+\s*$', partial_function_body) \
+ or search(r'[^_]ASSERT_NOT_REACHED\(\);(\s*[^;]+;)?\s*return(\s+[^;]+)?;', partial_function_body) \
+ or search(r'(default|case\s+.+):\s*[^_]ASSERT_NOT_REACHED\(\);(\s*[^;]+;)*\s*break;', partial_function_body):
+ continue
+
+ error(line_number - (function_line_count - (i + 1)), 'security/assertion_fallthrough', 4,
+ 'ASSERT_NOT_REACHED() statement fallthrough may result in unexpected code execution.')
+
+
def check_for_leaky_patterns(clean_lines, line_number, function_state, error):
"""Check for constructs known to be leak prone.
Args:
@@ -4496,6 +4539,7 @@
if asm_state.is_in_asm(): # Ignore further checks because asm blocks formatted differently.
return
check_function_definition(filename, file_extension, clean_lines, line, class_state, function_state, error)
+ check_function_body(filename, file_extension, clean_lines, line, class_state, function_state, error)
check_for_leaky_patterns(clean_lines, line, function_state, error)
check_for_multiline_comments_and_strings(clean_lines, line, error)
check_style(clean_lines, line, file_extension, class_state, file_state, enum_state, error)
@@ -4655,6 +4699,7 @@
'runtime/wtf_move',
'runtime/wtf_never_destroyed',
'security/assertion',
+ 'security/assertion_fallthrough',
'security/_javascript_core_wtf_blockptr',
'security/missing_warn_unused_return',
'security/printf',
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py (287788 => 287789)
--- trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py 2022-01-07 22:56:47 UTC (rev 287788)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py 2022-01-07 22:56:53 UTC (rev 287789)
@@ -338,6 +338,22 @@
error_collector)
self.assertEqual(error_collector.results(), expected_warning)
+ def perform_function_body_check(self, file_name, lines, expected_warning):
+ file_extension = file_name.split('.')[1]
+ clean_lines = cpp_style.CleansedLines(lines.splitlines())
+ function_state = cpp_style._FunctionState(5)
+ error_collector = ErrorCollector(self.assertTrue)
+
+ for i in range(clean_lines.num_lines()):
+ cpp_style.detect_functions(clean_lines, i, function_state, error_collector)
+ self.assertEqual(function_state.in_a_function, True)
+ self.assertEqual(error_collector.results(), '')
+
+ class_state = cpp_style._ClassState()
+ cpp_style.check_function_body(file_name, file_extension, clean_lines, clean_lines.num_lines() - 1, class_state,
+ function_state, error_collector)
+ self.assertEqual(error_collector.results(), expected_warning)
+
# Perform lint and compare the error message with "expected_message".
def assert_lint(self, code, expected_message, file_name='foo.cpp'):
self.assertEqual(expected_message, self.perform_single_line_lint(code, file_name))
@@ -1624,6 +1640,220 @@
' for improved thread safety.'
' [runtime/threadsafe_fn] [2]')
+ def test_debug_not_reached_assertion(self):
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'void f()\n'
+ '{\n'
+ ' if (auto createdHandle = SandboxExtension::createHandle(URL.fileSystemPath(), SandboxExtension::Type::ReadWrite))\n'
+ ' handle = WTFMove(*createdHandle);\n'
+ ' else\n'
+ ' ASSERT_NOT_REACHED();\n\n'
+ ' m_dataStore->networkProcess().send(Messages::NetworkProcess::PublishDownloadProgress(m_downloadID, URL, handle), 0);]\n'
+ '}',
+ 'ASSERT_NOT_REACHED() statement fallthrough may result in unexpected code execution.'
+ ' [security/assertion_fallthrough] [4]')
+
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'void f()\n'
+ '{\n'
+ ' if (auto createdHandle = SandboxExtension::createHandle(URL.fileSystemPath(), SandboxExtension::Type::ReadWrite))\n'
+ ' handle = WTFMove(*createdHandle);\n'
+ ' else\n'
+ ' RELEASE_ASSERT_NOT_REACHED();\n\n'
+ ' m_dataStore->networkProcess().send(Messages::NetworkProcess::PublishDownloadProgress(m_downloadID, URL, handle), 0);]\n'
+ '}',
+ '')
+
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'void f()\n'
+ '{\n'
+ ' for (int i = 0; i < 10; ++i) {'
+ ' if (i % 2 == 0) {'
+ ' ASSERT_NOT_REACHED();\n'
+ ' continue;\n'
+ ' }\n'
+ ' }\n'
+ '}',
+ '')
+
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'void f(int i)\n'
+ '{\n'
+ ' if (i % 2 == 0) {'
+ ' ASSERT_NOT_REACHED();\n'
+ ' return;\n'
+ ' }\n'
+ '}',
+ '')
+
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'void f(int i)\n'
+ '{\n'
+ ' if (i % 2 == 0) {'
+ ' ASSERT_NOT_REACHED();\n'
+ ' completionHandler(nullptr);\n'
+ ' return;\n'
+ ' }\n'
+ '}',
+ '')
+
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'void f(int i)\n'
+ '{\n'
+ ' if (i % 2 == 0) {'
+ ' ASSERT_NOT_REACHED();\n'
+ ' failureBlock(nullptr);\n'
+ ' return;\n'
+ ' }\n'
+ '}',
+ '')
+
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'void f()\n'
+ '{\n'
+ ' ASSERT_NOT_REACHED();\n'
+ ' completionHandler(nullString());\n'
+ '}',
+ '')
+
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'void f(int i)\n'
+ '{\n'
+ ' if (i % 2 == 0) {\n'
+ ' ASSERT_NOT_REACHED();\n'
+ ' completionHandler(nullString());\n'
+ ' }\n'
+ '}',
+ '')
+
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'void f(int i)\n'
+ '{\n'
+ ' switch (i) {'
+ ' case 0:'
+ ' ASSERT_NOT_REACHED();\n'
+ ' }\n'
+ ' g();\n'
+ '}',
+ 'ASSERT_NOT_REACHED() statement fallthrough may result in unexpected code execution.'
+ ' [security/assertion_fallthrough] [4]')
+
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'void f(int i)\n'
+ '{\n'
+ ' switch (i) {'
+ ' default:'
+ ' ASSERT_NOT_REACHED();\n'
+ ' }\n'
+ ' g();\n'
+ '}',
+ 'ASSERT_NOT_REACHED() statement fallthrough may result in unexpected code execution.'
+ ' [security/assertion_fallthrough] [4]')
+
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'void f(int i)\n'
+ '{\n'
+ ' if (!i) {'
+ ' g();'
+ ' } else {'
+ ' ASSERT_NOT_REACHED();\n'
+ ' }\n'
+ '}',
+ '')
+
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'void f(int i)\n'
+ '{\n'
+ ' switch (i) {'
+ ' case 0:'
+ ' ASSERT_NOT_REACHED();\n'
+ ' break;\n'
+ ' }\n'
+ '}',
+ '')
+
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'void f(int i)\n'
+ '{\n'
+ ' switch (i) {'
+ ' default:'
+ ' ASSERT_NOT_REACHED();\n'
+ ' break;\n'
+ ' }\n'
+ '}',
+ '')
+
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'void f(int i)\n'
+ '{\n'
+ ' switch (i) {'
+ ' case 0:'
+ ' ASSERT_NOT_REACHED();\n'
+ ' m_completionHandler();\n'
+ ' break;\n'
+ ' }\n'
+ '}',
+ '')
+
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'void f(int i)\n'
+ '{\n'
+ ' switch (i) {'
+ ' default:'
+ ' ASSERT_NOT_REACHED();\n'
+ ' m_completionHandler();\n'
+ ' break;\n'
+ ' }\n'
+ '}',
+ '')
+
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'void f()\n'
+ '{\n'
+ '#if PLATFORM(COCOA)\n'
+ ' pageClient().clearTextIndicator(WebCore::TextIndicatorDismissalAnimation::FadeOut);\n'
+ '#else\n'
+ ' ASSERT_NOT_REACHED();\n'
+ '#endif\n'
+ '}',
+ '')
+
+ self.perform_function_body_check(
+ 'foo.cpp',
+ 'int f()\n'
+ '{\n'
+ ' ASSERT_NOT_REACHED();\n'
+ ' *errorCode = U_UNSUPPORTED_ERROR;\n'
+ ' return 0;\n'
+ '}',
+ '')
+
+ self.perform_function_body_check(
+ 'foo.h',
+ 'void f() { ASSERT_NOT_REACHED(); }',
+ '')
+ self.perform_function_body_check(
+ 'foo.h',
+ 'void* f() { ASSERT_NOT_REACHED(); return nullptr; }',
+ '')
+
+
def test_debug_security_assertion(self):
self.assert_lint(
'ASSERT_WITH_SECURITY_IMPLICATION(value)',
