Analysis failed about data type "BYTE"(cppcheck x64 1.84)

[Eita Karasawa]

I have an issue about data type "BYTE".

For example, I catch the following warning(style) from cppcheck with following code.
<error id="unsignedPositive" severity="style" msg="Unsigned variable 'hoge' can't be negative so it is unnecessary to test it." verbose="Unsigned variable 'hoge' can't be negative so it is unnecessary to test it." cwe="570">

<Hoge.cpp>

BYTE hoge = 0;
if ( 0 <= hoge ) {
    return;
}

I have multiple platforms.
For each platform, I define "BYTE" as follows.

<DataTypeDefs.h> (included by precompiled header)

#if defined(PLATFORM_0)
typedef char BYTE;
#elif defined(PLATFORM_1)
// On this platform, "char" is treated as "unsigned data type".
// "int8_t" is defined by PLATFORM_1's SDK.
typedef int8_t BYTE; 
#else
#define __int8 BYTE
#endif

I tried stopping using "DataTypeDefs.h" header and defined "BYTE" directly as follows.
The error was no longer reported.

<Hoge2.cpp>

#if defined(PLATFORM_0)
typedef char BYTE;
#elif defined(PLATFORM_1)
// On this platform, "char" is treated as "unsigned data type".
// "int8_t" is defined by PLATFORM_1's SDK.
typedef int8_t BYTE; 
#else
#define __int8 BYTE
#endif

BYTE hoge = 0;
if ( 0 <= hoge ) {
    return;
}

Please let me know how to fix this warning.

[Eita Karasawa]

There were the following differences from ver1.83.

[ case1 : with variable ]

BYTE hoge = 0;
if ( 0 <= hoge ) {
    return;
}

ver1.83 -> warning
ver1.84 -> warning


[ case2 : with function ]

BYTE GetHoge()
{
    return 0;
}

if ( 0 <= GetHoge() ) {
    return;
}

ver1.83 -> no warning
ver1.84 -> warning

[versat]

Without a reduced example and the exact command line it is a bit hard for me to say why it is exactly how it is and what to do.
But i can share some thoughts about it:
Depending on the platform that Cppcheck uses, char could be unsigned or not.
For example for this C code:

int f(char a)
{
    return (0 <= a);
}

Cppcheck outputs different messages depending on the platform (signed/not specified or unsigned):

$ ./cppcheck --enable=all --platform=unix32 char_un_signed.c
Checking char_un_signed.c ...
[char_un_signed.c:1]: (style) The function 'f' is never used.

$ ./cppcheck --enable=all --platform=unix32-unsigned char_un_signed.c
Checking char_un_signed.c ...
[char_un_signed.c:3]: (style) Unsigned variable 'a' can't be negative so it is unnecessary to test it.
[char_un_signed.c:1]: (style) The function 'f' is never used.

Maybe that helps a bit.

[Eita Karasawa]

Thank you for your response!

I have following simple example and command about this issue.

[main.cpp]

BYTE GetHoge()
{
    std::random_device rnd;
    std::mt19937 mt(rnd());
    std::uniform_int_distribution<> rand100(0, 99);
    return (rand100(mt) - 50);  // -50~49
}

int main()
{
    if (0 <= GetHoge())
    {
        return -1;
    }

    return 0;
}

[command & result]

$ ./cppcheck --enable=all --platform=win64 --verbose .\
[main.cpp:11]: (style) Unsigned variable '(' can't be negative so it is unnecessary to test it.

$ ./cppcheck  --enable=all --platform=unix64 --verbose .\
no warning

My environment platform is "win64".

If I use "--platform=unix64" option, there is no warning.
But there seems to be cases where warnings I wanted can not be obtained.

[main2.cpp]

class CHoge
{
public:
    CHoge() {}
    ~CHoge() {}
public:
    BYTE Get() { return hoge; }
    void Add(BYTE add) { hoge += 1; }
private:
    BYTE hoge;
};

