You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "WillAyd (via GitHub)" <gi...@apache.org> on 2024/11/01 18:25:18 UTC
[I] clang-tidy issue for decimal implementation [arrow-nanoarrow]
WillAyd opened a new issue, #672:
URL: https://github.com/apache/arrow-nanoarrow/issues/672
Running clang-tidy locally I get the following error:
<details>
```
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:387: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:367: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:367:5: note: Loop condition is false. Execution continues on line 373
for (int i = 0; i < decimal->n_words; i++) {
^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:373:7: note: Assuming 'is_negative' is 0
if (is_negative) {
^~~~~~~~~~~
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:373:3: note: Taking false branch
if (is_negative) {
^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:386: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:386: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:387:9: note: Assuming the condition is false
if (words_little_endian[i] != 0) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:387:5: note: Taking false branch
if (words_little_endian[i] != 0) {
^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:386: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:386: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:387: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:420: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:367: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:367:5: note: Loop condition is false. Execution continues on line 373
for (int i = 0; i < decimal->n_words; i++) {
^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:373:7: note: Assuming 'is_negative' is 0
if (is_negative) {
^~~~~~~~~~~
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:373:3: note: Taking false branch
if (is_negative) {
^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:386: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:386: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:387:9: note: Assuming the condition is true
if (words_little_endian[i] != 0) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:387:5: note: Taking true branch
if (words_little_endian[i] != 0) {
^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:389:7: note: Execution continues on line 394
break;
^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:394:3: note: Taking false branch
if (most_significant_elem_idx == -1) {
^
/home/willayd/clones/arrow-nanoarrow/builddir/../src/nanoarrow/common/utils.c:420:38: note: The left operand of '>>' is a garbage value
uint32_t hi = (uint32_t)(*elem >> 32);
~~~~~ ^
2 warnings generated.
```
</details>
I think clang-tidy is missing the assurance that `decimal->n_words <= sizeof(words_little_endian)` in this function. Is there a macro that should be used to make that assertion?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] clang-tidy issue for decimal implementation [arrow-nanoarrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd closed issue #672: clang-tidy issue for decimal implementation
URL: https://github.com/apache/arrow-nanoarrow/issues/672
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] clang-tidy issue for decimal implementation [arrow-nanoarrow]
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on issue #672:
URL: https://github.com/apache/arrow-nanoarrow/issues/672#issuecomment-2452534926
Ah I see...I think the Meson configuration is just missing that. PR coming soon
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] clang-tidy issue for decimal implementation [arrow-nanoarrow]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on issue #672:
URL: https://github.com/apache/arrow-nanoarrow/issues/672#issuecomment-2452511255
...it also should be said that the macro will only expand when `NANOARROW_DEBUG` is defined, and sometimes it can be a trick to let clang-tidy know to compile it with that define. There very well may be a better way!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] clang-tidy issue for decimal implementation [arrow-nanoarrow]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on issue #672:
URL: https://github.com/apache/arrow-nanoarrow/issues/672#issuecomment-2452480552
To get the first round to pass I added:
https://github.com/apache/arrow-nanoarrow/blob/d9cc16420f932b62aa63da9e9ca7d249898742cf/src/nanoarrow/common/utils.c#L359
...but maybe that's in the wrong function?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org