llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.25k stars 11.66k forks source link

clang-tidy flagging lambda capture list as array index #42320

Open llvmbot opened 5 years ago

llvmbot commented 5 years ago
Bugzilla Link 42975
Version unspecified
OS Linux
Reporter LLVM Bugzilla Contributor
CC @EugeneZelenko,@mgehre

Extended Description

I ran clang-tidy 7.0.1 on commit 1ee79c2 of tlslookieloo (https://github.com/keithmendozasr/tlslookieloo/tree/1ee79c2f6935cb2f2e7ffdabddbacd29f2219653) with the following checks:

-,cppcoreguidelines-,-cppcoreguidelines-pro-bounds-pointer-arithmetic -header-filter=./tlslookieloo/include/*

This check flagged that the "expectData" is a cppcoreguidelines-pro-bounds-constant-array-index:

[expectData](void *ptr){ memcpy(ptr, &expectData[0], sizeof(expectData)); }

In this context, "expectData" is is a const char[] that's enumerated in the lambda capture.

Here is the full context of where it's used:

TEST_F(TargetTest, messageRelayGood) // NOLINT { { InSequence s; const char expectData[] = "abc"; EXPECT_CALL((mock), SSL_read(NotNull(), NotNull(), 1024)) .WillOnce(DoAll(WithArg<1>(Invoke( [expectData](void ptr){ memcpy(ptr, &expectData[0], sizeof(expectData)); })), Return(sizeof(expectData))));

    // NOLINTNEXTLINE
    EXPECT_CALL((*mock), SSL_write(NotNull(), IsVoidEqStr(expectData), sizeof(expectData)))
        .WillOnce(Return(sizeof(expectData)));

    EXPECT_CALL((*mock), SSL_read(NotNull(), NotNull(), 1024))
        .WillOnce(Return(0));

    EXPECT_CALL((*mock), SSL_get_error(NotNull(), 0))
        .WillOnce(Return(SSL_ERROR_WANT_READ));
}

Target obj;
EXPECT_TRUE(obj.messageRelay(client, server, Target::MSGOWNER::CLIENT));

}

Complete system details:

llvmbot commented 4 years ago

I'll also investigate further why I did not see this warning for your codebase.

I suspect this had to do with googletest not being "built" at the time you ran clang-tidy on tests/unit/target.cpp; which, triggered the error on gtest/gtest.h when you were doing your testing. While I was creating the minimal example, I started with the lambda declaration looking like this

auto lambda_catcher = [expectData](void *ptr){...};

This flagged the [expectData] as a cppcoreguidelines-avoid-c-arrays.

Hope this extra info helps narrow down the cause. In the mean time I'm rebuilding clang-tidy with MinSizeRel build type.

llvmbot commented 4 years ago

Thanks for providing the minimal example! Now I can also reproduce the issue. I'll start working on it.

I'll also investigate further why I did not see this warning for your codebase.

llvmbot commented 4 years ago

I installed clang-tidy 9.0.0 and line 18 of the attached "minimal code triggering issue" is still being flagged as cppcoreguidelines-pro-bounds-constant-array-index.

llvmbot commented 4 years ago

minimal code triggering issue The attached test.cpp is a complete minimal code that will flag cppcoreguidelines-pro-bounds-constant-array-index on the lambda function that's being passed as a std::function parameter. For this particular code, I ran clang-tidy in the following way:

clang-tidy -checks=-*,cppcoreguidelines-* test.cpp -- -Wall -Werror -pedantic

Here is clang-tidy's full output:

408 warnings generated. /home/kmendoza/test_clang-tidy/test.cpp:11:47: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay] cout << "Value of payload after run: " << payload << endl; ^ /home/kmendoza/test_clang-tidy/test.cpp:18:10: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index] [expectData](void ptr){ ^ /home/kmendoza/test_clang-tidy/test.cpp:19:48: warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay] cout << "Value of expectData: " << expectData << endl; ^ Suppressed 405 warnings (405 in non-user code). Use -header-filter=. to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.

llvmbot commented 4 years ago

Unfortunately using clang-tidy 7.0.1 I don't see any cppcoreguidelines-pro-bounds-constant-array-index warning.

Have you tried maybe the newest clang-tidy?

Can you provide some minimal cpp file which reproduces the error?

llvmbot commented 4 years ago

Here is the full end-to-end step that I use to reproduce the error:

git clone https://github.com/keithmendozasr/tlslookieloo.git git checkout lee79c2 mkdir build_debug cd build_debug cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=1 cmake --build . lang-tidy --header-filter=./tlslookieloo/include/ --checks='-,cppcoreguidelines-*,-cppcoreguidelines-pro-bounds-pointer-arithmetic' ../tests/unit/target.cpp

llvmbot commented 4 years ago

Here's the full output when I run clang-tidy --header-filter=./tlslookieloo/include/ -checks=-,cppcoreguidelines-*,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-avoid-goto -p=./tlslookieloo/build_debug ./tlslookieloo/tests/unit/target.cpp

3485 warnings generated. Suppressed /home/kmendoza/tlslookieloo/tests/unit/target.cpp:401:18: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index] [expectData](void ptr){ ^ /home/kmendoza/tlslookieloo/tests/unit/target.cpp:425:14: warning: do not use array subscript when the index is not an integer constant expression; use gsl::at() instead [cppcoreguidelines-pro-bounds-constant-array-index] [expectData](void ptr){ ^ 3530 warnings (3483 in non-user code, 47 NOLINT). Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.

Some context that might prove useful: I usually run clang-tidy before I make a commit. In the location that I'm running it, cmake has been ran with -DCMAKE_BUILD_TYPE=Debug -DCMAKE_EXPORT_COMPILE_COMMANDS=1, and the code has been compiled using the g++ compiler.

I currently do not have a Dockerfile that compiles this project. There's a plan to generate some pre-built packages though.

llvmbot commented 4 years ago

Can you please paste the full output from clang-tidy?

I run your test and don't see cppcoreguidelines-pro-bounds-constant-array-index errors:

root@247ce3f096ed:~# /usr/bin/clang-tidy-7 --version LLVM (http://llvm.org/): LLVM version 7.0.1

Optimized build. Default target: x86_64-pc-linux-gnu Host CPU: haswell

root@247ce3f096ed:~# /usr/bin/clang-tidy-7 --checks="-,cppcoreguidelines-" -header-filter=./tlslookieloo/include/* tlslookieloo/tests/unit/target.cpp

Error while trying to load a compilation database: Could not auto-detect compilation database for file "tlslookieloo/tests/unit/target.cpp" No compilation database found in /root/tlslookieloo/tests/unit or any parent directory fixed-compilation-database: Error while opening fixed database: No such file or directory json-compilation-database: Error while opening JSON database: No such file or directory Running without flags. 1 error generated. Error while processing /root/tlslookieloo/tests/unit/target.cpp. /root/tlslookieloo/tests/unit/target.cpp:17:10: error: 'gtest/gtest.h' file not found [clang-diagnostic-error]

include "gtest/gtest.h"

     ^

Found compiler error(s).

On the other hand, when I use clang-tidy-10, I've got valid warnings (there are multiple other warnings, but I'm omitting them here):

... /root/tlslookieloo/tests/unit/target.cpp:256:11: warning: do not declare C-style arrays, use std::array<> instead [cppcoreguidelines-avoid-c-arrays] const char expectData[] = "abc"; ...

If you have Dockerfile for the environment which shows the issue, please share it.

carlosgalvezp commented 1 year ago

No longer reproducible on trunk.

carlosgalvezp commented 1 year ago

My bad, I was checking in C mode instead of C++ mode, problem persists!