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.

Sign up for GitHub

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

Jump to bottom

Run clang-tidy in CI or as an extended check #537

Closed
paleolimbot opened this issue Jun 24, 2024 · 14 comments · Fixed by #639 · May be fixed by #538
Closed

Run clang-tidy in CI or as an extended check #537

paleolimbot opened this issue Jun 24, 2024 · 14 comments · Fixed by #639 · May be fixed by #538
Milestone
nanoarrow 0.6.0

Comments

@paleolimbot
Copy link
Member

The clang-tidy job in ADBC higlighted a few warnings that should be addressed ( apache/arrow-adbc#1928 ). We should run this in our CI to ensure that nanoarrow doesn't cause problems for downstream projects that use clang-tidy (like ADBC!).

clang-tidy --use-color -export-fixes /tmp/tmp_73j2310/tmpw7bhm7r6.yaml -extra-arg=-Wno-unknown-warning-option -p=/home/runner/work/arrow-adbc/arrow-adbc/build/tidy -quiet /home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:186:42: warning: Call to 'malloc' has an allocation size of 0 bytes [clang-analyzer-optin.portability.UnixAPI]
void* ArrowMalloc(int64_t size) { return malloc(size); }
                                         ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:3334:7: note: Calling 'PrivateArrowArrayViewInitFromSchema'
      ArrowArrayViewInitFromSchema(&array_view, &private_data->schema, error));
      ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:1105:3: note: expanded from macro 'ArrowArrayViewInitFromSchema'
  NANOARROW_SYMBOL(NANOARROW_NAMESPACE, ArrowArrayViewInitFromSchema)
  ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:1025:32: note: expanded from macro 'NANOARROW_SYMBOL'
#define NANOARROW_SYMBOL(A, B) NANOARROW_CAT(A, B)
                               ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:1024:29: note: expanded from macro 'NANOARROW_CAT'
#define NANOARROW_CAT(A, B) A##B
                            ^
