Menu

false positives for [type var = var;] definition

vilox
2021-01-04
2021-01-11
  • vilox

    vilox - 2021-01-04

    Hi. The code like
    int n = n;
    triggers two messages from cppcheck (current stable version 2.3):
    filename.c:326:11: warning: Redundant assignment of 'n' to itself. [selfAssignment]
    int n = n;

    filename.c:326:13: error: Uninitialized variable: n [uninitvar]
    int n = n;

    However, int n = n; is a standard idiom to tell the compiler that variable is not initialized, but this is intended. Typically, this idiom has even been abstracted as macro. E.g., in linux kernel:
    include/linux/compiler-gcc.h:69:#define uninitialized_var(x) x = x

    So, de facto cppcheck notions above are false positives.
    Is it possible to teach cppcheck to recognize and respect this idiom to avoid false positives?

    There is a related problem. It is highly desirable to suppress inconsistencies like this directly in macro, say, uninitialized_var, but this is impossible, because cppcheck directives are comments.

    The better approach would be to have something like gcc attributes:

    #define uninitialized_var(x) x = x __cppcheck_attr__((__noinit__))
    int uninitialized_var(n);

     
  • Daniel Marjamäki

    well maybe we can handle some common idiom but I would like to understand why you would do that.

    It does not seem to be a portable idiom. There will be undefined behavior so as far as I see a standard conformant compiler would be allowed to for instance remove all code before and below the variable declaration.

    #define uninitialized_var(x) x = x __cppcheck_attr__((__noinit__))
    int uninitialized_var(n);
    

    well why not:

    #define uninitialized_var(x) x
    int uninitialized_var(n);
    
     
    • vilox

      vilox - 2021-01-06

      well why not:

      #define uninitialized_var(x) x
      int uninitialized_var(n);
      

      because compiler will complain about uninitialized variable. The int n = n is a typical way to tell the compiler: don't worry, it's not a mistake

      It does not seem to be a portable idiom

      There is no portable one, but the int n = n has very decent portability and accepted by many compilers. The notable exception is clang, that's why (linux kernel):
      include/linux/compiler-clang.h:9:#define uninitialized_var(x) x = *(&(x))

      But I'd like to emphasize that this particular idiom would not need to be recognized if there was a way to embed cppcheck suppress directive directly in uninitialized_var macro.

      I would like to understand why you would do that

      Do have uninitialized variables? E.g.:

      ...
      int x;
      if (condition)  { x = ... ; }
      /* do not use x */
      if (same condition) { /* use x; */ }
      
       
  • Daniel Marjamäki

    Do have uninitialized variables? E.g.:

    But in that example it seems it would be a false positive to warn about uninitialized variable?

    Are you saying the intention is to suppress some false positive from gcc? Then #define uninitialized_var(x) x seems better. But well I am missing something probably, then it would seem stupid to use that clang macro.

     

    Last edit: Daniel Marjamäki 2021-01-06
  • Daniel Marjamäki

    I would like a better explanation why this is done. I do not understand the point at all.

     
    • vilox

      vilox - 2021-01-06

      Hi, Daniel. The confusion is because two separate issues are mixed together, let me separate them. 1st issue is why [int n =n]. Look at example again:

      int n;
      if (condition_1) { n = ...; }
      /* dare not to use n */
      if (condition_1) {  /* use n */ }
      

      This example is oversimplified and compiler probably won't complain about uninitialized n in 2nd if. But in less obvious cases compiler can complain. The most portable and well-known trick to suppress such complain is a fictive initialization [int n =n].

      It may be used as-is and typically unmistakably identified by occasional reader as warning suppression trick. This is why it is not meaningless for cppcheck to recognize this trick.

      The code that cares about clarity, however, formalizes this trick as a macro. E.g.

      #define uninitialized_var(x) x = x // gcc, Sun C, ...
      #define uninitialized_var(x) x = *(&(x)) // clang
      

      This way has two advantages: a) gives to occasional reader an explicit and unequivocal indication that this variable has no sensible value at this time; b) adds portability, because compiler differences may be encapsulated inside macro definition.

      Here is a 2nd issue of this thread. cppcheck doubts about [int n =n] could also be resolved in such a macro if there was a way to put cppcheck directive in a macro. Even more, if there was such a way, there would not need to recognize [int n =n] trick, because it always can be rewritten as a macro.

      Going further, the ability to put any directive in a macro is priceless. Once upon a time C standard committee was obliged to introduce _Pragma unary operator just to allow #pragma's in macros. Do you remember notorious Sun's lint? It took it's directives from comments: /*FALLTHRU*/, ..., like cppcheck now. Once upon a time even retrograde Sun was obliged to introduce alternative syntax for lint directives, that one that can be macro-generated.

      Back to uninitialized_var, it's a convenient example. Today:

      // cppcheck-suppress [uninitvar,selfAssignment]
      int uninitialized_var(n);
      

      Tomorrow:

      // cppcheck-suppress [uninitvar,selfAssignment]
      // 2nd-analyzer-suppress [uninitvar,selfAssignment]
      int uninitialized_var(n);
      

      The day after tomorrow:

      // cppcheck-suppress [uninitvar,selfAssignment]
      // 2nd-analyzer-suppress [uninitvar,selfAssignment]
      // 3rd-analyzer-suppress [uninitvar,selfAssignment]
      int uninitialized_var(n);
      

      Not very neat, right? That's why I suppose the cppcheck need a suppress directive that can be macro-generated. Generally speaking, any directive that can be places in the source code ought to allow generation by macro.

       
  • Daniel Marjamäki

    The day after tomorrow:

    only if 2nd-analyzer and 3rd-analyzer has the same false positives as cppcheck.. that seems very unlikely. If cppcheck has a false positive I would rather see that the false positive is fixed and that you avoid suppressions for that.

    Is it possible to show some actual code that is a "good example" when this is needed?

     
  • Daniel Marjamäki

    I looked in the linux kernel tree.. and it seems that macro is deprecated:
    https://github.com/torvalds/linux/blob/master/scripts/coccinelle/misc/uninitialized_var.cocci

     
    • vilox

      vilox - 2021-01-08

      I looked in the linux kernel tree.. and it seems that macro is deprecated:

      The explanation / motivation for this deprecation is disputable.

      only if 2nd-analyzer and 3rd-analyzer has the same false positives as cppcheck.. that seems very unlikely.

      The question is different: directives of which analyzer must I spread abundantly over the code? "cppcheck, surely" is your response. If every compiler and every analyzer decides so, the code becomes unreadable. It is much friendlier to enable macro-generated directives, so the user can encapsulate compiler/analyzer directives in macros and expand them depending on compiler/analyzer.

      I would rather see that the false positive is fixed

      Unachievable goal. Moreover, false positive must typically be fixed now, and can not wait (and depend on) the fix for analyzer.

      Is it possible to show some actual code that is a "good example" when this is needed?

      I think this particular trick is payed too much attention. The ability to generate directive by macro is highly desirable for reason indicated above.

       
  • Daniel Marjamäki

    I think this particular trick is payed too much attention. The ability to generate directive by macro is highly desirable for reason indicated above.

    I think I finally understand. It's not necesarillyvar = var you want to have.. you just want to have some pattern to suppress cppcheck warnings that can be embedded in a macro.

    I think this sounds acceptable.

    with a cppcheck suppression you can suppress a false positive. but I feel it's an important feature that when the false positive is fixed in cppcheck you will get a warning that the suppression is not needed anymore. I feel I want to have such a feature for the directive also. That means you will not be able to use the same macro for cppcheck and other tools, but you will be able to use similar macros.

    directives of which analyzer must I spread abundantly over the code? "cppcheck, surely" is your response

    actually I do not feel comfortable about such directives. they make the code less readable.

    false positive must typically be fixed now, and can not wait

    I can understand that. But in the long run the tools should be fixed and the directive should be removed. It's just a quick ugly hack.

     

    Last edit: Daniel Marjamäki 2021-01-08
  • Daniel Marjamäki

    If you think I got it now.. I think I can create a ticket about this.

    Do you know if it would be used in some project?

     
    • vilox

      vilox - 2021-01-11

      It's not necesarilly var = var you want to have.. you just want to have some pattern to suppress cppcheck warnings that can be embedded in a macro.

      var = var was what I wanted, but during discussion it quickly became clear that macro-generated directives is much broader and better solution.

      ... you will not be able to use the same macro for cppcheck and other tools

      Why? I am not sure I understand you right. I feel you already have a design that implies that behavior. Could you, please, give more detailed explanation?

      Do you know if it would be used in some project?

      Is "it" == cppcheck? I am trying to understand in which extent cppcheck may be used to estimate a quality of 3rd-party code.

       
  • Daniel Marjamäki

    Could you, please, give more detailed explanation?

    Sorry I do not feel that there is a "robust" suggestion right now.

    If you write some quick hack in your code like:

    int uninitialized_var(x);
    

    such quick hack is ugly and does not make your code more readable.

    What is your suggestion to remove such macros when they are not needed anymore?

    If you write a // cppcheck-suppress uninitvar comment then cppcheck will warn you if that is not needed anymore so you can remove the suppression. If Cppcheck will write such warnings for uninitialized_var also then you can't use uninitialized_var to silence other tools because cppcheck will warn then.

    If you instead use different macros for cppcheck and other tools:

    int uninitialized_var_cppcheck(x);
    int uninitialized_var_other_tool(y);
    

    then there is no such problem. When Cppcheck is fixed you will get a warning that uninitialized_var_cppcheck is redundant and then you can remove it.

    Is "it" == cppcheck?

    "it" is the macro directive. Do you know if some project use such uninitialized_var(x) macro? I ask because Linux has deprecated that.

     

Log in to post a comment.

Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.