VS Code "Run Code Analysis on Active File" support

[Burt]

Hello,

It is not clear to me whether you intend to support this key feature built into the C/C++ extension. Just to make sure we are on the same page; this feature is activated by right clicking in a file editor window and selecting the option I show in quotes in the Subject line above. [Or it can be configured to run automatically without a right click when a file is opened, but that is not the default.] If you attempt to run this on files in the nRF Connect SDK, for example, I like to use location_module.c in the asset_tracker_v2 application, you will find that it does what it is supposed to do...it finds errors. It finds errors that should have been corrected 2 or 3 years ago. Not only are those errors misleading, but they will likely prevent code analysis from finding more interesting errors/warnings that I may have made while developing code.

My first fix is not perfect, but it gets rid of a problem that has plagued the problematic C/C++ extension for years despite people looking at the issue and thinking they have fixed it. Now, this only happens in Windows; not on Linux: I forget whether it happens on Mac. Without further ado, the fix is:

diff -Naur toolchains0/c57af46cb7/opt/zephyr-sdk/arm-zephyr-eabi/lib/gcc/arm-zephyr-eabi/12.2.0/include/stddef.h toolchains/c57af46cb7/opt/zephyr-sdk/arm-zephyr-eabi/lib/gcc/arm-zephyr-eabi/12.2.0/include/stddef.h
--- toolchains0/c57af46cb7/opt/zephyr-sdk/arm-zephyr-eabi/lib/gcc/arm-zephyr-eabi/12.2.0/include/stddef.h 2023-05-11 13:33:12.000000000 -0400
+++ toolchains/c57af46cb7/opt/zephyr-sdk/arm-zephyr-eabi/lib/gcc/arm-zephyr-eabi/12.2.0/include/stddef.h 2023-11-25 00:20:16.950551600 -0500
@@ -211,7 +211,12 @@
#define __SIZE_TYPE__ long unsigned int
#endif
#if !(defined (__GNUG__) && defined (size_t))
+#if __CDT_PARSER__ != 1
typedef __SIZE_TYPE__ size_t;
+#else
+#define __GNUC__ 12
+typedef unsigned long long size_t;
+#endif
#ifdef __BEOS__
typedef long ssize_t;
#endif /* __BEOS__ */

Note that I have used your __CDT_PARSER__ trick to change the type of size_t to unsigned long long for only the IntelliSense/clang-tidy case and I leave the existing code in place for regular builds. Please understand that the __GNUC__ definition is here to solve another IntelliSense issue that I will show you after you have had a chance to digest this change.

To see the usefulness of this, on Windows, please run Code Analysis on a source file that is part of asset_tracker_v2, and then look closely how the problem list has shortened from when you ran Code Analysis without the fix.

The downside is that size_t should not really be unsigned long long, but as it is a bug to re-type a variable except in clang with -Wno-typedef-redefinition, we are forced to use this. Actually, you can take my fix above but leave out the "typedef unsigned long long size_t;" line, since it is merely a re-typing without any change (not a bug to re-type to the same type).

Please give me a schedule for either using my fix or your own fix to solve the issue on Windows I have shown. Thank you.

[Burt]

Hi Trond,

I like your feature request 11749. It makes a lot of sense to me. Hopefully the C/C++ extension folks will implement the feature.

Indeed, my expression for ATOMIC_BITMAP_SIZE ended up bloated. I will switch to your definition. I will attempt to contribute the change to Zephyr. It will be my first attempt to contribute; I'll follow their documented recommendations.

Thank you again for the thought and effort you have put into all of this.

Burt

[Burt]

I have another baffling situation: Using nRF Connect for VS CodeI "Created a new application" from a sample, in other words, I created asset_tracker_v2 outside of the SDK tree, whereas previously I was working in the tree. And here everything seems to behave except that Code Analysis will not run at all. I don't even know how to debug this. 

[Burt]

Now in the "in tree", the extra-arg fix is working BUT in the file app_event.c whereas I used to see Code Analysis warnings due to strcat, I no longer see them. Here is some stuff from the command line: I get different results using clang-tidy from 

C:\Users\Burt>where clang-tidy
C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\Llvm\bin\clang-tidy.exe

than I do using the built in version:

