Fix #6028: normalize SDCC assembly before preprocessing

Author: KiritoYG

lib/cppcheck.cpp

@@ -1032,7 +1032,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
         // Get directives
         std::list<Directive> directives;
         preprocessor.createDirectives(directives);
-        preprocessor.simplifyPragmaAsm();
+        preprocessor.simplifyAsm();
 
         std::set<std::string> configurations;
         std::set<std::string> configDefines = { "__cplusplus" };
@@ -1060,7 +1060,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
                 if (!mSettings.keepComments)
                     Preprocessor::removeComments(data.tokens);
                 Preprocessor::createDirectives(data.tokens, directives);
-                Preprocessor::simplifyPragmaAsm(data.tokens);
+                Preprocessor::simplifyAsm(data.tokens);
                 // Discover new configurations from included file
                 if (configurations.size() < maxConfigs)
                     preprocessor.getConfigs(data.filename, data.tokens, configDefines, configurations);

lib/preprocessor.cpp

@@ -1133,15 +1133,89 @@ std::size_t Preprocessor::calculateHash(const std::string &toolinfo) const
     return (std::hash<std::string>{})(hashData);
 }
 
-void Preprocessor::simplifyPragmaAsm()
+static simplecpp::Token* simplifySdccAsm(simplecpp::TokenList& tokenList, simplecpp::Token* start)
 {
-    simplifyPragmaAsm(mTokens);
+    const simplecpp::Token* previous = start->previousSkipComments();
+    if (sameline(start, previous) && previous->op != '{' && previous->op != '}' && previous->op != ';')
+        return nullptr;
+    // Do not rewrite a macro definition or a GNU/MS-style asm statement.
+    for (const simplecpp::Token* tok = previous; sameline(start, tok); tok = tok->previous) {
+        if (tok->op == '#')
+            return nullptr;
+    }
+    const simplecpp::Token* first = start->nextSkipComments();
+    if (!first || (sameline(start, first) && first->op != ';') ||
+        first->op == '(' || first->op == '{' || first->str() == "volatile" ||
+        first->str() == "__volatile" || first->str() == "__volatile__" ||
+        first->str() == "goto" || first->str() == "inline")
+        return nullptr;
+
+    simplecpp::Token* end = start->next;
+    const simplecpp::Token* comment = nullptr;
+    bool nestedAsm = false;
+    for (; end; end = end->next) {
+        if (end->comment || sameline(end, comment))
+            continue;
+        if (end->op == ';') {
+            comment = end;
+            continue;
+        }
+        if (end->str() == "__endasm" && !sameline(end, end->previousSkipComments())) {
+            const simplecpp::Token* next = end->nextSkipComments();
+            if (!sameline(end, next) || next->op == ';')
+                break;
+        }
+        // An incomplete block must not consume C code or a different conditional
+        // branch. Leave such input to the normal preprocessor/tokenizer.
+        if (end->str() == "__asm")
+            nestedAsm = true;
+        if (end->op == '{' || end->op == '}' ||
+            (end->op == '#' && !sameline(end, end->previousSkipComments())))
+            return end->previous;
+    }
+    if (!end)
+        return tokenList.back();
+    // Do not normalize an inner block after rejecting an ambiguous outer one.
+    if (nestedAsm)
+        return end;
+
+    // Preserve an asm statement as an analysis barrier, including for empty
+    // blocks. Hide assembler # operands before simplecpp stringifies them.
+    std::unique_ptr<simplecpp::Token> open(new simplecpp::Token("(", start->location));
+    std::unique_ptr<simplecpp::Token> close(new simplecpp::Token(")", start->location));
+    // A second terminator would break an unbraced do/while or if/else body.
+    simplecpp::Token* terminator = end->next;
+    while (terminator && terminator->comment)
+        terminator = terminator->next;
+    if (terminator && terminator->op == ';')
+        tokenList.deleteToken(terminator);
+    start->setstr("asm");
+    end->setstr(";");
+    while (start->next != end)
+        tokenList.deleteToken(start->next);
+    open->previous = start;
+    open->next = close.get();
+    close->previous = open.get();
+    close->next = end;
+    start->next = open.release();
+    end->previous = close.release();
+    return end;
+}
+
+void Preprocessor::simplifyAsm()
+{
+    simplifyAsm(mTokens);
 }
 
