Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
igrr opened this issue Mar 21, 2022 · 4 comments
Open

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

igrr opened this issue Mar 21, 2022 · 4 comments

Comments

@igrr
Copy link
Member

igrr commented Mar 21, 2022

  • SARIF is a JSON based format for representing code scanning results, currently used by Github (see docs.)
  • SAST is another JSON based format, used by Gitlab (see schema.)

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
Copy link
Member Author

igrr commented Mar 21, 2022

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 espressif/idf-extra-components#28 currently handles this after running clang-tidy-sarif (and also excludes warnings reported for ESP-IDF itself, #7)

@igrr
Copy link
Member Author

igrr commented Mar 21, 2022

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
    {
     "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 igrr changed the title Add an option to generate SARIF and SAST output Add an option to generate SARIF and/or SAST output Mar 21, 2022
@github-actions github-actions bot changed the title Add an option to generate SARIF and/or SAST output Add an option to generate SARIF and/or SAST output (RDT-110) Mar 22, 2022
@igrr
Copy link
Member Author

igrr commented Sep 7, 2023

@igrr
Copy link
Member Author

igrr commented Aug 19, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant