Clang-Tidy finds an issue when using EXPECT_CALL from gmock.

Question

Clang-Tidy finds an issue when using EXPECT_CALL from gmock.

Closed this issue 6 years ago · 6 comments

I am using gmock in my tests and recently started to play around with the clang-tidy static code analysis.
The following issue arises when analyzing a test that uses EXPECT_CALL

knitschi@DevDebianBlank:~/CppLibraries/CppCodeBase$ clang-tidy-3.8 Sources/CodeAssistant/ClassFilesManipulator_unit_tests.cpp -checks=-*,clang-analyzer-*,-clang-analyzer-alpha* -p Generated/LinuxMakeClangNoPCH
1 warning generated.
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1272:5: warning: Use of memory after it is freed [clang-analyzer-cplusplus.NewDelete]
    return function_mocker_->AddNewExpectation(
    ^
/home/knitschi/CppLibraries/CppCodeBase/Sources/CodeAssistant/ClassFilesManipulator_unit_tests.cpp:37:9: note: Calling 'MockSpec::InternalExpectedAt'
        EXPECT_CALL(*mockedModuleInspector, rawFilenameExistsInModule(_)).WillRepeatedly(Return(false)); // this makes sure that files are added
        ^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1845:32: note: expanded from macro 'EXPECT_CALL'
#define EXPECT_CALL(obj, call) GMOCK_EXPECT_CALL_IMPL_(obj, call)
                               ^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1844:5: note: expanded from macro 'GMOCK_EXPECT_CALL_IMPL_'
    ((obj).gmock_##call).InternalExpectedAt(__FILE__, __LINE__, #obj, #call)
    ^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1272:12: note: Calling 'FunctionMockerBase::AddNewExpectation'
    return function_mocker_->AddNewExpectation(
           ^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1609:9: note: Memory is allocated
        new TypedExpectation<F>(this, file, line, source_text, m);
        ^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1615:5: note: Taking false branch
    if (implicit_sequence != NULL) {
    ^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1619:13: note: Calling '~linked_ptr'
    return *expectation;
            ^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gtest/internal/gtest-linked_ptr.h:153:19: note: Calling 'linked_ptr::depart'
  ~linked_ptr() { depart(); }
                  ^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gtest/internal/gtest-linked_ptr.h:205:5: note: Taking true branch
    if (link_.depart()) delete value_;
    ^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gtest/internal/gtest-linked_ptr.h:205:25: note: Memory is released
    if (link_.depart()) delete value_;
                        ^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gtest/internal/gtest-linked_ptr.h:153:19: note: Returning; memory was released
  ~linked_ptr() { depart(); }
                  ^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1619:13: note: Returning from '~linked_ptr'
    return *expectation;
            ^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1272:12: note: Returning; memory was released
    return function_mocker_->AddNewExpectation(
           ^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1272:5: note: Use of memory after it is freed
    return function_mocker_->AddNewExpectation(
    ^

I dont know the gmock and gtest code so I can not tell if this is a real problem. But it bugs me that it "breaks" my otherwise clean analysis result.

Answer 1 · 2016-08-22T15:06:51.000Z

On Sat, Aug 20, 2016 at 11:25 AM, Knitschi notifications@github.com wrote:

I am using gmock in my tests and recently started to play around with the
clang-tidy static code analysis.
The following issue arises when analyzing a test that uses EXPECT_CALL

knitschi@DevDebianBlank:~/CppLibraries/CppCodeBase$ clang-tidy-3.8 Sources/CodeAssistant/ClassFilesManipulator_unit_tests.cpp -checks=-,clang-analyzer-,-clang-analyzer-alpha* -p Generated/LinuxMakeClangNoPCH
1 warning generated.
/home/knitschi/CppLibraries/HunterLibraries/Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1272:5: warning: Use of memory after it is freed [clang-analyzer-cplusplus.NewDelete]
return function_mocker->AddNewExpectation(
^
/home/knitschi/CppLibraries/CppCodeBase/Sources/CodeAssistant/ClassFilesManipulator_unit_tests.cpp:37:9: note: Calling 'MockSpec::InternalExpectedAt'
EXPECT_CALL(*mockedModuleInspector, rawFilenameExistsInModule()).WillRepeatedly(Return(false)); // this makes sure that files are added
^
/home/knitschi/CppLibraries/HunterLibraries/Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1845:32: note: expanded from macro 'EXPECT_CALL'
#define EXPECT_CALL(obj, call) GMOCK_EXPECT_CALL_IMPL(obj, call)
^
/home/knitschi/CppLibraries/HunterLibraries/Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1844:5: note: expanded from macro 'GMOCK_EXPECT_CALL_IMPL'
((obj).gmock##call).InternalExpectedAt(FILE, LINE, #obj, #call)
^
/home/knitschi/CppLibraries/HunterLibraries/Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1272:12: note: Calling 'FunctionMockerBase::AddNewExpectation'
return function_mocker->AddNewExpectation(
^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1609:9: note: Memory is allocated
new TypedExpectation(this, file, line, source_text, m);
^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1615:5: note: Taking false branch
if (implicit_sequence != NULL) {
^
/home/knitschi/CppLibraries/HunterLibraries/Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1619:13: note: Calling '~linked_ptr'
return *expectation;
^
/home/knitschi/CppLibraries/HunterLibraries/Base/a7ba977/f53c868/a43d315/Install/include/gtest/internal/gtest-linked_ptr.h:153:19: note: Calling 'linked_ptr::depart'
~linked_ptr() { depart(); }
^
/home/knitschi/CppLibraries/HunterLibraries/Base/a7ba977/f53c868/a43d315/Install/include/gtest/internal/gtest-linked_ptr.h:205:5: note: Taking true branch
if (link.depart()) delete value;
^
/home/knitschi/CppLibraries/HunterLibraries/Base/a7ba977/f53c868/a43d315/Install/include/gtest/internal/gtest-linked_ptr.h:205:25: note: Memory is released
if (link.depart()) delete value;
^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gtest/internal/gtest-linked_ptr.h:153:19: note: Returning; memory was released
~linked_ptr() { depart(); }
^
/home/knitschi/CppLibraries/HunterLibraries/_Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1619:13: note: Returning from '~linked_ptr'
return *expectation;
^
/home/knitschi/CppLibraries/HunterLibraries/Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1272:12: note: Returning; memory was released
return function_mocker->AddNewExpectation(
^
/home/knitschi/CppLibraries/HunterLibraries/Base/a7ba977/f53c868/a43d315/Install/include/gmock/gmock-spec-builders.h:1272:5: note: Use of memory after it is freed
return function_mocker->AddNewExpectation(
^

I dont know the gmock and gtest code so I can not tell if this is a real
problem. But it bugs me that it "breaks" my otherwise clean analysis result.

This is a false positive in the static analyzer.
It is saying that "expectation" is being freed if it takes the "true"
branch in the destructor of the linked_ptr.
But it is not taking the "true" branch, because the linked_ptr is copied
already into the untyped_expectations_ vector.
We could modify the code to workaround the false positive, but imo the
clang-analyzer-* checks are too noisy and I don't think we should add
complexity to the code to cater to false positives from these checks.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#853, or mute the thread
https://github.com/notifications/unsubscribe-auth/ANcRPYOmy6gX2BV5Qj5-jGNLyfVk48zXks5qhxxsgaJpZM4JpGNt
.

Answer 2 · 2016-08-23T17:43:48.000Z

Ok. Thanks for the reply. I will have to find another solution then.

Answer 3 · 2016-08-26T10:55:05.000Z

Tell this to the clang people. If they agree, they might fix the static analyzer.

Answer 4 · 2016-09-01T17:53:37.000Z

@Bu11etmagnet Shame on me, but I am currently too lazy to work out a minimal example and find a way to give it to the clang people. And I need to find a way to suppress warnings form external code anyway.

Answer 5 · 2016-09-17T14:10:10.000Z

I added a question how to suppress clang-tidy warnings on stackoverflow.
David Hallas made me aware of the fact that you already have // NOLINT comments in the file to suppress warnings. He solved the problem by adding that comment to line 1790 of gmock-spec-builders.h. Maybe you can consider to add that change.

Answer 6 · 2016-10-15T13:13:56.000Z

For me putting the // NOLINT behind line 1272 of gmock_spec_builders.h solved the problem. I will create a pull request for it.