Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Zingam opened this issue Aug 4, 2022 · 8 comments
Closed
Assignees
Labels
bug Feature: Code Analysis Related to integration with clang-tidy, cppcheck, cl.exe /analyze, etc. fixed Check the Milestone for the release in which the fix is or will be available. Language Service quick fix verified Bug has been reproduced
Milestone

Comments

@Zingam
Copy link

Zingam commented Aug 4, 2022

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
llvm/llvm-project#56669

As @sean-mcmanus suggested that it maybe a VScode extension bug I'm filing it again. #9322 (comment)

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

@sean-mcmanus sean-mcmanus self-assigned this Aug 4, 2022
@sean-mcmanus sean-mcmanus added bug Language Service Feature: Code Analysis Related to integration with clang-tidy, cppcheck, cl.exe /analyze, etc. labels Aug 4, 2022
@sean-mcmanus sean-mcmanus added this to the 1.12 milestone Aug 4, 2022
@sean-mcmanus
Copy link
Contributor

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 sean-mcmanus added more info needed The issue report is not actionable in its current state not reproing We're not able to reproduce the issue (it's unlikely to get fixed until we find one). labels Aug 4, 2022
@H-G-Hristov
Copy link

H-G-Hristov commented Aug 15, 2022

---
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
Copy link

A minimal project:

Archive.zip

@H-G-Hristov
Copy link

@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
Copy link

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

@sean-mcmanus sean-mcmanus removed more info needed The issue report is not actionable in its current state not reproing We're not able to reproduce the issue (it's unlikely to get fixed until we find one). labels Aug 15, 2022
@sean-mcmanus
Copy link
Contributor

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

@sean-mcmanus sean-mcmanus added verified Bug has been reproduced fixed Check the Milestone for the release in which the fix is or will be available. labels Aug 15, 2022
@sean-mcmanus sean-mcmanus modified the milestones: 1.12, 1.12.2 Aug 15, 2022
@sean-mcmanus
Copy link
Contributor

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

@sean-mcmanus
Copy link
Contributor

@github-actions github-actions bot locked and limited conversation to collaborators Oct 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Feature: Code Analysis Related to integration with clang-tidy, cppcheck, cl.exe /analyze, etc. fixed Check the Milestone for the release in which the fix is or will be available. Language Service quick fix verified Bug has been reproduced
Projects
None yet
Development

No branches or pull requests

3 participants