search

espressif / clang-tidy-runner

MIT License
9 stars 5 forks source link

Add an option to generate SARIF and/or SAST output (RDT-110) #9

Open igrr opened 2 years ago

igrr commented 2 years ago

When we run clang-tidy, we get a warnings.txt file as output. It would be nice to add functionality to parse the warnings.txt file and output SARIF or SAST JSON files which can then be fed into Github or Gitlab.

For reference, there is a clang-tidy-sarif tool which performs this kind of conversion, written in Rust: https://github.com/psastras/sarif-rs/tree/main/clang-tidy-sarif.

igrr commented 2 years ago

Another related issue, warnings.txt currently contains absolute file paths. When generating SARIF, we need to convert them to file paths relative to the repository root, otherwise Github won't associate the reported warnings with source files.

A script in https://github.com/espressif/idf-extra-components/pull/28 currently handles this after running clang-tidy-sarif (and also excludes warnings reported for ESP-IDF itself, https://github.com/espressif/clang-tidy-runner/issues/7)

igrr commented 2 years ago

Another issue with clang-tidy-sarif tool is that it only processes the first line (warning: or error:) and ignores subsequent note: lines, which provide additional context about the issue. These note lines are useful in order to understand the conditions when the issue occurs.

Example:

warnings.txt ``` /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:218:22: warning: Call to 'realloc' has an allocation size of 0 bytes [clang-analyzer-optin.portability.UnixAPI] args->data_out = realloc(args->data_out, data_out_size); ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:269:9: note: Assuming 'ctx' is not equal to NULL if (ctx == NULL || args == NULL || args->data_in == NULL) { ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:269:9: note: Left side of '||' is false /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:269:24: note: Assuming 'args' is not equal to NULL if (ctx == NULL || args == NULL || args->data_in == NULL) { ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:269:9: note: Left side of '||' is false if (ctx == NULL || args == NULL || args->data_in == NULL) { ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:269:40: note: Assuming field 'data_in' is not equal to NULL if (ctx == NULL || args == NULL || args->data_in == NULL) { ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:269:5: note: Taking false branch if (ctx == NULL || args == NULL || args->data_in == NULL) { ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:273:9: note: 'handle' is not equal to NULL if (handle == NULL) { ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:273:5: note: Taking false branch if (handle == NULL) { ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:281:5: note: Control jumps to 'case ESP_PRE_ENC_IMG_READ_EXTRA_HEADER:' at line 393 switch (handle->state) { ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:395:23: note: Assuming the condition is true curr_index += MIN(args->data_in_len - curr_index, RESERVED_HEADER - handle->binary_file_read); ^ /opt/esp/tools/xtensa-clang/12.0.1-d9341b81fc/xtensa-esp32-elf-clang/bin/../xtensa-esp32-elf/sys-include/sys/param.h:29:19: note: expanded from macro 'MIN' #define MIN(a,b) ((a) < (b) ? (a) : (b)) ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:395:23: note: '?' condition is true curr_index += MIN(args->data_in_len - curr_index, RESERVED_HEADER - handle->binary_file_read); ^ /opt/esp/tools/xtensa-clang/12.0.1-d9341b81fc/xtensa-esp32-elf-clang/bin/../xtensa-esp32-elf/sys-include/sys/param.h:29:19: note: expanded from macro 'MIN' #define MIN(a,b) ((a) < (b) ? (a) : (b)) ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:396:37: note: '?' condition is true handle->binary_file_read += MIN(args->data_in_len - temp, RESERVED_HEADER - handle->binary_file_read); ^ /opt/esp/tools/xtensa-clang/12.0.1-d9341b81fc/xtensa-esp32-elf-clang/bin/../xtensa-esp32-elf/sys-include/sys/param.h:29:19: note: expanded from macro 'MIN' #define MIN(a,b) ((a) < (b) ? (a) : (b)) ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:397:13: note: Assuming field 'binary_file_read' is equal to RESERVED_HEADER if (handle->binary_file_read == RESERVED_HEADER) { ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:397:9: note: Taking true branch if (handle->binary_file_read == RESERVED_HEADER) { ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:407:15: note: Calling 'process_bin' err = process_bin(handle, args, curr_index); ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:172:9: note: Assuming field 'binary_file_read' is equal to field 'binary_file_len' if (handle->binary_file_read != handle->binary_file_len) { ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:172:5: note: Taking false branch if (handle->binary_file_read != handle->binary_file_len) { ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:217:5: note: The value 0 is assigned to 'data_out_size' data_out_size = handle->cache_buf_len + data_len - curr_index; ^ /__w/idf-extra-components/idf-extra-components/esp_encrypted_img/src/esp_encrypted_img.c:218:22: note: Call to 'realloc' has an allocation size of 0 bytes args->data_out = realloc(args->data_out, data_out_size); ^ ```
sarif.json produced using clang-tidy-sarif ```json { "level": "warning", "locations": [ { "physicalLocation": { "artifactLocation": { "uri": "esp_encrypted_img/src/esp_encrypted_img.c" }, "region": { "startColumn": 22, "startLine": 218 } } } ], "message": { "text": "Call to 'realloc' has an allocation size of 0 bytes [clang-analyzer-optin.portability.UnixAPI]" } }, ```
igrr commented 1 year ago

Libraries which may help:

igrr commented 3 weeks ago

Now it's possible to get diagnostics in YAML format from clang-tidy via -export-fixes argument (which seems to work even for issues that don't have fix suggestions). This should make this task simpler, since we don't need to parse stderr anymore.

There are also discussions upstream about adding SARIF support to clang-tidy itself.