BYTE GetHoge()
{
    std::random_device rnd;
    std::mt19937 mt(rnd());
    std::uniform_int_distribution<> rand100(0, 99);
    return (rand100(mt) - 50);  // -50~49
}

int main()
{
    CHoge classHoge;
    classHoge.Add(1);
    if (0 <= classHoge.Get())
    {
        return -2;
    }

    if (0 <= GetHoge())
    {
        return -1;
    }

    return 0;
}

[command & result]

$ ./cppcheck --enable=all --platform=win64 --verbose .\
[main.cpp:25]: (style) Unsigned variable '(' can't be negative so it is unnecessary to test it.
[main.cpp:30]: (style) Unsigned variable '(' can't be negative so it is unnecessary to test it.
[main.cpp:4]: (warning) Member variable 'CHoge::hoge' is not initialized in the constructor.

$ ./cppcheck  --enable=all --platform=unix64 --verbose .\
no warning

I'd like to catch "not initialized in the constructor" warning and use "win64" option.

[versat]

If you specify a Windows platform (for example with --platform=win64) Cppcheck automatically loads the library windows.cfg.
In windows.cfg BYTE is specified as an unsigned datatype with the size of a char.
For other platforms BYTE is not specified, so Cppcheck can not make any assumptions about it (unless you provide any information in some way) and thus will not warn about issues like the "Unsigned variable XYZ can not be negative".
From Cppcheck s point of view BYTE could even be a class and in that case an initialization is not necessary (at least not always). So it does not warn about "Member variable XYZ is not initialized in the constructor" too. Otherwise you would get many false positives because Cppcheck would assume anything that could easily be wrong. And one of Cppcheck s goals is to avoid as many false positives as possible.

[Eita Karasawa]

I see.
In windows.cfg, BYTE is specified as an unsigned char type.
And from the CppCheck side, BYTE is also a class.
Thank you for useful information!

By the way, on some platforms char is treated as unsigned,
so I defined BYTE as a signed data type with following code.