note: expanded from here
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:301:83: note: expanded from macro 'NANOARROW_RETURN_NOT_OK'
  _NANOARROW_RETURN_NOT_OK_IMPL(_NANOARROW_MAKE_NAME(errno_status_, __COUNTER__), EXPR)
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:173:23: note: expanded from macro '_NANOARROW_RETURN_NOT_OK_IMPL'
    const int NAME = (EXPR);                      \
                      ^~~~
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:2545:7: note: 'result' is equal to NANOARROW_OK
  if (result != NANOARROW_OK) {
      ^~~~~~
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:2545:3: note: Taking false branch
  if (result != NANOARROW_OK) {
  ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:2552:12: note: Calling 'PrivateArrowArrayViewAllocateChildren'
  result = ArrowArrayViewAllocateChildren(array_view, schema->n_children);
           ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:1107:3: note: expanded from macro 'ArrowArrayViewAllocateChildren'
  NANOARROW_SYMBOL(NANOARROW_NAMESPACE, ArrowArrayViewAllocateChildren)
  ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:1025:32: note: expanded from macro 'NANOARROW_SYMBOL'
#define NANOARROW_SYMBOL(A, B) NANOARROW_CAT(A, B)
                               ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:1024:29: note: expanded from macro 'NANOARROW_CAT'
#define NANOARROW_CAT(A, B) A##B
                            ^
note: expanded from here
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:2497:19: note: Field 'children' is equal to NULL
  if (array_view->children != NULL) {
                  ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:2497:3: note: Taking false branch
  if (array_view->children != NULL) {
  ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:2502:44: note: Passing the value 0 via 1st parameter 'size'
      (struct ArrowArrayView**)ArrowMalloc(n_children * sizeof(struct ArrowArrayView*));
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:2502:32: note: Calling 'PrivateArrowMalloc'
      (struct ArrowArrayView**)ArrowMalloc(n_children * sizeof(struct ArrowArrayView*));
                               ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:1030:21: note: expanded from macro 'ArrowMalloc'
#define ArrowMalloc NANOARROW_SYMBOL(NANOARROW_NAMESPACE, ArrowMalloc)
                    ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:1025:32: note: expanded from macro 'NANOARROW_SYMBOL'
#define NANOARROW_SYMBOL(A, B) NANOARROW_CAT(A, B)
                               ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:1024:29: note: expanded from macro 'NANOARROW_CAT'
#define NANOARROW_CAT(A, B) A##B
                            ^
note: expanded from here
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:186:42: note: Call to 'malloc' has an allocation size of 0 bytes
void* ArrowMalloc(int64_t size) { return malloc(size); }
                                         ^      ~~~~
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:2763:13: warning: Value stored to 'min_buffer_size_bytes' during its initialization is never read [clang-analyzer-deadcode.DeadStores]
    int64_t min_buffer_size_bytes = array_view->buffer_views[i].size_bytes + 1;
            ^~~~~~~~~~~~~~~~~~~~~   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:2763:13: note: Value stored to 'min_buffer_size_bytes' during its initialization is never read
    int64_t min_buffer_size_bytes = array_view->buffer_views[i].size_bytes + 1;
            ^~~~~~~~~~~~~~~~~~~~~   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:3303:7: warning: Potential leak of memory pointed to by 'private_data' [clang-analyzer-unix.Malloc]
      ArrowBasicArrayStreamRelease(array_stream);
      ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:3287:40: note: Calling 'PrivateArrowMalloc'
      (struct BasicArrayStreamPrivate*)ArrowMalloc(
                                       ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:1030:21: note: expanded from macro 'ArrowMalloc'
#define ArrowMalloc NANOARROW_SYMBOL(NANOARROW_NAMESPACE, ArrowMalloc)
                    ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:1025:32: note: expanded from macro 'NANOARROW_SYMBOL'
#define NANOARROW_SYMBOL(A, B) NANOARROW_CAT(A, B)
                               ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:1024:29: note: expanded from macro 'NANOARROW_CAT'
#define NANOARROW_CAT(A, B) A##B
                            ^
note: expanded from here
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:186:42: note: Memory is allocated
void* ArrowMalloc(int64_t size) { return malloc(size); }
                                         ^~~~~~~~~~~~
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:3287:40: note: Returned allocated memory
      (struct BasicArrayStreamPrivate*)ArrowMalloc(
                                       ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:1030:21: note: expanded from macro 'ArrowMalloc'
#define ArrowMalloc NANOARROW_SYMBOL(NANOARROW_NAMESPACE, ArrowMalloc)
                    ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:1025:32: note: expanded from macro 'NANOARROW_SYMBOL'
#define NANOARROW_SYMBOL(A, B) NANOARROW_CAT(A, B)
                               ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:1024:29: note: expanded from macro 'NANOARROW_CAT'
#define NANOARROW_CAT(A, B) A##B
                            ^
note: expanded from here
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:3289:7: note: Assuming 'private_data' is not equal to NULL
  if (private_data == NULL) {
      ^~~~~~~~~~~~~~~~~~~~
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:3289:3: note: Taking false branch
  if (private_data == NULL) {
  ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:3299:7: note: Assuming 'n_arrays' is > 0
  if (n_arrays > 0) {
      ^~~~~~~~~~~~
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:3299:3: note: Taking true branch
  if (n_arrays > 0) {
  ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:3302:9: note: Assuming field 'arrays' is equal to NULL
    if (private_data->arrays == NULL) {
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:3302:5: note: Taking true branch
    if (private_data->arrays == NULL) {
    ^
/home/runner/work/arrow-adbc/arrow-adbc/c/vendor/nanoarrow/nanoarrow.c:3303:7: note: Potential leak of memory pointed to by 'private_data'
      ArrowBasicArrayStreamRelease(array_stream);
      ^
Applying fixes ...

https://github.com/apache/arrow-adbc/actions/runs/9590445640/job/26445818971?pr=1928#step:7:344
This was referenced Jun 24, 2024
Merged
Draft
@vyasr
Copy link
Contributor

vyasr commented Sep 30, 2024

FWIW this issue just popped up in cudf as well. In my case, the tracebacks all point back to inline_buffer.h, specifically

  /home/coder/cudf/cpp/build/conda/cuda-12.5/release/_deps/nanoarrow-src/src/nanoarrow/common/inline_buffer.h:350:8: warning: Dereference of null pointer (loaded from variable 'out') [clang-analyzer-core.  NullDereference]                                                                                                                                                                                            /home/coder/cudf/cpp/build/conda/cuda-12.5/release/_deps/nanoarrow-src/src/nanoarrow/common/inline_buffer.h:474:23: warning: Array access (from variable 'bits') results in a null pointer dereference [cl  ang-analyzer-core.NullDereference]                                                                                                                                                                        
  /home/coder/cudf/cpp/build/conda/cuda-12.5/release/_deps/nanoarrow-src/src/nanoarrow/common/inline_buffer.h:480:21: warning: Array access (from variable 'bits') results in a null pointer dereference [cl  ang-analyzer-core.NullDereference]                                                                                                                                                                        
  /home/coder/cudf/cpp/build/conda/cuda-12.5/release/_deps/nanoarrow-src/src/nanoarrow/common/inline_buffer.h:640:17: warning: Dereference of null pointer (loaded from variable 'out_cursor') [clang-analyz  er-core.NullDereference

I'm also having difficulty suppressing them in our runs. The recent release of clang-tidy 19 exposed the ability to exclude headers, which helps, but I'm running into cases specifically with nanoarrow where the exclusions don't work. I suspect that the issue comes from the extensive use of macros and clang-tidy having special logic around the way macros are handled in the call stack, but I'm not sure about that.

@paleolimbot
Copy link
Member Author

I'll take another look today!

@WillAyd
Copy link
Contributor

WillAyd commented Sep 30, 2024 •
edited
Loading

Meson has built-in support for clang-tidy, so if we wanted to I think can just add a .clang-tidy configuration to the project root and run meson compile clang-tidy to check

@paleolimbot
Copy link
Member Author

I'll plug through fixing all of these tomorrow, but I'll also share here the workaround from ADBC which may be helpful here which involves some magic with jq:

https://github.com/apache/arrow-adbc/blob/8c6243f1058529c47285eabdef6bb83a118f6040/ci/scripts/cpp_clang_tidy.sh#L67-L74

@paleolimbot paleolimbot mentioned this issue Sep 30, 2024
Merged
@vyasr
Copy link
Contributor

vyasr commented Oct 1, 2024

Ah yeah I'm doing something similar to prevent clang-tidy from checking nanoarrow sources, but I still see some errors in nanoarrow headers because clang-tidy will check all headers included in source files that are checked. In theory those should be possible to filter, and I've been able to filter all other vendored/downloaded at build-time headers, but not nanoarrow's. I suspect the issue is specifically with how macros are being handled in nested contexts and that defeating the filtering capabilities of clang-tidy.

@paleolimbot paleolimbot added this to the nanoarrow 0.6.0 milestone Oct 2, 2024
paleolimbot added a commit that referenced this issue Oct 2, 2024
@paleolimbot
chore(ci): Add clang-tidy checks and ensure they pass (#639)
b1ba426 
This PR adds a `clang-tidy` check to CI and fixes several issues that it
identified (including a few from other repos like ADBC and cudf).

A reboot of #538; closes #537.
@WillAyd
Copy link
Contributor

WillAyd commented Oct 4, 2024

Is the current implementation expected to be exhaustive? I still see a lot of errors when running locally - not sure if that is expected or not:

$ clang-tidy src/nanoarrow/common/inline_array.h 

11 warnings and 4 errors generated.
Error while processing /home/willayd/clones/arrow-nanoarrow/src/nanoarrow/common/inline_array.h.
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/inline_buffer.h:138:21: error: assigning to 'struct ArrowBufferAllocator' from incompatible type 'int' [clang-diagnostic-error]
  buffer->allocator = ArrowBufferAllocatorDefault();
                    ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/inline_buffer.h:614:20: error: static declaration of 'ArrowBitmapAppendUnsafe' follows non-static declaration [clang-diagnostic-error]
static inline void ArrowBitmapAppendUnsafe(struct ArrowBitmap* bitmap,
                   ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/inline_buffer.h:610:3: note: previous implicit declaration is here
  ArrowBitmapAppendUnsafe(bitmap, bits_are_set, length);
  ^
/home/willayd/clones/arrow-nanoarrow/src/nanoarrow/common/inline_array.h:75:15: warning: implicitly declaring library function 'strtol' with type 'long (const char *, char **, int)' [clang-diagnostic-implicit-function-declaration]
    type_id = strtol(type_ids, &end_ptr, 10);
              ^
/home/willayd/clones/arrow-nanoarrow/src/nanoarrow/common/inline_array.h:75:15: note: include the header <stdlib.h> or explicitly provide a declaration for 'strtol'
/home/willayd/clones/arrow-nanoarrow/src/nanoarrow/common/inline_array.h:243:33: warning: implicit declaration of function 'ArrowArrayAppendEmpty' is invalid in C99 [clang-diagnostic-implicit-function-declaration]
        NANOARROW_RETURN_NOT_OK(ArrowArrayAppendEmpty(array->children[i], n));
                                ^
/home/willayd/clones/arrow-nanoarrow/src/nanoarrow/common/inline_array.h:255:31: warning: implicit declaration of function 'ArrowArrayAppendEmpty' is invalid in C99 [clang-diagnostic-implicit-function-declaration]
      NANOARROW_RETURN_NOT_OK(ArrowArrayAppendEmpty(
                              ^
/home/willayd/clones/arrow-nanoarrow/src/nanoarrow/common/inline_array.h:260:33: warning: implicit declaration of function 'ArrowArrayAppendEmpty' is invalid in C99 [clang-diagnostic-implicit-function-declaration]
        NANOARROW_RETURN_NOT_OK(ArrowArrayAppendEmpty(array->children[i], n));
                                ^
/home/willayd/clones/arrow-nanoarrow/src/nanoarrow/common/inline_array.h:330:30: error: static declaration of 'ArrowArrayAppendEmpty' follows non-static declaration [clang-diagnostic-error]
static inline ArrowErrorCode ArrowArrayAppendEmpty(struct ArrowArray* array, int64_t n) {
                             ^
/home/willayd/clones/arrow-nanoarrow/src/nanoarrow/common/inline_array.h:243:33: note: previous implicit declaration is here
        NANOARROW_RETURN_NOT_OK(ArrowArrayAppendEmpty(array->children[i], n));
                                ^
/home/willayd/clones/arrow-nanoarrow/src/nanoarrow/common/inline_array.h:362:14: warning: implicit declaration of function 'ArrowArrayAppendUInt' is invalid in C99 [clang-diagnostic-implicit-function-declaration]
      return ArrowArrayAppendUInt(array, value);
             ^
/home/willayd/clones/arrow-nanoarrow/src/nanoarrow/common/inline_array.h:388:30: error: static declaration of 'ArrowArrayAppendUInt' follows non-static declaration [clang-diagnostic-error]
static inline ArrowErrorCode ArrowArrayAppendUInt(struct ArrowArray* array,
                             ^
/home/willayd/clones/arrow-nanoarrow/src/nanoarrow/common/inline_array.h:362:14: note: previous implicit declaration is here
      return ArrowArrayAppendUInt(array, value);
             ^
/home/willayd/clones/arrow-nanoarrow/src/nanoarrow/common/inline_array.h:515:36: warning: cast to 'struct ArrowBuffer *' from smaller integer type 'int' [clang-diagnostic-int-to-pointer-cast]
  private_data->variadic_buffers = (struct ArrowBuffer*)ArrowRealloc(
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/willayd/clones/arrow-nanoarrow/src/nanoarrow/common/inline_array.h:515:57: warning: implicit declaration of function 'ArrowRealloc' is invalid in C99 [clang-diagnostic-implicit-function-declaration]
  private_data->variadic_buffers = (struct ArrowBuffer*)ArrowRealloc(
                                                        ^
/home/willayd/clones/arrow-nanoarrow/src/nanoarrow/common/inline_array.h:520:41: warning: cast to 'int64_t *' (aka 'long *') from smaller integer type 'int' [clang-diagnostic-int-to-pointer-cast]
  private_data->variadic_buffer_sizes = (int64_t*)ArrowRealloc(
                                        ^~~~~~~~~~~~~~~~~~~~~~~
/home/willayd/clones/arrow-nanoarrow/src/nanoarrow/common/inline_array.h:827:3: warning: implicit declaration of function 'ArrowArrayViewInitFromType' is invalid in C99 [clang-diagnostic-implicit-function-declaration]
  ArrowArrayViewInitFromType(src, NANOARROW_TYPE_UNINITIALIZED);
  ^

@vyasr
Copy link
Contributor

vyasr commented Oct 4, 2024

Wow this is amazing timing... #639 (comment)

@paleolimbot
Copy link
Member Author

It looks like you're running it directly on a single header? That header is not quite self-contained, which does need to be fixed, but we use run-clang-tidy which I think only checks source files and not headers. (You can modify the script ci/scripts/run-clang-tidy.sh to do additional checks and make a PR if there's other things that should be checked!)

Wow this is amazing timing.

I'll take a look!

@WillAyd
Copy link
Contributor

WillAyd commented Oct 4, 2024

Ah OK. Will take a look - some source files have issues too:

$ clang-tidy src/nanoarrow/common/utils.c 
2 warnings generated.
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:385:32: warning: The left operand of '!=' is a garbage value due to array index out of bounds [clang-analyzer-core.UndefinedBinaryOperatorResult]
    if (words_little_endian[i] != 0) {
        ~~~~~~~~~~~~~~~~~~~~~~ ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:363:7: note: Assuming field 'low_word_index' is not equal to 0
  if (decimal->low_word_index == 0) {
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:363:3: note: Taking false branch
  if (decimal->low_word_index == 0) {
  ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:366:21: note: Assuming 'i' is >= field 'n_words'
    for (int i = 0; i < decimal->n_words; i++) {
                    ^~~~~~~~~~~~~~~~~~~~
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:366:5: note: Loop condition is false. Execution continues on line 372
    for (int i = 0; i < decimal->n_words; i++) {
    ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:372:7: note: Assuming 'is_negative' is 0
  if (is_negative) {
      ^~~~~~~~~~~
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:372:3: note: Taking false branch
  if (is_negative) {
  ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:384:38: note: Assuming 'i' is >= 0
  for (int i = decimal->n_words - 1; i >= 0; i--) {
                                     ^~~~~~
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:384:3: note: Loop condition is true.  Entering loop body
  for (int i = decimal->n_words - 1; i >= 0; i--) {
  ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:385:9: note: Assuming the condition is false
    if (words_little_endian[i] != 0) {
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:385:5: note: Taking false branch
    if (words_little_endian[i] != 0) {
    ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:384:46: note: The value 2147483646 is assigned to 'i'
  for (int i = decimal->n_words - 1; i >= 0; i--) {
                                             ^~~
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:384:3: note: Loop condition is true.  Entering loop body
  for (int i = decimal->n_words - 1; i >= 0; i--) {
  ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:385:32: note: The left operand of '!=' is a garbage value due to array index out of bounds
    if (words_little_endian[i] != 0) {
        ~~~~~~~~~~~~~~~~~~~~~~ ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:418:38: warning: The left operand of '>>' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
      uint32_t hi = (uint32_t)(*elem >> 32);
                               ~~~~~ ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:363:7: note: Assuming field 'low_word_index' is not equal to 0
  if (decimal->low_word_index == 0) {
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:363:3: note: Taking false branch
  if (decimal->low_word_index == 0) {
  ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:366:21: note: Assuming 'i' is >= field 'n_words'
    for (int i = 0; i < decimal->n_words; i++) {
                    ^~~~~~~~~~~~~~~~~~~~
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:366:5: note: Loop condition is false. Execution continues on line 372
    for (int i = 0; i < decimal->n_words; i++) {
    ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:372:7: note: Assuming 'is_negative' is 0
  if (is_negative) {
      ^~~~~~~~~~~
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:372:3: note: Taking false branch
  if (is_negative) {
  ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:384:38: note: Assuming 'i' is >= 0
  for (int i = decimal->n_words - 1; i >= 0; i--) {
                                     ^~~~~~
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:384:3: note: Loop condition is true.  Entering loop body
  for (int i = decimal->n_words - 1; i >= 0; i--) {
  ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:385:9: note: Assuming the condition is true
    if (words_little_endian[i] != 0) {
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:385:5: note: Taking true branch
    if (words_little_endian[i] != 0) {
    ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:387:7: note:  Execution continues on line 392
      break;
      ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:392:3: note: Taking false branch
  if (most_significant_elem_idx == -1) {
  ^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:418:38: note: The left operand of '>>' is a garbage value
      uint32_t hi = (uint32_t)(*elem >> 32);
                               ~~~~~ ^

@paleolimbot
Copy link
Member Author

Do you have -DNANOARROW_DEBUG in the flags you're passing to clang-tidy? (This is one of the things run-clang-tidy takes care of...ensuring the build flags are also passed to clang-tidy).

@WillAyd
Copy link
Contributor

WillAyd commented Oct 4, 2024

Hmm well I got this from running the ninja target that Meson creates, which I think gets all of the necessary flags from the compilation database. Will have to take a closer look though

@paleolimbot
Copy link
Member Author

Then perhaps you didn't build in debug mode? I only ask because that exact error disappeared in CI when I added NANOARROW_DCHECK(n_words == 2 || n_words == 4) there.

@WillAyd
Copy link
Contributor

WillAyd commented Oct 4, 2024

I get the same warning in both debug / release builds, but haven't looked too much closer than that

@paleolimbot
Copy link
Member Author

I'm not an expert here but it's very possible I missed something! If adding a clang-tidy-meson job to the clang-tidy.yaml workflow makes sense and can reproduce some of those errors, go for it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
nanoarrow 0.6.0
3 participants
@WillAyd @vyasr @paleolimbot