Skip to content

Commit

Permalink
fixed #12526 - corrected bailout in valueFlowConditionExpressions()
Browse files Browse the repository at this point in the history
… / restored `valueFlowBailoutIncompleteVar` in daca (#6153)

The bailout was moved out of an inner loop in
a3617fe but it kept the previous
`break`. This caused it to bail out completely instead of just skipping
the function.

References for the added defines:

https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-createeventexa#parameters
https://learn.microsoft.com/en-us/windows/win32/secauthz/well-known-sids
  • Loading branch information
firewave committed Mar 21, 2024
1 parent 69037c9 commit 14c24de
Show file tree
Hide file tree
Showing 16 changed files with 118 additions and 24 deletions.
13 changes: 13 additions & 0 deletions cfg/windows.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -4907,6 +4907,8 @@ HFONT CreateFont(
<not-bool/>
</arg>
</function>
<define name="CREATE_EVENT_INITIAL_SET" value="0x00000002"/>
<define name="CREATE_EVENT_MANUAL_RESET" value="0x00000001"/>
<!--HANDLE WINAPI OpenEvent(
_In_ DWORD dwDesiredAccess,
_In_ BOOL bInheritHandle,
Expand Down Expand Up @@ -5453,6 +5455,17 @@ HFONT CreateFont(
<not-null/>
</arg>
</function>
<define name="SECURITY_NULL_SID_AUTHORITY" value="0"/>
<define name="SECURITY_WORLD_SID_AUTHORITY" value="1"/>
<define name="SECURITY_LOCAL_SID_AUTHORITY" value="2"/>
<define name="SECURITY_CREATOR_SID_AUTHORITY" value="3"/>
<define name="SECURITY_NT_AUTHORITY" value="5"/>
<define name="SECURITY_NULL_RID" value="0"/>
<define name="SECURITY_WORLD_RID" value="0"/>
<define name="SECURITY_LOCAL_RID" value="0"/>
<define name="SECURITY_LOCAL_LOGON_RID" value="1"/>
<define name="SECURITY_CREATOR_OWNER_RID" value="0"/>
<define name="SECURITY_CREATOR_GROUP_RID" value="1"/>
<!--PVOID WINAPI FreeSid(_In_ PSID pSid);-->
<function name="FreeSid">
<noreturn>false</noreturn>
Expand Down
2 changes: 1 addition & 1 deletion lib/tokenize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4251,7 +4251,7 @@ static bool setVarIdParseDeclaration(Token** tok, const VariableMap& variableMap
bracket = true;
} else if (tok2->str() == "::") {
singleNameCount = 0;
} else if (tok2->str() != "*" && tok2->str() != "::" && tok2->str() != "...") {
} else if (tok2->str() != "*" && tok2->str() != "...") {
break;
}
tok2 = tok2->next();
Expand Down
7 changes: 5 additions & 2 deletions lib/valueflow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5334,16 +5334,19 @@ static const Scope* getLoopScope(const Token* tok)
//
static void valueFlowConditionExpressions(const TokenList &tokenlist, const SymbolDatabase& symboldatabase, ErrorLogger *errorLogger, const Settings &settings)
{
if (settings.checkLevel == Settings::CheckLevel::normal)
if (!settings.daca && (settings.checkLevel == Settings::CheckLevel::normal))
return;

for (const Scope * scope : symboldatabase.functionScopes) {
if (const Token* incompleteTok = findIncompleteVar(scope->bodyStart, scope->bodyEnd)) {
if (settings.debugwarnings)
bailoutIncompleteVar(tokenlist, errorLogger, incompleteTok, "Skipping function due to incomplete variable " + incompleteTok->str());
break;
continue;
}

if (settings.daca && (settings.checkLevel == Settings::CheckLevel::normal))
continue;

for (Token* tok = const_cast<Token*>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
if (!Token::simpleMatch(tok, "if ("))
continue;
Expand Down
1 change: 1 addition & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ if (BUILD_TESTS)
--suppress=valueFlowBailout
--suppress=purgedConfiguration
--suppress=unmatchedSuppression
--suppress=checkersReport
${CMAKE_CURRENT_SOURCE_DIR}/cfg/${CFG_TEST}
)
endif()
Expand Down
2 changes: 2 additions & 0 deletions test/cfg/gnu.c
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ void memleak_xmalloc()

void memleak_mmap()
{
// cppcheck-suppress valueFlowBailoutIncompleteVar
const void * p_mmap = mmap(NULL, 1, PROT_NONE, MAP_ANONYMOUS | MAP_SHARED, -1, 0);
printf("%p", p_mmap);
// cppcheck-suppress memleak
Expand Down Expand Up @@ -466,6 +467,7 @@ int nullPointer_epoll_ctl(int epfd, int op, int fd, struct epoll_event *event)
// Remove (deregister) the target file descriptor fd from the
// epoll instance referred to by epfd. The event is ignored and
// can be NULL.
// cppcheck-suppress valueFlowBailoutIncompleteVar
return epoll_ctl(epfd, EPOLL_CTL_DEL, fd, NULL);
}
#endif
2 changes: 2 additions & 0 deletions test/cfg/googletest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ TEST(test_cppcheck, cppcheck)
// #9964 - avoid compareBoolExpressionWithInt false positive
TEST(Test, assert_false_fp)
{
// cppcheck-suppress valueFlowBailoutIncompleteVar
ASSERT_FALSE(errno < 0);
}

Expand All @@ -73,6 +74,7 @@ TEST(Test, warning_in_assert_macros)
// cppcheck-suppress duplicateExpression
ASSERT_GE(i, i);

// cppcheck-suppress valueFlowBailoutIncompleteVar
unsigned int u = errno;
// cppcheck-suppress [unsignedPositive]
ASSERT_GE(u, 0);
Expand Down
7 changes: 5 additions & 2 deletions test/cfg/gtk.c
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ void g_new0_test()
int b;
};
// valid
// cppcheck-suppress valueFlowBailoutIncompleteVar
struct a * pNew1 = g_new0(struct a, 5);
printf("%p", pNew1);
g_free(pNew1);
Expand All @@ -320,6 +321,7 @@ void g_try_new_test()
int b;
};
// valid
// cppcheck-suppress valueFlowBailoutIncompleteVar
struct a * pNew1 = g_try_new(struct a, 5);
printf("%p", pNew1);
g_free(pNew1);
Expand All @@ -337,6 +339,7 @@ void g_try_new0_test()
int b;
};
// valid
// cppcheck-suppress valueFlowBailoutIncompleteVar
struct a * pNew1 = g_try_new0(struct a, 5);
printf("%p", pNew1);
g_free(pNew1);
Expand All @@ -354,7 +357,7 @@ void g_renew_test()
struct a {
int b;
};
// cppcheck-suppress leakReturnValNotUsed
// cppcheck-suppress [leakReturnValNotUsed,valueFlowBailoutIncompleteVar]
g_renew(struct a, NULL, 1);

struct a * pNew = g_new(struct a, 1);
Expand All @@ -369,7 +372,7 @@ void g_try_renew_test()
struct a {
int b;
};
// cppcheck-suppress leakReturnValNotUsed
// cppcheck-suppress [leakReturnValNotUsed,valueFlowBailoutIncompleteVar]
g_try_renew(struct a, NULL, 1);

struct a * pNew = g_try_new(struct a, 1);
Expand Down
2 changes: 1 addition & 1 deletion test/cfg/opencv2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,6 @@ void ignoredReturnValue()
void memleak()
{
const char * pBuf = (char *)cv::fastMalloc(1000); // cppcheck-suppress cstyleCast
std::cout << pBuf;
std::cout << pBuf; // cppcheck-suppress valueFlowBailoutIncompleteVar
// cppcheck-suppress memleak
}
12 changes: 9 additions & 3 deletions test/cfg/posix.c
Original file line number Diff line number Diff line change
Expand Up @@ -1062,6 +1062,7 @@ void * memleak_mmap2() // #8327
void memleak_getline() { // #11043
char *line = NULL;
size_t size = 0;
// cppcheck-suppress valueFlowBailoutIncompleteVar
getline(&line, &size, stdin);
// cppcheck-suppress memleak
line = NULL;
Expand All @@ -1082,6 +1083,7 @@ void memleak_getline_array(FILE* stream) { // #12498
void memleak_getdelim(int delim) {
char *line = NULL;
size_t size = 0;
// cppcheck-suppress valueFlowBailoutIncompleteVar
getdelim(&line, &size, delim, stdin);
// cppcheck-suppress memleak
line = NULL;
Expand All @@ -1101,6 +1103,7 @@ void memleak_getdelim_array(FILE* stream, int delim) {

void * identicalCondition_mmap(int fd, size_t size) // #9940
{
// cppcheck-suppress valueFlowBailoutIncompleteVar
void* buffer = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
if (buffer == MAP_FAILED) {
return NULL;
Expand All @@ -1113,6 +1116,7 @@ int munmap_no_double_free(int tofd, // #11396
size_t len)
{
int rc;
// cppcheck-suppress valueFlowBailoutIncompleteVar
const void* fptr = mmap(NULL,len,PROT_READ|PROT_WRITE,MAP_SHARED,fromfd,(off_t)0);
if (fptr == MAP_FAILED) {
return -1;
Expand Down Expand Up @@ -1147,6 +1151,7 @@ void resourceLeak_fdopen(int fd)

void resourceLeak_fdopen2(const char* fn) // #2767
{
// cppcheck-suppress valueFlowBailoutIncompleteVar
int fi = open(fn, O_RDONLY);
FILE* fd = fdopen(fi, "r");
fclose(fd);
Expand Down Expand Up @@ -1193,14 +1198,14 @@ void resourceLeak_socket(void)

void resourceLeak_open1(void)
{
// cppcheck-suppress unreadVariable
// cppcheck-suppress [unreadVariable,valueFlowBailoutIncompleteVar]
int fd = open("file", O_RDWR | O_CREAT);
// cppcheck-suppress resourceLeak
}

void resourceLeak_open2(void)
{
// cppcheck-suppress unreadVariable
// cppcheck-suppress [unreadVariable,valueFlowBailoutIncompleteVar]
int fd = open("file", O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
// cppcheck-suppress resourceLeak
}
Expand All @@ -1213,6 +1218,7 @@ void noleak(int x, int y, int z)
closedir(p2);
int s = socket(AF_INET,SOCK_STREAM,0);
close(s);
// cppcheck-suppress valueFlowBailoutIncompleteVar
int fd1 = open("a", O_RDWR | O_CREAT);
close(fd1);
int fd2 = open("a", O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
Expand Down Expand Up @@ -1357,7 +1363,7 @@ void timet_h(const struct timespec* ptp1)
clockid_t clk_id1, clk_id2, clk_id3;
// cppcheck-suppress constVariablePointer
struct timespec* ptp;
// cppcheck-suppress uninitvar
// cppcheck-suppress [uninitvar,valueFlowBailoutIncompleteVar]
clock_settime(CLOCK_REALTIME, ptp);
// cppcheck-suppress uninitvar
clock_settime(clk_id1, ptp);
Expand Down
2 changes: 1 addition & 1 deletion test/cfg/qt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ void validCode(int * pIntPtr, QString & qstrArg, double d)

printf(QT_TR_NOOP("Hi"));

// cppcheck-suppress checkLibraryFunction
// cppcheck-suppress [checkLibraryFunction,valueFlowBailoutIncompleteVar]
Q_DECLARE_LOGGING_CATEGORY(logging_category_test);
QT_FORWARD_DECLARE_CLASS(forwardDeclaredClass);
QT_FORWARD_DECLARE_STRUCT(forwardDeclaredStruct);
Expand Down
2 changes: 1 addition & 1 deletion test/cfg/runtests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ CFG="$DIR"../../cfg/
# TODO: remove missingInclude disabling when it no longer is implied by --enable=information
# Cppcheck options
# need to suppress unmatchedSuppression in case valueFlowBailout is not reported
CPPCHECK_OPT='--check-library --platform=unix64 --enable=style,information --inconclusive --force --check-level=exhaustive --error-exitcode=-1 --disable=missingInclude --inline-suppr --template="{file}:{line}:{severity}:{id}:{message}" --debug-warnings --suppress=valueFlowBailout --suppress=purgedConfiguration --suppress=unmatchedSuppression'
CPPCHECK_OPT='--check-library --platform=unix64 --enable=style,information --inconclusive --force --check-level=exhaustive --error-exitcode=-1 --disable=missingInclude --inline-suppr --template="{file}:{line}:{severity}:{id}:{message}" --debug-warnings --suppress=valueFlowBailout --suppress=purgedConfiguration --suppress=unmatchedSuppression --suppress=checkersReport'

# Compiler settings
CXX=g++
Expand Down
8 changes: 8 additions & 0 deletions test/cfg/std.c
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ void nullpointer(int value)
fp = 0;
// No FP
fflush(0); // If stream is a null pointer, all streams are flushed.
// cppcheck-suppress valueFlowBailoutIncompleteVar
fp = freopen(0,"abc",stdin);
fclose(fp);
fp = NULL;
Expand Down Expand Up @@ -732,6 +733,7 @@ void uninitvar_fgets(void)
char *str;
int n;

// cppcheck-suppress valueFlowBailoutIncompleteVar
fgets(buf,10,stdin);

// cppcheck-suppress uninitvar
Expand All @@ -749,6 +751,7 @@ void uninitvar_fputc(void)
int i;
FILE *fp;

// cppcheck-suppress valueFlowBailoutIncompleteVar
fputc('a', stdout);

// cppcheck-suppress uninitvar
Expand All @@ -763,6 +766,7 @@ void uninitvar_fputs(void)
const char *s;
FILE *fp;

// cppcheck-suppress valueFlowBailoutIncompleteVar
fputs("a", stdout);

// cppcheck-suppress uninitvar
Expand Down Expand Up @@ -3456,6 +3460,7 @@ void invalidFunctionArg_log10(float f, double d, const long double ld)
(void)log10f(0.0f);
(void)log10f(1.4013e-45f); // note: calculated by nextafterf(0.0f, 1.0f);
(void)log10f(f);
// cppcheck-suppress valueFlowBailoutIncompleteVar
(void)log10f(FLT_MAX);

// cppcheck-suppress invalidFunctionArg
Expand All @@ -3480,6 +3485,7 @@ void invalidFunctionArg_log(float f, double d, const long double ld)
(void)logf(0.0f);
(void)logf(1.4013e-45f); // note: calculated by nextafterf(0.0f, 1.0f);
(void)logf(f);
// cppcheck-suppress valueFlowBailoutIncompleteVar
(void)logf(FLT_MAX);

// cppcheck-suppress invalidFunctionArg
Expand All @@ -3504,6 +3510,7 @@ void invalidFunctionArg_log2(float f, double d, const long double ld)
(void)log2f(0.0f);
(void)log2f(1.4013e-45f); // note: calculated by nextafterf(0.0f, 1.0f);
(void)log2f(f);
// cppcheck-suppress valueFlowBailoutIncompleteVar
(void)log2f(FLT_MAX);

// cppcheck-suppress invalidFunctionArg
Expand Down Expand Up @@ -4973,6 +4980,7 @@ void invalidPrintfArgType_printf(void)
// #7016
uint8_t n = 7;
// TODO cppcheck-suppress invalidPrintfArgType_uint
// cppcheck-suppress valueFlowBailoutIncompleteVar
printf("%" PRIi16 "\n", n);
}

Expand Down
16 changes: 11 additions & 5 deletions test/cfg/std.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -662,8 +662,10 @@ void memleak_localtime_s(const std::time_t *restrict time, struct tm *restrict r
{
const time_t t = time(0);
const struct tm* const now = new tm();
if (localtime_s(now, &t) == 0)
if (localtime_s(now, &t) == 0) {
// cppcheck-suppress valueFlowBailoutIncompleteVar
std::cout << now->tm_mday << std::endl;
}
// cppcheck-suppress memleak
}
#endif // __STDC_LIB_EXT1__
Expand Down Expand Up @@ -706,6 +708,7 @@ void invalidFunctionArg_std_wstring_substr(const std::wstring &str, std::size_t
(void)str.substr(pos,-1);
// no warning is expected for
(void)str.substr(pos,len);
// cppcheck-suppress valueFlowBailoutIncompleteVar
(void)str.substr(pos, std::wstring::npos);
}

Expand Down Expand Up @@ -4256,28 +4259,30 @@ void nullPointer_system(const char *c)
void uninitvar_setw(void)
{
int i;
// cppcheck-suppress uninitvar
// cppcheck-suppress [uninitvar,valueFlowBailoutIncompleteVar]
std::cout << std::setw(i);
}

void uninitvar_setiosflags(void)
{
std::ios_base::fmtflags mask;
// TODO cppcheck-suppress uninitvar
// cppcheck-suppress valueFlowBailoutIncompleteVar
std::cout << std::setiosflags(mask); // #6987 - false negative
}

void uninitvar_resetiosflags(void)
{
std::ios_base::fmtflags mask;
// TODO cppcheck-suppress uninitvar
// cppcheck-suppress valueFlowBailoutIncompleteVar
std::cout << std::resetiosflags(mask); // #6987 - false negative
}

void uninitvar_setfill(void)
{
char c;
// cppcheck-suppress uninitvar
// cppcheck-suppress [uninitvar,valueFlowBailoutIncompleteVar]
std::cout << std::setfill(c);

wchar_t wc;
Expand All @@ -4288,14 +4293,14 @@ void uninitvar_setfill(void)
void uninitvar_setprecision(void)
{
int p;
// cppcheck-suppress uninitvar
// cppcheck-suppress [uninitvar,valueFlowBailoutIncompleteVar]
std::cout << std::setprecision(p);
}

void uninitvar_setbase(void)
{
int p;
// cppcheck-suppress uninitvar
// cppcheck-suppress [uninitvar,valueFlowBailoutIncompleteVar]
std::cout << std::setbase(p);
}

Expand Down Expand Up @@ -4732,6 +4737,7 @@ void stdbind_helper(int a)

void stdbind()
{
// cppcheck-suppress valueFlowBailoutIncompleteVar
using namespace std::placeholders;

// TODO cppcheck-suppress ignoredReturnValue #9369
Expand Down
Loading

0 comments on commit 14c24de

Please sign in to comment.