-void Preprocessor::simplifyPragmaAsm(simplecpp::TokenList &tokenList)
+void Preprocessor::simplifyAsm(simplecpp::TokenList &tokenList)
 {
     // assembler code..
     for (simplecpp::Token *tok = tokenList.front(); tok; tok = tok->next) {
+        if (tok->str() == "__asm") {
+            if (simplecpp::Token* end = simplifySdccAsm(tokenList, tok))
+                tok = end;
+            continue;
+        }
         if (tok->op != '#')
             continue;
         if (sameline(tok, tok->previousSkipComments()))

lib/preprocessor.h

@@ -144,9 +144,9 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
      */
     std::size_t calculateHash(const std::string &toolinfo) const;
 
-    void simplifyPragmaAsm();
+    void simplifyAsm();
 
-    static void simplifyPragmaAsm(simplecpp::TokenList &tokenList);
+    static void simplifyAsm(simplecpp::TokenList &tokenList);
 
     static void getErrorMessages(ErrorLogger &errorLogger, const Settings &settings);
 

test/cli/other_test.py

@@ -90,6 +90,25 @@ def test_preprocessor_error(tmpdir):
     assert exitcode != 0
 
 
+@pytest.mark.parametrize('in_header', [False, True])
+def test_sdcc_asm_operands(tmp_path, in_header):  # #6028
+    source = ('int f(void) {\n'
+              '    __asm\n'
+              '        movx @dptr,a\n'
+              '        mov b,#(s_XINIT>>8)\n'
+              '    __endasm;\n'
+              '    return 1/0;\n'
+              '}\n')
+    main_file = tmp_path / 'main.c'
+    asm_file = tmp_path / 'asm.h' if in_header else main_file
+    asm_file.write_text(source)
+    if in_header:
+        main_file.write_text('#include "asm.h"\n')
+    exitcode, _, stderr = cppcheck(['--error-exitcode=1', '--template={file}:{line}:{id}', str(main_file)])
+    assert exitcode == 1
+    assert stderr == '{}:6:zerodiv\n'.format(asm_file)
+
+
 __ANSI_BOLD = "\x1b[1m"
 __ANSI_FG_RED = "\x1b[31m"
 __ANSI_FG_DEFAULT = "\x1b[39m"

test/testpreprocessor.cpp

@@ -136,7 +136,7 @@ class TestPreprocessor : public TestFixture {
         if (inlineSuppression)
             preprocessor.inlineSuppressions(*inlineSuppression);
         preprocessor.removeComments();
-        preprocessor.simplifyPragmaAsm();
+        preprocessor.simplifyAsm();
 
         std::map<std::string, std::string> cfgcode;
         if (cfgs.empty()) {
@@ -255,6 +255,13 @@ class TestPreprocessor : public TestFixture {
         TEST_CASE(pragma);
         TEST_CASE(pragma_asm_1);
         TEST_CASE(pragma_asm_2);
+        TEST_CASE(sdccAsmOperands);
+        TEST_CASE(sdccAsmEmptyAndAdjacent);
+        TEST_CASE(sdccAsmComments);
+        TEST_CASE(sdccAsmIncomplete);
+        TEST_CASE(sdccAsmOtherSyntax);
+        TEST_CASE(sdccAsmConditional);
+        TEST_CASE(sdccAsmControlFlow);
         TEST_CASE(endifsemicolon);
         TEST_CASE(missing_doublequote);
         TEST_CASE(handle_error);
@@ -1562,6 +1569,151 @@ class TestPreprocessor : public TestFixture {
         ASSERT_EQUALS("asm ( )\n;\n\nbbb", actual.at(""));
     }
 
+    void sdccAsmOperands() { // #6028
+        {
+            const char code[] = "void f() {\n"
+                                "__asm\n"
+                                "    movx @dptr,a\n"
+                                "__endasm;\n"
+                                "}\n";
+            ASSERT_EQUALS("void f ( ) {\nasm ( )\n\n;\n}", getcode(settings0, *this, code).at(""));
+        }
+        {
+            const char code[] = "__asm\n"
+                                "    mov b,#(s_XINIT>>8)\n"
+                                "__endasm";
+            ASSERT_EQUALS("asm ( )\n\n;", getcode(settings0, *this, code).at(""));
+        }
+        {
+            const char code[] = "int f(int x) {\n"
+                                "    ++x;\n"
+                                "    __asm\n"
+                                "        anl a,#0x0f\n"
+                                "        inc a\n"
+                                "        movc a,@a+pc\n"
+                                "        ret\n"
+                                "    __endasm;\n"
+                                "    return x;\n"
+                                "}\n";
+            ASSERT_EQUALS("int f ( int x ) {\n++ x ;\nasm ( )\n\n\n\n\n;\nreturn x ;\n}",
+                          getcode(settings0, *this, code).at(""));
+        }
+        ASSERT_EQUALS("", errout_str());
+    }
+
+    void sdccAsmEmptyAndAdjacent() {
+        ASSERT_EQUALS("asm ( )\n;", getcode(settings0, *this, "__asm\n__endasm").at(""));
+        const char code[] = "__asm\n"
+                            "__endasm;\n"
+                            "__asm\n"
+                            "    nop\n"
+                            "__endasm;\n"
+                            "int after;\n";
+        ASSERT_EQUALS("asm ( )\n;\nasm ( )\n\n;\nint after ;", getcode(settings0, *this, code).at(""));
+        ASSERT_EQUALS("", errout_str());
+    }
+
+    void sdccAsmComments() {
+        const char code[] = "/* __asm */\n"
+                            "const char *s = \"__asm __endasm\";\n"
+                            "__asm ; __endasm } #error ignored\n"
+                            "    ; __asm {\n"
+                            "    mov a,b ; __endasm\n"
+                            "    /* __endasm */ nop\n"
+                            "    mov a,__endasm\n"
+                            "__endasm;\n"
+                            "int after; // __asm\n";
+        ASSERT_EQUALS("\nconst char * s = \"__asm __endasm\" ;\nasm ( )\n\n\n\n\n;\nint after ;",
+                      getcode(settings0, *this, code).at(""));
+        ASSERT_EQUALS("", errout_str());
+    }
+
+    void sdccAsmIncomplete() {
+        ASSERT_EQUALS("__asm\nnop", getcode(settings0, *this, "__asm\nnop").at(""));
+        ASSERT_EQUALS("__asm\nnop\n__asm\nnop", getcode(settings0, *this, "__asm\nnop\n__asm\nnop").at(""));
+        ASSERT_EQUALS("__asm\nnop\n__asm\nnop\n__endasm ;\nint after ;",
+                      getcode(settings0, *this, "__asm\nnop\n__asm\nnop\n__endasm;\nint after;").at(""));
+        {
+            const char code[] = "void f() {\n"
+                                "__asm\n"
+                                "    nop\n"
+                                "}\n"
+                                "int after;\n"
+                                "__endasm;\n";
+            ASSERT_EQUALS("void f ( ) {\n__asm\nnop\n}\nint after ;\n__endasm ;",
+                          getcode(settings0, *this, code).at(""));
+        }
+        {
+            const char code[] = "__asm\n"
+                                "    nop\n"
+                                "#define N 1\n"
+                                "__endasm;\n"
+                                "int after;\n";
+            ASSERT_EQUALS("__asm\nnop\n\n__endasm ;\nint after ;", getcode(settings0, *this, code).at(""));
+        }
+        ASSERT_EQUALS("", errout_str());
+    }
+
+    void sdccAsmOtherSyntax() {
+        const char code[] = "void f() {\n"
+                            "    __asm (\"nop\");\n"
+                            "    __asm\n"
+                            "        (\"nop\");\n"
+                            "    __asm volatile (\"nop\");\n"
+                            "    __asm goto (\"\" : : : : label);\n"
+                            "label: ;\n"
+                            "    __asm { nop }\n"
+                            "}\n"
+                            "__asm void g(void) {\n"
+                            "    nop\n"
+                            "}\n";
+        ASSERT_EQUALS("void f ( ) {\n__asm ( \"nop\" ) ;\n__asm\n( \"nop\" ) ;\n"
+                      "__asm volatile ( \"nop\" ) ;\n__asm goto ( \"\" : : : : label ) ;\n"
+                      "label : ;\n__asm { nop }\n}\n__asm void g ( void ) {\nnop\n}",
+                      getcode(settings0, *this, code).at(""));
+        ASSERT_EQUALS("\nvoid f ( ) { $__asm ( \"nop\" ) ; }",
+                      getcode(settings0, *this, "#define ASM __asm\nvoid f() { ASM(\"nop\"); }").at(""));
+        ASSERT_EQUALS("", errout_str());
+    }
+
+    void sdccAsmConditional() {
+        const char code[] = "void f() {\n"
+                            "#ifdef USE_ASM\n"
+                            "    __asm\n"
+                            "        mov b,#(s_XINIT>>8)\n"
+                            "        movx @dptr,a\n"
+                            "    __endasm;\n"
+                            "#else\n"
+                            "    fallback();\n"
+                            "#endif\n"
+                            "    after();\n"
+                            "}\n";
+        ASSERT_EQUALS("void f ( ) {\n\nasm ( )\n\n\n;\n\n\n\nafter ( ) ;\n}",
+                      getcodeforcfg(settings0, *this, code, "USE_ASM", "file.c"));
+        ASSERT_EQUALS("void f ( ) {\n\n\n\n\n\n\nfallback ( ) ;\n\nafter ( ) ;\n}",
+                      getcodeforcfg(settings0, *this, code, "", "file.c"));
+        ASSERT_EQUALS("", errout_str());
+    }
+
+    void sdccAsmControlFlow() {
+        const char code[] = "void f(int x) {\n"
+                            "    if (x)\n"
+                            "        __asm\n"
+                            "            nop\n"
+                            "        __endasm;\n"
+                            "    else\n"
+                            "        fallback();\n"
+                            "    do\n"
+                            "        __asm\n"
+                            "            nop\n"
+                            "        __endasm /* comment */;\n"
+                            "    while (x);\n"
+                            "}\n";
+        ASSERT_EQUALS("void f ( int x ) {\nif ( x )\nasm ( )\n\n;\nelse\nfallback ( ) ;\n"
+                      "do\nasm ( )\n\n;\nwhile ( x ) ;\n}", getcode(settings0, *this, code).at(""));
+        ASSERT_EQUALS("", errout_str());
+    }
+
     void endifsemicolon() {
         const char filedata[] = "void f() {\n"
                                 "#ifdef A\n"