C:\ncs2\v2.5.0\nrf\applications\asset_tracker_v2>clang-tidy -p build src\events\app_module_event.c -checks=-*,clang-analyzer-security.insecureAPI.strcpy --system-headers
3 warnings and 2 errors generated.
Error while processing C:\ncs2\v2.5.0\nrf\applications\asset_tracker_v2\src\events\app_module_event.c.
error: unknown argument: '-fno-reorder-functions' [clang-diagnostic-error]
error: unknown argument: '-mfp16-format=ieee' [clang-diagnostic-error]
C:\ncs2\v2.5.0\nrf\applications\asset_tracker_v2\src\events\app_module_event.c:82:4: warning: Call to function 'strcat' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcat'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
strcat(data_types, type2str(event->data_list[i])); AND ON AND ON AND ON...

and here is builtin

C:\ncs2\v2.5.0\nrf\applications\asset_tracker_v2>\Users\Burt\.vscode\extensions\ms-vscode.cpptools-1.19.1-win32-x64\LLVM\bin\clang-tidy -p build src\events\app_module_event.c -checks=-*,clang-analyzer-security.insecureAPI.strcpy --system-headers --extra-arg=--target=arm-zephyr-eabi-gcc
1 warning and 3 errors generated.
Error while processing C:\ncs2\v2.5.0\nrf\applications\asset_tracker_v2\src\events\app_module_event.c.
error: unknown argument: '-fno-reorder-functions' [clang-diagnostic-error]
error: unknown argument: '-mfp16-format=ieee' [clang-diagnostic-error]
C:/ncs/toolchains/c57af46cb7/opt/zephyr-sdk/arm-zephyr-eabi/arm-zephyr-eabi\include\sys/cdefs.h:47:10: error: 'stddef.h' file not found [clang-diagnostic-error]
47 | #include <stddef.h>
| ^~~~~~~~~~
Suppressed 1 warnings (1 with check filters).
Found compiler error(s).

C:\ncs2\v2.5.0\nrf\applications\asset_tracker_v2>

Everything is crazy with clang-tidy!!!!! Note although I was playing around with -checks here I did not have that in my settings.json originally (and most of the time).

Burt

[Burt]

The packaged clang-tidy can be made to work on the command line by adding to compile_commands.json

-IC:/ncs/toolchains/c57af46cb7/opt/zephyr-sdk/arm-zephyr-eabi/lib/gcc/arm-zephyr-eabi/12.2.0/include -IC:/ncs/toolchains/c57af46cb7/opt/zephyr-sdk/arm-zephyr-eabi/lib/gcc/arm-zephyr-eabi/12.2.0/include-fixed

and if --extra-arg=--target=arm-zephyr-eabi-gcc is not present then also add

-IC:/ncs/toolchains/c57af46cb7/opt/zephyr-sdk/arm-zephyr-eabi/arm-zephyr-eabi/sys-include

Those paths are not needed under Linux. Even the sysroot doesn't seem to be needed in the Linux compile_commands.json.

However, I am not seeing the strcat warnings in either Linux or Windows VS Code at this time; only on the command line.

Another interesting thing: --target seems to know about only "arm" and "eabi" and it does not care whether it is written arm-eabi or eabi-arm. Doesn't seem to care about "zephyr" or "gcc". I read that you have to go to the source code to see which "triplets" are recognized. Ugh!

Burt

[Burt]

I made some good progress but I still need help. I made progress because I finally figured out where to find the OUTPUT for C/C++, so I can debug. All the appropriate warnings are showing up in the OUTPUT window. I did have to disable "-p build" (I had added it to match my command line usage.) And I do have to keep --system-headers in order to see the strcat warnings in the OUTPUT. Something seems wrong about that, and maybe that's the problem. Consider this snippet from app_module_event.c that has a few mods from the original code:

           

            size_t max_append_len = sizeof(data_types) - strlen(data_types) - 1;

            if (i < event->count - 1) {

                /* For every type except the last one, we also need space

                 * for a comma and a space character.

                 */

                //max_append_len -= 2;

            }

//          if (strlen(type2str(event->data_list[i])) > max_append_len) {

//              return;

//          }

            strcat(data_types, type2str(event->data_list[i]));

 

Unfortunately the formatting from VS Code didn't show up here, but clang-tidy warned me that max_append_len is not being read, but no warnings about strcat got transferred from the clang-tidy output to the editor window.

FYI, from settings.json

   "C_Cpp.codeAnalysis.clangTidy.args": [

        "--extra-arg=--target=arm-eabi",

        "--system-headers"/*

        "-p build",

        "-checks=-clang-d*"*/

      ],

Burt