vscode-cpptools icon indicating copy to clipboard operation
vscode-cpptools copied to clipboard

[clang-tidy] Generates invalid code - adds invalid escapes to string literals

Open Zingam opened this issue 1 year ago • 1 comments

Environment

  • OS and version: Ubuntu 22.04
  • VS Code: 1.69.2
  • C/C++ extension: 1.11.4
  • OS and version of remote machine (if applicable):

I filed this bug agains Clang-Tidy here https://github.com/llvm/llvm-project/issues/56669

As @sean-mcmanus suggested that it maybe a VScode extension bug I'm filing it again. https://github.com/microsoft/vscode-cpptools/issues/9322#issuecomment-1192862172

Bug Summary and Steps to Reproduce

Bug Summary:

https://user-images.githubusercontent.com/47526411/180406454-7ad4fedd-813a-447b-bcb6-e8f95834c04e.png

The following was automatically fixed by Clang-Tidy and reformatted with clang-format:

void Config::CheckModified() {
    struct stat fileinfo;
    time_t mytime;
    char Buf[MAX_FILENAME];

    if (stat(szConfigName, &fileinfo) == -1) {
        fprintf(stderr, "CheckModified() Error: Cannot stat [%s]!\n", szConfigName);
        return;
    } else {
        mytime = fileinfo.st_mtime;
        if (mytime > m_LastModifiedTime) {
            snprintf(Buf, MAX_FILENAME, "%s", szConfigName);
            // BugID 7073: Suppressing output to stderr, it seems to be bothering dave.
            // fprintf(stderr, "CheckModified() Reloading Configuration [%s]\n", Buf);
            ResetState();
            LoadConfig(Buf);
        }
    }
}

Invalid output:

void Config::CheckModified() {
    struct stat fileinfo;
    time_t mytime = 0;
    char Buf[MAX_FILENAME];

    if (stat(szConfigName, &fileinfo) == -1) {
        fprintf(stderr, "CheckModified() Error: Cannot stat [%s]!\n", szConfigName);
        return;
    }
    mytime = fileinfo.st_mtime;
    if (mytime > m_LastModifiedTime) {
            snprintf(Buf, MAX_FILENAME, \"%s\", szConfigName);
            // BugID 7073: Suppressing output to stderr, it seems to be bothering dave.
            // fprintf(stderr, \"CheckModified() Reloading Configuration [%s]\
\", Buf);
            ResetState();
            LoadConfig(Buf);
    }
}

Steps to reproduce:

  1. In this environment...
  2. With this config...
  3. Do '...'
  4. See error...

Other Extensions

No response

Additional Information

No response

Zingam avatar Aug 04 '22 04:08 Zingam

What clang-tidy check is being run exactly? Are you able to provide an isolated repro sample code? Does the bug require formatting or not?

From the thread on LLVM, it sounds like it's a clang-tidy bug? Our extension invokes clang-tidy and processes the fix yaml -- so either we're getting invalid fixes from clang-tidy or there's a bug in processing of those fixes.

sean-mcmanus avatar Aug 04 '22 17:08 sean-mcmanus

---
AnalyzeTemporaryDtors: false
FormatStyle:           none
HeaderFilterRegex:     'agent/[^/]*\.(hpp|cpp|hxx|cxx)$|library/[^/]*\.(hpp|cpp|hxx|cxx)$|initial-validate/[^/]*\.(hpp|cpp|hxx|cxx)$|modules/[^/]*\.(hpp|cpp|hxx|cxx)$'
User:                  user
WarningsAsErrors:      false
# Checks order:
# 1. Disable all default checks `-*`
# 2. Enable all desired checks `-<check_name_group>-*`
# 3. Disable specific checks with `-<check_name>`
Checks: >
  -*,
  boost-*,
  bugprone-*,
  cert-*,
  clang-analyzer-*,
  clang-diagnostic-*,
  concurrency-*,
  cppcoreguidelines-*,
  google-*,
  hicpp-*,
  misc-*,
  modernize-*,
  performance-*,
  portability-*,
  readability-*,
  -modernize-use-trailing-return-type,
  -readability-redundant-access-specifiers,
CheckOptions:
  - key:             cert-dcl16-c.NewSuffixes
    value:           'L;LL;LU;LLU'
  - key:             cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField
    value:           'false'
  - key:             cert-str34-c.DiagnoseSignedUnsignedCharComparisons
    value:           'false'
  - key:             google-readability-namespace-comments.ShortNamespaceLines
    value:           '10'
  - key:             cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors
    value:           'true'
  - key:             cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor
    value:           'true' 
  - key:             cppcoreguidelines-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic
    value:           'true'
  - key:             google-readability-braces-around-statements.ShortStatementLines
    value:           '1'
  - key:             google-readability-function-size.StatementThreshold
    value:           '800'
  - key:             google-readability-namespace-comments.SpacesBeforeComments
    value:           '2'
  - key:             llvm-else-after-return.WarnOnConditionVariables
    value:           'false'
  - key:             llvm-else-after-return.WarnOnUnfixable
    value:           'false'
  - key:             llvm-qualified-auto.AddConstToQualified
    value:           'false'
  - key:             misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic
    value:           'true'
  - key:             modernize-loop-convert.MaxCopySize
    value:           '16'
  - key:             modernize-loop-convert.MinConfidence
    value:           reasonable
  - key:             modernize-loop-convert.NamingStyle
    value:           CamelCase
  - key:             modernize-pass-by-value.IncludeStyle
    value:           llvm
  - key:             modernize-replace-auto-ptr.IncludeStyle
    value:           llvm
  - key:             modernize-use-nullptr.NullMacros
    value:           'NULL'
...

Applying the above on:

#define MAX_FILENAME 2
char* szConfigName;

void ResetState() {}
void LoadConfig(char** buf) {(void) buf; }

void CheckModified() {
    struct stat fileinfo;
    time_t mytime;
    char Buf[MAX_FILENAME];

    if (stat(szConfigName, &fileinfo) == -1) {
        fprintf(stderr, "CheckModified() Error: Cannot stat [%s]!\n", szConfigName);
        return;
    } else {
        mytime = fileinfo.st_mtime;
        if (mytime > 1) {
            snprintf(Buf, MAX_FILENAME, "%s", szConfigName);
            // BugID 7073: Suppressing output to stderr, it seems to be bothering dave.
            // fprintf(stderr, "CheckModified() Reloading Configuration [%s]\n", Buf);
            ResetState();
            LoadConfig(Buf);
        }
    }
}

produces:

#define MAX_FILENAME 2
char* szConfigName;

void ResetState() {}
void LoadConfig(char** buf) {(void) buf; }

void CheckModified() {
    struct stat fileinfo;
    time_t mytime = 0;
    char Buf[MAX_FILENAME];

    if (stat(szConfigName, &fileinfo) == -1) {
        fprintf(stderr, "CheckModified() Error: Cannot stat [%s]!\n", szConfigName);
        return;
    }         mytime = fileinfo.st_mtime;
        if (mytime > 1) {
            snprintf(Buf, MAX_FILENAME, \"%s\", szConfigName);
            // BugID 7073: Suppressing output to stderr, it seems to be bothering dave.
            // fprintf(stderr, \"CheckModified() Reloading Configuration [%s]\
\", Buf);
            ResetState();
            LoadConfig(Buf);
        }
   
}

H-G-Hristov avatar Aug 15 '22 10:08 H-G-Hristov

A minimal project:

Archive.zip

H-G-Hristov avatar Aug 15 '22 12:08 H-G-Hristov

@sean-mcmanus I looks like a VSCode issue to me. I run this: clang-tidy -fix main.cpp and I got this:

#include <cstdio>
#include <sys/stat.h>

#define MAX_FILENAME 2
char* szConfigName;

void ResetState() {}
void LoadConfig(const char* buf) {(void) buf; }

void CheckModified(int i) {
    struct stat fileinfo{};
    time_t mytime = 0;
    char Buf[MAX_FILENAME];

    if (stat(szConfigName, &fileinfo) == -1) {
        fprintf(stderr, "CheckModified() Error: Cannot stat [%s]!\n", szConfigName);
        return;
    }         mytime = fileinfo.st_mtime;
        if (mytime > i) {
            snprintf(Buf, MAX_FILENAME, "%s", szConfigName);
            // BugID 7073: Suppressing output to stderr, it seems to be bothering dave.
            // fprintf(stderr, "CheckModified() Reloading Configuration [%s]\n", Buf);
            ResetState();
            LoadConfig(Buf);
        }
   
}

int main() {
    int iiiiii = 0;
    CheckModified(iiiiii);
}

This is an updated version: Archive.zip

H-G-Hristov avatar Aug 15 '22 12:08 H-G-Hristov

@sean-mcmanus I applied all fixes but applying the fix readability-else-after-return produced the same issue.

H-G-Hristov avatar Aug 15 '22 12:08 H-G-Hristov

@H-G-Hristov Thanks, I got the repro.

sean-mcmanus avatar Aug 15 '22 18:08 sean-mcmanus

The fix should be in our next insiders release (next week).

sean-mcmanus avatar Aug 17 '22 00:08 sean-mcmanus

Fixed with https://github.com/microsoft/vscode-cpptools/releases/tag/v1.12.2 .

sean-mcmanus avatar Aug 25 '22 19:08 sean-mcmanus