[SDK_header.h] (I can't modify it.)

#define __INT8_TYPE__ signed char
typedef __INT8_TYPE__ __int8_t;
typedef __int8_t int8_t;

[Application_header.h]

typedef int8_t BYTE; // define BYTE as an signed char

However, when examining the source code of the application with CppCheck,
BYTE was judged to be unsigned, and the above warning was reported.

[Application_source.cpp]

BYTE GetHoge()
{
    std::random_device rnd;
    std::mt19937 mt(rnd());
    std::uniform_int_distribution<> rand100(0, 99);
    return (rand100(mt) - 50);  // -50~49
}

int main()
{
    if (0 <= GetHoge()) // reported by cppcheck as "it is unnecessary to test it"
    {
        return -1;
    }

    return 0;
}

I tried to include [SDK_header.h] in --includes - file or --include, but there was still warning.
While keeping platform as win64, Is there a way to prevent this warning from being reported?

[versat]

If Cppcheck sees the typedef int8_t BYTE; it seems to correctly not warn any longer, even when using platform win64.

I've tested it in the following way:
BYTE_issue.h:

typedef int8_t BYTE; // define BYTE as an signed char

BYTE_issue.cpp:

BYTE GetHoge()
{
    std::random_device rnd;
    std::mt19937 mt(rnd());
    std::uniform_int_distribution<> rand100(0, 99);
    return (rand100(mt) - 50);  // -50~49
}

int main()
{
    if (0 <= GetHoge()) // reported by cppcheck as "it is unnecessary to test it"
    {
        return -1;
    }

    return 0;
}

$ ./cppcheck --enable=all --platform=win64 --include=BYTE_issue.h BYTE_issue.cpp
Checking BYTE_issue.cpp ...

$ ./cppcheck --enable=all --platform=win64 BYTE_issue.cpp
Checking BYTE_issue.cpp ...
[BYTE_issue.cpp:11]: (style) Unsigned variable '(' can't be negative so it is unnecessary to test it.

When using --include=BYTE_issue.h there is no warning.
Without the --include=BYTE_issue.h Cppcheck uses the unsigned type from windows.cfg and warns accordingly.

If it still does not work for you with --include=Application_header.h maybe try an absolute path here or temporarily copy the header to the location from which the analysis is started and test if that works. Cppcheck seems to silently ignore issues when files that are specified via --include= do not exist or can not be found (not sure if that is really a good behavior or if there should at least be some warning/information message).

[versat]

I added ticket 8695 Inform the user if a file specified via --include is not found.

[Eita Karasawa]

Thanks a lot!
I tried various tests.
So I guess that this issue may have been reproduced in a minimal environment.

[BYTE_issue.cpp]

//#include "BYTE_issue_OK.h"
#include "BYTE_issue_NG.h"

BYTE GetHoge()
{
    std::random_device rnd;
    std::mt19937 mt(rnd());
    std::uniform_int_distribution<> rand100(0, 99);
    return (rand100(mt) - 50);  // -50~49
}

int main()
{
    if (0 <= GetHoge()) // reported by cppcheck as "it is unnecessary to test it"
    {
        return -1;
    }

    return 0;
}

[BYTE_issue_OK.h] (no warning)

typedef int8_t BYTE; // define BYTE as an signed char

[BYTE_issue_NG.h] (the warning is reported.)

#if defined(PLATFORM_0)
    typedef char BYTE;
#elif defined(PLATFORM_1)
    typedef int8_t BYTE; // define BYTE as an signed char
#else
    #define __int8 BYTE
#endif

[command and result]

$ ./cppcheck --enable=all --platform=win64 --include=BYTE_issue_OK.h BYTE_issue.cpp
Checking BYTE_issue.cpp ...

$ ./cppcheck --enable=all --platform=win64 --include=BYTE_issue_NG.h BYTE_issue.cpp
Checking BYTE_issue.cpp ...
[BYTE_issue.cpp:13]: (style) Unsigned variable '(' can't be negative so it is unnecessary to test it.
Checking BYTE_issue.cpp: PLATFORM_0...
Checking BYTE_issue.cpp: PLATFORM_1...

"PLATFORM_0" and "PLATFORM_1" are defined by Visual Studio Project file(.vcxproj).
Can I fix this issue about my environment?

[versat]

The output is what i would expect and i see no wrong behavior.
If PLATFORM_0 or PLATFORM_1 is defined no warning is shown because BYTE is of signed type.
When no PLATFORM_* is defined BYTE is not changed/defined or so and the default unsigned type of windows.cfg is used.

Can I fix this issue about my environment?

Not sure if i got you. What exactly do you want to fix?
Cppcheck by default tests different combinations of configurations/defines/macros (whatever to call them). And in the example it also tests a configuration where BYTE is unchanged (PLATFORM_0 and PLATFORM_1 are both not defined) and therefore is of unsigned type which results in the warning. As far as i know there is no simple way to just skip the configuration where the PLATFORM_* macros are not defined. You would have to create several projects and define PLATFORM_X different for each project and never let this be undefined.

Btw.: Are you sure it should be #define __int8 BYTE and not #define BYTE __int8 or typedef __int8 BYTE? I can not tell what is correct from the example, but for Windows platforms Cppcheck already assumes __int8 is char (see windows.cfg).

[Eita Karasawa]

I'm sorry.
"#define BYTE __int8" is correct.(of cource same result)

[Eita Karasawa]

What exactly do you want to fix?

I want to treat "BYTE" as a signed char on all platforms and fix all style and warning with Cppcheck(platform=win64).
Is it impossible with windows.cfg (=win64)?

Thank to you, I've just understood that "__int8" is treated as char (=unsigned) according to windows.cfg.
(I refer https://github.com/danmar/cppcheck/blob/master/cfg/windows.cfg)

I thought that "BYTE" is signed data type, because
I use "int8" as signed char and "unsigned int8" as unsigned char in my environment.

[versat]

For Windows platforms __int8 is handled like a signed char (signed because the default for Windows is not unsigned see platform.cpp).

__int8 in windows.cfg:

<platformtype name="__int8" value="char">
    <platform type="win32A"/>
    <platform type="win32W"/>
    <platform type="win64"/>
  </platformtype>

For BYTEthere is explicitly an <unsigned/> element in the configuration.

BYTE in windows.cfg:

  <platformtype name="BYTE" value="char">
    <unsigned/>
    <platform type="win32A"/>
    <platform type="win32W"/>
    <platform type="win64"/>
  </platformtype>

[versat]

With the changed BYTE_issue_NG.h file:

#if defined(PLATFORM_0)
    typedef char BYTE;
#elif defined(PLATFORM_1)
    typedef int8_t BYTE; // define BYTE as an signed char
#else
    #define BYTE __int8
#endif

I do no longer get any warnings:

$ ./cppcheck --enable=all --platform=win64 --include=BYTE_issue_NG.h BYTE_issue.cpp
Checking BYTE_issue.cpp ...
Checking BYTE_issue.cpp: PLATFORM_0...
Checking BYTE_issue.cpp: PLATFORM_1...

In any case BYTE is now a signed type and Cppcheck does no longer issue any message.

[Eita Karasawa]

Certainly, it seems to have been fixed in a minimal environment.
Likewise, I applied it to my development environment as follows.

[Application_header.h]

#if defined(PLATFORM_0)
    typedef char BYTE;
#elif defined(PLATFORM_1)
    typedef int8_t BYTE; // define BYTE as an signed char
#else
    #define BYTE __int8
#endif

[command and result]

$ ./cppcheck --enable=all --platform=win64 --include=C:/source/Application_header.h C:\source
    -> 0 reports

$ ./cppcheck --enable=all --platform=win64 C:/source
    -> 57 reports
    　(This issue still remains and there are reports other than this issue.)

About 50 cases of warning/style were reported before change,
but there were 0 reports after change.
Not only this issue, it seems that all reports are no longer being reported.

Did I make a mistake?

[versat]

Hard to say if something is wrong or what could be wrong.
You can add additional parameters to the command line to see if something could be wrong.
I would first add -v for a bit more verbose output.
If that does not help add --debug(or since Cppcheck 1.85 --debug-normal) instead. There is much output then, but you can see better how Cppcheck sees the code.
If you have very much configurations/macros that are checked you can try -fto force Cppcheck to check all of them (default is checking up to 12 if i remember correctly).

[Eita Karasawa]

I use -v and --debug options but there is no changes.

So I modified command as follows.

$ ./cppcheck --enable=all --platform=win64 --include=./Application_header.h C:\source
    -> 57 reports

When I used the relative path from the check-folder, there were 57 reports.

Also, the include item(Includes:) of the log was blank.

Checking C:\source\test.cpp ...
Defines:
Includes:
Platform:win64

Am I correctly using --include option?

[versat]

When I used the relative path from the check-folder, there were 57 reports.

Strange. I would not expect any difference whether the absolute or relative path is used. I have just quickly done a test and that resulted in the same output.
Does the output from --debugwhich looks like preprocessor output is what you would expect?

Am I correctly using --include option?

It looks correct to me. Header files which are specified via --include= are not listed under "Includes:", only the include paths specified for example via -I are list there as far as i know.

[Eita Karasawa]

Sorry for my late reply.
I have been testing variously.

I could not get the results I expected with --debug option.
There is many report as "severity="debug"" but there is still this issue and no more new information.

I will give up on solving this issue...
Because I can't prepare the minimum environment for this issue and I can't show all my environment publicly to get advice.

I appreciate your cooperation.
Thank you.

[Eita Karasawa]

Additional notes:
I can't fix this issue by "using an absolute path" and "copying SDK_header.h file to my local environment".

[versat]

I did not meant this to be a real fix but only for testing if it changes the behavior and we can see if maybe just the header is not found. But i see that this is not so easy :)

[Eita Karasawa]

I'm sorry that I misunderstood you.
There was not any change even "using an absolute path" and "copying SDK_header.h file to my local environment".

[versat]

Ok, so that is not the problem. I misunderstood you too.