{"action":"created","issue":{"url":"https://api.github.com/repos/danmar/cppcheck/issues/1153","repository_url":"https://api.github.com/repos/danmar/cppcheck","labels_url":"https://api.github.com/repos/danmar/cppcheck/issues/1153/labels{/name}","comments_url":"https://api.github.com/repos/danmar/cppcheck/issues/1153/comments","events_url":"https://api.github.com/repos/danmar/cppcheck/issues/1153/events","html_url":"https://github.com/danmar/cppcheck/pull/1153","id":312168396,"number":1153,"title":"Extend valueflow comparison ops","user":{"login":"pfultz2","id":1306044,"avatar_url":"https://avatars1.githubusercontent.com/u/1306044?v=4","gravatar_id":"","url":"https://api.github.com/users/pfultz2","html_url":"https://github.com/pfultz2","followers_url":"https://api.github.com/users/pfultz2/followers","following_url":"https://api.github.com/users/pfultz2/following{/other_user}","gists_url":"https://api.github.com/users/pfultz2/gists{/gist_id}","starred_url":"https://api.github.com/users/pfultz2/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pfultz2/subscriptions","organizations_url":"https://api.github.com/users/pfultz2/orgs","repos_url":"https://api.github.com/users/pfultz2/repos","events_url":"https://api.github.com/users/pfultz2/events{/privacy}","received_events_url":"https://api.github.com/users/pfultz2/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2018-04-07T04:07:58Z","updated_at":"2018-04-07T14:05:09Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/danmar/cppcheck/pulls/1153","html_url":"https://github.com/danmar/cppcheck/pull/1153","diff_url":"https://github.com/danmar/cppcheck/pull/1153.diff","patch_url":"https://github.com/danmar/cppcheck/pull/1153.patch"},"body":"This will make it work for else clauses, and when the branch has no return. For cases like:\r\n\r\n```cpp\r\nvoid f(int x) {\r\n    if (x < 123) { }\r\n    else { a = x; }\r\n}\r\n```\r\n\r\nand:\r\n\r\n```cpp\r\nvoid f(int x) {\r\n    if (x < 123) { throw \"\"; }\r\n    a = x;\r\n}\r\n```"},"comment":{"url":"https://api.github.com/repos/danmar/cppcheck/issues/comments/379471783","html_url":"https://github.com/danmar/cppcheck/pull/1153#issuecomment-379471783","issue_url":"https://api.github.com/repos/danmar/cppcheck/issues/1153","id":379471783,"user":{"login":"pfultz2","id":1306044,"avatar_url":"https://avatars1.githubusercontent.com/u/1306044?v=4","gravatar_id":"","url":"https://api.github.com/users/pfultz2","html_url":"https://github.com/pfultz2","followers_url":"https://api.github.com/users/pfultz2/followers","following_url":"https://api.github.com/users/pfultz2/following{/other_user}","gists_url":"https://api.github.com/users/pfultz2/gists{/gist_id}","starred_url":"https://api.github.com/users/pfultz2/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pfultz2/subscriptions","organizations_url":"https://api.github.com/users/pfultz2/orgs","repos_url":"https://api.github.com/users/pfultz2/repos","events_url":"https://api.github.com/users/pfultz2/events{/privacy}","received_events_url":"https://api.github.com/users/pfultz2/received_events","type":"User","site_admin":false},"created_at":"2018-04-07T14:05:09Z","updated_at":"2018-04-07T14:05:09Z","author_association":"CONTRIBUTOR","body":"So I ran this over more code, and I have found several array out of bounds.\r\n\r\nphp-src-php-7.1.15/ext/mbstring/libmbfl/filters/mbfilter_sjis_2004.c:337:\r\n```cpp\r\nfor (k = 0; k < jisx0213_p2_ofst_len; k++) {\r\n        if (s1 == jisx0213_p2_ofst[k]-1) {\r\n                break;\r\n        }\r\n}\r\nk = k - (jisx0213_p2_ofst[k]-1);\r\n```\r\n\r\nAccessing `k` outside the loop will be out of bounds.\r\n\r\nqtbase-5.11.0-alpha1/src/3rdparty/xkbcommon/src/xkbcomp/ast-build.c:719:\r\n\r\n```cpp\r\nconst char *\r\nxkb_file_type_to_string(enum xkb_file_type type)\r\n{\r\n    if (type > _FILE_TYPE_NUM_ENTRIES)\r\n        return \"unknown\";\r\n    return xkb_file_type_strings[type];\r\n}\r\n```\r\n\r\nHere the condition should be `type >= _FILE_TYPE_NUM_ENTRIES` otherwise `xkb_file_type_strings` will be out of bounds when `type == _FILE_TYPE_NUM_ENTRIES`.\r\n\r\nllvm/lib/Target/ARM/ARMCallingConv.h:102:\r\n```cpp\r\nunsigned i;\r\nfor (i = 0; i < 2; ++i)\r\n  if (HiRegList[i] == Reg)\r\n    break;\r\n\r\nunsigned T = State.AllocateReg(LoRegList[i]);\r\n```\r\n\r\nIt may be the case that the `break` should always be called(ie either HiRegList[0] or HiRegList[1] is equal to `Reg`). However, an `assert` should be used ensure this is the case. I dont know if cppcheck takes an `assert` into account. I will look into this.\r\n\r\nAlso, I found this, which appears to be a false positive:\r\n\r\n> php-src-php-7.1.15/ext/standard/file.c:362: warning: Either the condition 'if(operation&&errno==EWOULDBLOCK&&wouldblock)' is redundant, otherwise there is negative array index -1. [negativeIndex]\r\n\r\n```cpp\r\nif (act < 1 || act > 3) {\r\n        php_error_docref(NULL, E_WARNING, \"Illegal operation argument\");\r\n        RETURN_FALSE;\r\n}\r\n\r\nif (wouldblock) {\r\n        zval_dtor(wouldblock);\r\n        ZVAL_LONG(wouldblock, 0);\r\n}\r\n\r\n/* flock_values contains all possible actions if (operation & 4) we won't block on the lock */\r\nact = flock_values[act - 1] | (operation & PHP_LOCK_NB ? LOCK_NB : 0);\r\nif (php_stream_lock(stream, act)) {\r\n        if (operation && errno == EWOULDBLOCK && wouldblock) {\r\n                ZVAL_LONG(wouldblock, 1);\r\n        }\r\n        RETURN_FALSE;\r\n}\r\n```\r\n\r\nI am not sure how it arrived to this conclusion."}}
{"actor":{"display_login":"pfultz2"}}