Changeset 287789 in webkit


Ignore:
Timestamp:
Jan 7, 2022, 2:56:53 PM (4 years ago)
Author:
ddkilzer@apple.com
Message:

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.
Location:
trunk/Tools
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r287777 r287789  
     12022-01-07  David Kilzer  <ddkilzer@apple.com>
     2
     3        check-webkit-style: add checker for unexpected fall through after ASSERT_NOT_REACHED() statements
     4        <https://webkit.org/b/234932>
     5        <rdar://87213520>
     6
     7        Reviewed by Brent Fulgham.
     8
     9        This checker only returns a confidence level of 4 (out of 5)
     10        since there are too many different code patterns to check them
     11        all perfectly using regular expressions.
     12
     13        Run new checker tests like this:
     14        $ test-webkitpy webkitpy.style.checkers.cpp_unittest.CppStyleTest.test_debug_not_reached_assertion
     15
     16        Run this checker against a file or folder like this:
     17        $ check-webkit-style --filter "-,+security/assertion_fallthrough" (file|folder)
     18
     19        * Scripts/webkitpy/style/checkers/cpp.py:
     20        (SingleLineView.__init__):
     21        - Expose `trimmed_lines` as a property on SingleLineView.
     22        (_FunctionState.body_view): Add.
     23        - Add method to return a SingleLineView object containing the
     24          body of the function.
     25        (check_function_body): Add.
     26        - New function to check for fall through after
     27          ASSERT_NOT_REACHED() statements.
     28        (process_line):
     29        - Call check_function_body() to implement the new check.
     30        (CppChecker.categories):
     31        - Add 'security/assertion_fallthrough' to the list of known
     32          checkers.
     33        * Scripts/webkitpy/style/checkers/cpp_unittest.py:
     34        (CppStyleTestBase.perform_function_body_check): Add.
     35        - Add method to call check_function_body() for testing.
     36        (CppStyleTest.test_debug_not_reached_assertion): Add.
     37        - Add tests for the new checker.
     38
    1392022-01-07  Wenson Hsieh  <wenson_hsieh@apple.com>
    240

  • trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py

    r286772 r287789  
    439439        """
    440440        # Get the rows of interest.
    441         trimmed_lines = lines[start_position.row:end_position.row + 1]
     441        self.trimmed_lines = lines[start_position.row:end_position.row + 1]
    442442
    443443        # Remove the columns on the last line that aren't included.
    444         trimmed_lines[-1] = trimmed_lines[-1][:end_position.column]
     444        self.trimmed_lines[-1] = self.trimmed_lines[-1][:end_position.column]
    445445
    446446        # Remove the columns on the first line that aren't included.
    447         trimmed_lines[0] = trimmed_lines[0][start_position.column:]
     447        self.trimmed_lines[0] = self.trimmed_lines[0][start_position.column:]
    448448
    449449        # Create a single line with all of the parameters.
    450         self.single_line = ' '.join(trimmed_lines)
     450        self.single_line = ' '.join(self.trimmed_lines)
    451451
    452452        # Keep the row lengths, so we can calculate the original row number
    453453        # given a column in the single line (adding 1 due to the space added
    454454        # during the join).
    455         self._row_lengths = [len(line) + 1 for line in trimmed_lines]
     455        self._row_lengths = [len(line) + 1 for line in self.trimmed_lines]
    456456        self._starting_row = start_position.row
    457457
    … …  
    600600        elided = self._clean_lines.elided
    601601        return SingleLineView(elided, self.parameter_end_position, self.body_start_position).single_line.strip()
     602
     603    def body_view(self):
     604        """Returns the function body."""
     605        elided = self._clean_lines.elided
     606        return SingleLineView(elided, self.body_start_position, self.end_position)
    602607
    603608    def attributes_after_definition(self, attribute_regex):
    … …  
    20452050
    20462051
     2052def check_function_body(filename, file_extension, clean_lines, line_number, class_state, function_state, error):
     2053    """Check function bodies for style issues.
     2054
     2055    Args:
     2056       filename: Filename of the file that is being processed.
     2057       file_extension: The current file extension, without the leading dot.
     2058       clean_lines: A CleansedLines instance containing the file.
     2059       line_number: The number of the line to check.
     2060       function_state: Current function name and lines in body so far.
     2061       error: The function to call with any errors found.
     2062    """
     2063    if line_number != function_state.end_position.row:  # last line
     2064        return
     2065
     2066    function_body_view = function_state.body_view()
     2067    function_line_count = len(function_body_view.trimmed_lines)
     2068
     2069    # Check for uncontrolled fall-through after ASSERT_NOT_REACHED() statement.
     2070    for i in range(0, function_line_count):
     2071        current_line = function_body_view.trimmed_lines[i]
     2072        if not re.search(r'[^_]ASSERT_NOT_REACHED\(\);', current_line):
     2073            continue
     2074
     2075        min_index = max(0, i - 1)
     2076        max_index = min(function_line_count, i + 4)
     2077        partial_function_body = ' '.join(function_body_view.trimmed_lines[min_index:max_index])
     2078
     2079        if search(r'[^_]ASSERT_NOT_REACHED\(\);\s*(continue|return(\s+[^;]+)?);', partial_function_body) \
     2080                or search(r'[^_]ASSERT_NOT_REACHED\(\);(\s*#endif)?(\s*})+\s*$', partial_function_body) \
     2081                or search(r'[^_]ASSERT_NOT_REACHED\(\);(\s*completionHandler[^;]+;)?(\s*})+\s*$', partial_function_body) \
     2082                or search(r'[^_]ASSERT_NOT_REACHED\(\);(\s*[^;]+;)?\s*return(\s+[^;]+)?;', partial_function_body) \
     2083                or search(r'(default|case\s+.+):\s*[^_]ASSERT_NOT_REACHED\(\);(\s*[^;]+;)*\s*break;', partial_function_body):
     2084            continue
     2085
     2086        error(line_number - (function_line_count - (i + 1)), 'security/assertion_fallthrough', 4,
     2087              'ASSERT_NOT_REACHED() statement fallthrough may result in unexpected code execution.')
     2088
     2089
    20472090def check_for_leaky_patterns(clean_lines, line_number, function_state, error):
    20482091    """Check for constructs known to be leak prone.
    … …  
    44974540        return
    44984541    check_function_definition(filename, file_extension, clean_lines, line, class_state, function_state, error)
     4542    check_function_body(filename, file_extension, clean_lines, line, class_state, function_state, error)
    44994543    check_for_leaky_patterns(clean_lines, line, function_state, error)
    45004544    check_for_multiline_comments_and_strings(clean_lines, line, error)
    … …  
    46564700        'runtime/wtf_never_destroyed',
    46574701        'security/assertion',
     4702        'security/assertion_fallthrough',
    46584703        'security/javascriptcore_wtf_blockptr',
    46594704        'security/missing_warn_unused_return',

  • trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py

    r283625 r287789  
    337337        cpp_style.check_function_definition(file_name, file_extension, clean_lines, 0, class_state, function_state,
    338338                                            error_collector)
     339        self.assertEqual(error_collector.results(), expected_warning)
     340
     341    def perform_function_body_check(self, file_name, lines, expected_warning):
     342        file_extension = file_name.split('.')[1]
     343        clean_lines = cpp_style.CleansedLines(lines.splitlines())
     344        function_state = cpp_style._FunctionState(5)
     345        error_collector = ErrorCollector(self.assertTrue)
     346
     347        for i in range(clean_lines.num_lines()):
     348            cpp_style.detect_functions(clean_lines, i, function_state, error_collector)
     349        self.assertEqual(function_state.in_a_function, True)
     350        self.assertEqual(error_collector.results(), '')
     351
     352        class_state = cpp_style._ClassState()
     353        cpp_style.check_function_body(file_name, file_extension, clean_lines, clean_lines.num_lines() - 1, class_state,
     354                                      function_state, error_collector)
    339355        self.assertEqual(error_collector.results(), expected_warning)
    340356
    … …  
    16241640                         ' for improved thread safety.'
    16251641                         '  [runtime/threadsafe_fn] [2]')
     1642
     1643    def test_debug_not_reached_assertion(self):
     1644        self.perform_function_body_check(
     1645            'foo.cpp',
     1646            'void f()\n'
     1647            '{\n'
     1648            '    if (auto createdHandle = SandboxExtension::createHandle(URL.fileSystemPath(), SandboxExtension::Type::ReadWrite))\n'
     1649            '        handle = WTFMove(*createdHandle);\n'
     1650            '    else\n'
     1651            '        ASSERT_NOT_REACHED();\n\n'
     1652            '    m_dataStore->networkProcess().send(Messages::NetworkProcess::PublishDownloadProgress(m_downloadID, URL, handle), 0);]\n'
     1653            '}',
     1654            'ASSERT_NOT_REACHED() statement fallthrough may result in unexpected code execution.'
     1655            '  [security/assertion_fallthrough] [4]')
     1656
     1657        self.perform_function_body_check(
     1658            'foo.cpp',
     1659            'void f()\n'
     1660            '{\n'
     1661            '    if (auto createdHandle = SandboxExtension::createHandle(URL.fileSystemPath(), SandboxExtension::Type::ReadWrite))\n'
     1662            '        handle = WTFMove(*createdHandle);\n'
     1663            '    else\n'
     1664            '        RELEASE_ASSERT_NOT_REACHED();\n\n'
     1665            '    m_dataStore->networkProcess().send(Messages::NetworkProcess::PublishDownloadProgress(m_downloadID, URL, handle), 0);]\n'
     1666            '}',
     1667            '')
     1668
     1669        self.perform_function_body_check(
     1670            'foo.cpp',
     1671            'void f()\n'
     1672            '{\n'
     1673            '    for (int i = 0; i < 10; ++i) {'
     1674            '        if (i % 2 == 0) {'
     1675            '            ASSERT_NOT_REACHED();\n'
     1676            '            continue;\n'
     1677            '        }\n'
     1678            '    }\n'
     1679            '}',
     1680            '')
     1681
     1682        self.perform_function_body_check(
     1683            'foo.cpp',
     1684            'void f(int i)\n'
     1685            '{\n'
     1686            '    if (i % 2 == 0) {'
     1687            '        ASSERT_NOT_REACHED();\n'
     1688            '        return;\n'
     1689            '    }\n'
     1690            '}',
     1691            '')
     1692
     1693        self.perform_function_body_check(
     1694            'foo.cpp',
     1695            'void f(int i)\n'
     1696            '{\n'
     1697            '    if (i % 2 == 0) {'
     1698            '        ASSERT_NOT_REACHED();\n'
     1699            '        completionHandler(nullptr);\n'
     1700            '        return;\n'
     1701            '    }\n'
     1702            '}',
     1703            '')
     1704
     1705        self.perform_function_body_check(
     1706            'foo.cpp',
     1707            'void f(int i)\n'
     1708            '{\n'
     1709            '    if (i % 2 == 0) {'
     1710            '        ASSERT_NOT_REACHED();\n'
     1711            '        failureBlock(nullptr);\n'
     1712            '        return;\n'
     1713            '    }\n'
     1714            '}',
     1715            '')
     1716
     1717        self.perform_function_body_check(
     1718            'foo.cpp',
     1719            'void f()\n'
     1720            '{\n'
     1721            '    ASSERT_NOT_REACHED();\n'
     1722            '    completionHandler(nullString());\n'
     1723            '}',
     1724            '')
     1725
     1726        self.perform_function_body_check(
     1727            'foo.cpp',
     1728            'void f(int i)\n'
     1729            '{\n'
     1730            '    if (i % 2 == 0) {\n'
     1731            '        ASSERT_NOT_REACHED();\n'
     1732            '        completionHandler(nullString());\n'
     1733            '    }\n'
     1734            '}',
     1735            '')
     1736
     1737        self.perform_function_body_check(
     1738            'foo.cpp',
     1739            'void f(int i)\n'
     1740            '{\n'
     1741            '    switch (i) {'
     1742            '    case 0:'
     1743            '        ASSERT_NOT_REACHED();\n'
     1744            '    }\n'
     1745            '    g();\n'
     1746            '}',
     1747            'ASSERT_NOT_REACHED() statement fallthrough may result in unexpected code execution.'
     1748            '  [security/assertion_fallthrough] [4]')
     1749
     1750        self.perform_function_body_check(
     1751            'foo.cpp',
     1752            'void f(int i)\n'
     1753            '{\n'
     1754            '    switch (i) {'
     1755            '    default:'
     1756            '        ASSERT_NOT_REACHED();\n'
     1757            '    }\n'
     1758            '    g();\n'
     1759            '}',
     1760            'ASSERT_NOT_REACHED() statement fallthrough may result in unexpected code execution.'
     1761            '  [security/assertion_fallthrough] [4]')
     1762
     1763        self.perform_function_body_check(
     1764            'foo.cpp',
     1765            'void f(int i)\n'
     1766            '{\n'
     1767            '    if (!i) {'
     1768            '        g();'
     1769            '    } else {'
     1770            '        ASSERT_NOT_REACHED();\n'
     1771            '    }\n'
     1772            '}',
     1773            '')
     1774
     1775        self.perform_function_body_check(
     1776            'foo.cpp',
     1777            'void f(int i)\n'
     1778            '{\n'
     1779            '    switch (i) {'
     1780            '    case 0:'
     1781            '        ASSERT_NOT_REACHED();\n'
     1782            '        break;\n'
     1783            '    }\n'
     1784            '}',
     1785            '')
     1786
     1787        self.perform_function_body_check(
     1788            'foo.cpp',
     1789            'void f(int i)\n'
     1790            '{\n'
     1791            '    switch (i) {'
     1792            '    default:'
     1793            '        ASSERT_NOT_REACHED();\n'
     1794            '        break;\n'
     1795            '    }\n'
     1796            '}',
     1797            '')
     1798
     1799        self.perform_function_body_check(
     1800            'foo.cpp',
     1801            'void f(int i)\n'
     1802            '{\n'
     1803            '    switch (i) {'
     1804            '    case 0:'
     1805            '        ASSERT_NOT_REACHED();\n'
     1806            '        m_completionHandler();\n'
     1807            '        break;\n'
     1808            '    }\n'
     1809            '}',
     1810            '')
     1811
     1812        self.perform_function_body_check(
     1813            'foo.cpp',
     1814            'void f(int i)\n'
     1815            '{\n'
     1816            '    switch (i) {'
     1817            '    default:'
     1818            '        ASSERT_NOT_REACHED();\n'
     1819            '        m_completionHandler();\n'
     1820            '        break;\n'
     1821            '    }\n'
     1822            '}',
     1823            '')
     1824
     1825        self.perform_function_body_check(
     1826            'foo.cpp',
     1827            'void f()\n'
     1828            '{\n'
     1829            '#if PLATFORM(COCOA)\n'
     1830            '    pageClient().clearTextIndicator(WebCore::TextIndicatorDismissalAnimation::FadeOut);\n'
     1831            '#else\n'
     1832            '    ASSERT_NOT_REACHED();\n'
     1833            '#endif\n'
     1834            '}',
     1835            '')
     1836
     1837        self.perform_function_body_check(
     1838            'foo.cpp',
     1839            'int f()\n'
     1840            '{\n'
     1841            '    ASSERT_NOT_REACHED();\n'
     1842            '    *errorCode = U_UNSUPPORTED_ERROR;\n'
     1843            '    return 0;\n'
     1844            '}',
     1845            '')
     1846
     1847        self.perform_function_body_check(
     1848            'foo.h',
     1849            'void f() { ASSERT_NOT_REACHED(); }',
     1850            '')
     1851        self.perform_function_body_check(
     1852            'foo.h',
     1853            'void* f() { ASSERT_NOT_REACHED(); return nullptr; }',
     1854            '')
     1855
    16261856
    16271857    def test_debug_security_assertion(self):
Note: See TracChangeset for help on using the changeset viewer.