From e1545db930cf514d47a2b45d76ea5eb2fc9613c5 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Mon, 30 Sep 2024 17:06:54 -0500 Subject: [PATCH 01/22] start again --- r/.clang-tidy | 21 +++++++++++++++++++++ src/nanoarrow/common/nanoarrow_hpp_test.cc | 17 +++++++++-------- src/nanoarrow/nanoarrow.hpp | 6 +++++- 3 files changed, 35 insertions(+), 9 deletions(-) create mode 100644 r/.clang-tidy diff --git a/r/.clang-tidy b/r/.clang-tidy new file mode 100644 index 000000000..9a4e8f54c --- /dev/null +++ b/r/.clang-tidy @@ -0,0 +1,21 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +--- +# Disable valist, it's buggy: https://github.com/llvm/llvm-project/issues/40656 +Checks: '-clang-analyzer-valist.Uninitialized' +FormatStyle: google +UseColor: true diff --git a/src/nanoarrow/common/nanoarrow_hpp_test.cc b/src/nanoarrow/common/nanoarrow_hpp_test.cc index 31250f6cd..b76aa4cf0 100644 --- a/src/nanoarrow/common/nanoarrow_hpp_test.cc +++ b/src/nanoarrow/common/nanoarrow_hpp_test.cc @@ -50,7 +50,7 @@ TEST(NanoarrowHppTest, NanoarrowHppUniqueArrayTest) { // move constructor nanoarrow::UniqueArray array2 = std::move(array); - EXPECT_EQ(array->release, nullptr); + EXPECT_EQ(array->release, nullptr); // NOLINT(clang-analyzer-cplusplus.Move) EXPECT_NE(array2->release, nullptr); EXPECT_EQ(array2->length, 1); @@ -71,7 +71,7 @@ TEST(NanoarrowHppTest, NanoarrowHppUniqueSchemaTest) { // move constructor nanoarrow::UniqueSchema schema2 = std::move(schema); - EXPECT_EQ(schema->release, nullptr); + EXPECT_EQ(schema->release, nullptr); // NOLINT(clang-analyzer-cplusplus.Move) EXPECT_NE(schema2->release, nullptr); EXPECT_STREQ(schema2->format, "i"); @@ -101,7 +101,7 @@ TEST(NanoarrowHppTest, NanoarrowHppUniqueArrayStreamTest) { // move constructor nanoarrow::UniqueArrayStream array_stream2 = std::move(array_stream); - EXPECT_EQ(array_stream->release, nullptr); + EXPECT_EQ(array_stream->release, nullptr); // NOLINT(clang-analyzer-cplusplus.Move) EXPECT_NE(array_stream2->release, nullptr); EXPECT_EQ(ArrowArrayStreamGetSchema(array_stream2.get(), schema.get(), nullptr), NANOARROW_OK); @@ -137,8 +137,8 @@ TEST(NanoarrowHppTest, NanoarrowHppUniqueBufferTest) { // move constructor nanoarrow::UniqueBuffer buffer2 = std::move(buffer); - EXPECT_EQ(buffer->data, nullptr); - EXPECT_EQ(buffer->size_bytes, 0); + EXPECT_EQ(buffer->data, nullptr); // NOLINT(clang-analyzer-cplusplus.Move) + EXPECT_EQ(buffer->size_bytes, 0); // NOLINT(clang-analyzer-cplusplus.Move) EXPECT_NE(buffer2->data, nullptr); EXPECT_EQ(buffer2->size_bytes, 123); @@ -161,8 +161,8 @@ TEST(NanoarrowHppTest, NanoarrowHppUniqueBitmapTest) { // move constructor nanoarrow::UniqueBitmap bitmap2 = std::move(bitmap); - EXPECT_EQ(bitmap->buffer.data, nullptr); - EXPECT_EQ(bitmap->size_bits, 0); + EXPECT_EQ(bitmap->buffer.data, nullptr); // NOLINT(clang-analyzer-cplusplus.Move) + EXPECT_EQ(bitmap->size_bits, 0); // NOLINT(clang-analyzer-cplusplus.Move) EXPECT_NE(bitmap2->buffer.data, nullptr); EXPECT_EQ(bitmap2->size_bits, 123); @@ -250,7 +250,8 @@ TEST(NanoarrowHppTest, NanoarrowHppUniqueArrayViewTest) { // move constructor nanoarrow::UniqueArrayView array_view2 = std::move(array_view); - EXPECT_EQ(array_view->storage_type, NANOARROW_TYPE_UNINITIALIZED); + EXPECT_EQ(array_view->storage_type, // NOLINT(clang-analyzer-cplusplus.Move) + NANOARROW_TYPE_UNINITIALIZED); EXPECT_EQ(array_view2->storage_type, NANOARROW_TYPE_STRUCT); // pointer constructor diff --git a/src/nanoarrow/nanoarrow.hpp b/src/nanoarrow/nanoarrow.hpp index 49ba38f0d..97c33db0a 100644 --- a/src/nanoarrow/nanoarrow.hpp +++ b/src/nanoarrow/nanoarrow.hpp @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +#include #include #include #include @@ -216,7 +217,10 @@ template class Unique { public: /// \brief Construct an invalid instance of T holding no resources - Unique() { init_pointer(&data_); } + Unique() { + std::memset(&data_, 0, sizeof(data_)); + init_pointer(&data_); + } /// \brief Move and take ownership of data Unique(T* data) { move_pointer(data, &data_); } From 0093dd07db2d2a00150e75c5b22c6910e75b8c87 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Mon, 30 Sep 2024 17:21:55 -0500 Subject: [PATCH 02/22] fix two more clang-tidy warnings --- src/nanoarrow/common/array.c | 3 +-- src/nanoarrow/common/schema.c | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/nanoarrow/common/array.c b/src/nanoarrow/common/array.c index 9e32ac896..299c0af1f 100644 --- a/src/nanoarrow/common/array.c +++ b/src/nanoarrow/common/array.c @@ -841,8 +841,7 @@ static int ArrowArrayViewValidateMinimal(struct ArrowArrayView* array_view, // is always data dependent for all current Arrow types. for (int i = 0; i < 2; i++) { int64_t element_size_bytes = array_view->layout.element_size_bits[i] / 8; - // Initialize with a value that will cause an error if accidentally used uninitialized - int64_t min_buffer_size_bytes = array_view->buffer_views[i].size_bytes + 1; + int64_t min_buffer_size_bytes; switch (array_view->layout.buffer_type[i]) { case NANOARROW_BUFFER_TYPE_VALIDITY: diff --git a/src/nanoarrow/common/schema.c b/src/nanoarrow/common/schema.c index 93e9ec579..28fb33803 100644 --- a/src/nanoarrow/common/schema.c +++ b/src/nanoarrow/common/schema.c @@ -1346,7 +1346,7 @@ static inline void ArrowToStringLogChars(char** out, int64_t n_chars_last, // In the unlikely snprintf() returning a negative value (encoding error), // ensure the result won't cause an out-of-bounds access. if (n_chars_last < 0) { - n_chars = 0; + n_chars_last = 0; } *n_chars += n_chars_last; From 240f0c3ee68dab23fadec90f73682fb625c31396 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Mon, 30 Sep 2024 17:30:42 -0500 Subject: [PATCH 03/22] ignore dead store --- src/nanoarrow/common/array.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/nanoarrow/common/array.c b/src/nanoarrow/common/array.c index 299c0af1f..3d04d0bc0 100644 --- a/src/nanoarrow/common/array.c +++ b/src/nanoarrow/common/array.c @@ -841,7 +841,10 @@ static int ArrowArrayViewValidateMinimal(struct ArrowArrayView* array_view, // is always data dependent for all current Arrow types. for (int i = 0; i < 2; i++) { int64_t element_size_bytes = array_view->layout.element_size_bits[i] / 8; - int64_t min_buffer_size_bytes; + // Initialize with a value that will cause an error if accidentally used uninitialized + // Need to suppress the clang-tidy warning because gcc warns for possible use + int64_t min_buffer_size_bytes = // NOLINT(clang-analyzer-deadcode.DeadStores) + array_view->buffer_views[i].size_bytes + 1; switch (array_view->layout.buffer_type[i]) { case NANOARROW_BUFFER_TYPE_VALIDITY: From 681094c2f25c8d1955bcec74362395db4168dbc5 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 13:02:24 -0500 Subject: [PATCH 04/22] dcheck for number of decimal words --- src/nanoarrow/common/utils.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nanoarrow/common/utils.c b/src/nanoarrow/common/utils.c index 93e4d4b78..7be65ea4f 100644 --- a/src/nanoarrow/common/utils.c +++ b/src/nanoarrow/common/utils.c @@ -356,6 +356,7 @@ ArrowErrorCode ArrowDecimalSetDigits(struct ArrowDecimal* decimal, // https://github.com/apache/arrow/blob/cd3321b28b0c9703e5d7105d6146c1270bbadd7f/cpp/src/arrow/util/decimal.cc#L365 ArrowErrorCode ArrowDecimalAppendDigitsToBuffer(const struct ArrowDecimal* decimal, struct ArrowBuffer* buffer) { + NANOARROW_DCHECK(decimal->n_words == 2 || decimal->n_words == 4); int is_negative = ArrowDecimalSign(decimal) < 0; uint64_t words_little_endian[4]; From 80cfc9bed5cacbb368a857fca528da0a0056dad7 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 13:20:19 -0500 Subject: [PATCH 05/22] add dcheck --- src/nanoarrow/common/inline_buffer.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/nanoarrow/common/inline_buffer.h b/src/nanoarrow/common/inline_buffer.h index 54623e7f2..a3d9e60e5 100644 --- a/src/nanoarrow/common/inline_buffer.h +++ b/src/nanoarrow/common/inline_buffer.h @@ -214,6 +214,7 @@ static inline ArrowErrorCode ArrowBufferReserve(struct ArrowBuffer* buffer, static inline void ArrowBufferAppendUnsafe(struct ArrowBuffer* buffer, const void* data, int64_t size_bytes) { if (size_bytes > 0) { + NANOARROW_DCHECK(buffer->data != NULL); memcpy(buffer->data + buffer->size_bytes, data, size_bytes); buffer->size_bytes += size_bytes; } @@ -295,8 +296,10 @@ static inline ArrowErrorCode ArrowBufferAppendFill(struct ArrowBuffer* buffer, NANOARROW_RETURN_NOT_OK(ArrowBufferReserve(buffer, size_bytes)); + NANOARROW_DCHECK(buffer->data != NULL); // To help clang-tidy memset(buffer->data + buffer->size_bytes, value, size_bytes); buffer->size_bytes += size_bytes; + return NANOARROW_OK; } @@ -413,6 +416,8 @@ static inline void ArrowBitsUnpackInt32(const uint8_t* bits, int64_t start_offse return; } + NANOARROW_DCHECK(bits != NULL && out != NULL); + const int64_t i_begin = start_offset; const int64_t i_end = start_offset + length; const int64_t i_last_valid = i_end - 1; @@ -461,6 +466,12 @@ static inline void ArrowBitSetTo(uint8_t* bits, int64_t i, uint8_t bit_is_set) { static inline void ArrowBitsSetTo(uint8_t* bits, int64_t start_offset, int64_t length, uint8_t bits_are_set) { + if (length == 0) { + return; + } + + NANOARROW_DCHECK(bits != NULL); + const int64_t i_begin = start_offset; const int64_t i_end = start_offset + length; const uint8_t fill_byte = (uint8_t)(-bits_are_set); @@ -504,6 +515,8 @@ static inline int64_t ArrowBitCountSet(const uint8_t* bits, int64_t start_offset return 0; } + NANOARROW_DCHECK(bits != NULL); + const int64_t i_begin = start_offset; const int64_t i_end = start_offset + length; const int64_t i_last_valid = i_end - 1; From beaef477d3d0ad62f64898a0ac169bb4b2eaaaba Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 13:21:29 -0500 Subject: [PATCH 06/22] dev --- src/nanoarrow/common/inline_buffer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nanoarrow/common/inline_buffer.h b/src/nanoarrow/common/inline_buffer.h index a3d9e60e5..e26c2de96 100644 --- a/src/nanoarrow/common/inline_buffer.h +++ b/src/nanoarrow/common/inline_buffer.h @@ -296,7 +296,7 @@ static inline ArrowErrorCode ArrowBufferAppendFill(struct ArrowBuffer* buffer, NANOARROW_RETURN_NOT_OK(ArrowBufferReserve(buffer, size_bytes)); - NANOARROW_DCHECK(buffer->data != NULL); // To help clang-tidy + NANOARROW_DCHECK(buffer->data != NULL); // To help clang-tidy memset(buffer->data + buffer->size_bytes, value, size_bytes); buffer->size_bytes += size_bytes; From cefedda1f0996a304504edfc150406b2ac51b4f6 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 15:05:44 -0500 Subject: [PATCH 07/22] sort non-flatcc --- r/.clang-tidy => .clang-tidy | 5 +- out.txt | 171 ++++++++++++++++++++++++++++++ src/nanoarrow/ipc/decoder.c | 4 + src/nanoarrow/ipc/ipc_hpp_test.cc | 8 +- 4 files changed, 183 insertions(+), 5 deletions(-) rename r/.clang-tidy => .clang-tidy (73%) create mode 100644 out.txt diff --git a/r/.clang-tidy b/.clang-tidy similarity index 73% rename from r/.clang-tidy rename to .clang-tidy index 9a4e8f54c..35adac39b 100644 --- a/r/.clang-tidy +++ b/.clang-tidy @@ -16,6 +16,9 @@ # under the License. --- # Disable valist, it's buggy: https://github.com/llvm/llvm-project/issues/40656 -Checks: '-clang-analyzer-valist.Uninitialized' +# Disable DeprecatedOrUnsafeBufferHandling because it suggests we replace +# memset and memcpy with memset_s() and memcpy_s() if compiled with C11. Because +# we also support C99, we can't blindly replace those calls. +Checks: '-clang-analyzer-valist.Uninitialized,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling' FormatStyle: google UseColor: true diff --git a/out.txt b/out.txt new file mode 100644 index 000000000..e1e312201 --- /dev/null +++ b/out.txt @@ -0,0 +1,171 @@ +Enabled checks: + clang-analyzer-apiModeling.StdCLibraryFunctions + clang-analyzer-apiModeling.TrustNonnull + clang-analyzer-apiModeling.google.GTest + clang-analyzer-apiModeling.llvm.CastValue + clang-analyzer-apiModeling.llvm.ReturnValue + clang-analyzer-core.CallAndMessage + clang-analyzer-core.CallAndMessageModeling + clang-analyzer-core.DivideZero + clang-analyzer-core.DynamicTypePropagation + clang-analyzer-core.NonNullParamChecker + clang-analyzer-core.NonnilStringConstants + clang-analyzer-core.NullDereference + clang-analyzer-core.StackAddrEscapeBase + clang-analyzer-core.StackAddressEscape + clang-analyzer-core.UndefinedBinaryOperatorResult + clang-analyzer-core.VLASize + clang-analyzer-core.builtin.BuiltinFunctions + clang-analyzer-core.builtin.NoReturnFunctions + clang-analyzer-core.uninitialized.ArraySubscript + clang-analyzer-core.uninitialized.Assign + clang-analyzer-core.uninitialized.Branch + clang-analyzer-core.uninitialized.CapturedBlockVariable + clang-analyzer-core.uninitialized.UndefReturn + clang-analyzer-cplusplus.InnerPointer + clang-analyzer-cplusplus.Move + clang-analyzer-cplusplus.NewDelete + clang-analyzer-cplusplus.NewDeleteLeaks + clang-analyzer-cplusplus.PlacementNew + clang-analyzer-cplusplus.PureVirtualCall + clang-analyzer-cplusplus.SelfAssignment + clang-analyzer-cplusplus.SmartPtrModeling + clang-analyzer-cplusplus.StringChecker + clang-analyzer-cplusplus.VirtualCallModeling + clang-analyzer-deadcode.DeadStores + clang-analyzer-fuchsia.HandleChecker + clang-analyzer-nullability.NullPassedToNonnull + clang-analyzer-nullability.NullReturnedFromNonnull + clang-analyzer-nullability.NullabilityBase + clang-analyzer-nullability.NullableDereferenced + clang-analyzer-nullability.NullablePassedToNonnull + clang-analyzer-nullability.NullableReturnedFromNonnull + clang-analyzer-optin.cplusplus.UninitializedObject + clang-analyzer-optin.cplusplus.VirtualCall + clang-analyzer-optin.mpi.MPI-Checker + clang-analyzer-optin.osx.OSObjectCStyleCast + clang-analyzer-optin.osx.cocoa.localizability.EmptyLocalizationContextChecker + clang-analyzer-optin.osx.cocoa.localizability.NonLocalizedStringChecker + clang-analyzer-optin.performance.GCDAntipattern + clang-analyzer-optin.performance.Padding + clang-analyzer-optin.portability.UnixAPI + clang-analyzer-osx.API + clang-analyzer-osx.MIG + clang-analyzer-osx.NSOrCFErrorDerefChecker + clang-analyzer-osx.NumberObjectConversion + clang-analyzer-osx.OSObjectRetainCount + clang-analyzer-osx.ObjCProperty + clang-analyzer-osx.SecKeychainAPI + clang-analyzer-osx.cocoa.AtSync + clang-analyzer-osx.cocoa.AutoreleaseWrite + clang-analyzer-osx.cocoa.ClassRelease + clang-analyzer-osx.cocoa.Dealloc + clang-analyzer-osx.cocoa.IncompatibleMethodTypes + clang-analyzer-osx.cocoa.Loops + clang-analyzer-osx.cocoa.MissingSuperCall + clang-analyzer-osx.cocoa.NSAutoreleasePool + clang-analyzer-osx.cocoa.NSError + clang-analyzer-osx.cocoa.NilArg + clang-analyzer-osx.cocoa.NonNilReturnValue + clang-analyzer-osx.cocoa.ObjCGenerics + clang-analyzer-osx.cocoa.RetainCount + clang-analyzer-osx.cocoa.RetainCountBase + clang-analyzer-osx.cocoa.RunLoopAutoreleaseLeak + clang-analyzer-osx.cocoa.SelfInit + clang-analyzer-osx.cocoa.SuperDealloc + clang-analyzer-osx.cocoa.UnusedIvars + clang-analyzer-osx.cocoa.VariadicMethodTypes + clang-analyzer-osx.coreFoundation.CFError + clang-analyzer-osx.coreFoundation.CFNumber + clang-analyzer-osx.coreFoundation.CFRetainRelease + clang-analyzer-osx.coreFoundation.containers.OutOfBounds + clang-analyzer-osx.coreFoundation.containers.PointerSizedValues + clang-analyzer-security.FloatLoopCounter + clang-analyzer-security.insecureAPI.SecuritySyntaxChecker + clang-analyzer-security.insecureAPI.UncheckedReturn + clang-analyzer-security.insecureAPI.bcmp + clang-analyzer-security.insecureAPI.bcopy + clang-analyzer-security.insecureAPI.bzero + clang-analyzer-security.insecureAPI.decodeValueOfObjCType + clang-analyzer-security.insecureAPI.getpw + clang-analyzer-security.insecureAPI.gets + clang-analyzer-security.insecureAPI.mkstemp + clang-analyzer-security.insecureAPI.mktemp + clang-analyzer-security.insecureAPI.rand + clang-analyzer-security.insecureAPI.strcpy + clang-analyzer-security.insecureAPI.vfork + clang-analyzer-unix.API + clang-analyzer-unix.DynamicMemoryModeling + clang-analyzer-unix.Malloc + clang-analyzer-unix.MallocSizeof + clang-analyzer-unix.MismatchedDeallocator + clang-analyzer-unix.Vfork + clang-analyzer-unix.cstring.BadSizeArg + clang-analyzer-unix.cstring.CStringModeling + clang-analyzer-unix.cstring.NullArg + clang-analyzer-valist.CopyToSelf + clang-analyzer-valist.Unterminated + clang-analyzer-valist.ValistBase + clang-analyzer-webkit.NoUncountedMemberChecker + clang-analyzer-webkit.RefCntblBaseVirtualDtor + clang-analyzer-webkit.UncountedLambdaCapturesChecker + +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/array_stream.c +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/reader.c +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/utils.c +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/refmap.c +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/verifier.c +/home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/verifier.c:662:15: warning: Although the value stored to 'vte_type' is used in the enclosing expression, the value is never actually read from 'vte_type' [clang-analyzer-deadcode.DeadStores] + if (0 == (vte_type = read_vt_entry(td, id - 1))) { + ^ ~~~~~~~~~~~~~~~~~~~~~~~~~ +/home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/verifier.c:662:15: note: Although the value stored to 'vte_type' is used in the enclosing expression, the value is never actually read from 'vte_type' + if (0 == (vte_type = read_vt_entry(td, id - 1))) { + ^ ~~~~~~~~~~~~~~~~~~~~~~~~~ +/home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/verifier.c:663:19: warning: Although the value stored to 'vte_table' is used in the enclosing expression, the value is never actually read from 'vte_table' [clang-analyzer-deadcode.DeadStores] + if (0 == (vte_table = read_vt_entry(td, id))) { + ^ ~~~~~~~~~~~~~~~~~~~~~ +/home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/verifier.c:663:19: note: Although the value stored to 'vte_table' is used in the enclosing expression, the value is never actually read from 'vte_table' + if (0 == (vte_table = read_vt_entry(td, id))) { + ^ ~~~~~~~~~~~~~~~~~~~~~ +2 warnings generated. +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/build/_deps/googletest-src/googletest/src/gtest_main.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/emitter.c +/home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/emitter.c:154:28: warning: Value stored to 'p' during its initialization is never read [clang-analyzer-deadcode.DeadStores] + flatcc_emitter_page_t *p = E->front; + ^ ~~~~~~~~ +/home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/emitter.c:154:28: note: Value stored to 'p' during its initialization is never read + flatcc_emitter_page_t *p = E->front; + ^ ~~~~~~~~ +1 warning generated. +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/writer.c +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/build/_deps/googletest-src/googlemock/src/gmock-all.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/build/_deps/googletest-src/googlemock/src/gmock_main.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/build/_deps/googletest-src/googletest/src/gtest-all.cc +1 warning generated. +Suppressed 1 warnings (1 in non-user code). +Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/integration/ipc_integration.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/encoder.c +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/integration/c_data_integration.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/integration/c_data_integration_test.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/encoder_test.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/array_stream_test.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/schema.c +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/ipc_hpp_test.cc +Suppressed 4 warnings (4 NOLINT). +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/builder.c +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/files_test.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/writer_test.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/decoder.c +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/array.c +Suppressed 1 warnings (1 NOLINT). +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/testing/testing.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/reader_test.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/utils_test.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/nanoarrow_hpp_test.cc +Suppressed 6 warnings (6 NOLINT). +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/decoder_test.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/buffer_test.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/testing/testing_test.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/schema_test.cc +clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/array_test.cc diff --git a/src/nanoarrow/ipc/decoder.c b/src/nanoarrow/ipc/decoder.c index dcde43359..962227240 100644 --- a/src/nanoarrow/ipc/decoder.c +++ b/src/nanoarrow/ipc/decoder.c @@ -1506,6 +1506,9 @@ struct ArrowIpcIntervalMonthDayNano { static int ArrowIpcDecoderSwapEndian(struct ArrowIpcBufferSource* src, struct ArrowBufferView* out_view, struct ArrowBuffer* dst, struct ArrowError* error) { + NANOARROW_DCHECK(out_view->size_bytes > 0); + NANOARROW_DCHECK(out_view->data.data != NULL); + // Some buffer data types don't need any endian swapping switch (src->data_type) { case NANOARROW_TYPE_BOOL: @@ -1539,6 +1542,7 @@ static int ArrowIpcDecoderSwapEndian(struct ArrowIpcBufferSource* src, uint64_t* ptr_dst = (uint64_t*)dst->data; uint64_t words[4]; int n_words = (int)(src->element_size_bits / 64); + NANOARROW_DCHECK(n_words == 2 || n_words == 4); for (int64_t i = 0; i < (dst->size_bytes / n_words / 8); i++) { for (int j = 0; j < n_words; j++) { diff --git a/src/nanoarrow/ipc/ipc_hpp_test.cc b/src/nanoarrow/ipc/ipc_hpp_test.cc index ab5dcb687..cb3b29898 100644 --- a/src/nanoarrow/ipc/ipc_hpp_test.cc +++ b/src/nanoarrow/ipc/ipc_hpp_test.cc @@ -28,7 +28,7 @@ TEST(NanoarrowIpcHppTest, NanoarrowIpcHppTestUniqueDecoder) { nanoarrow::ipc::UniqueDecoder decoder2 = std::move(decoder); EXPECT_NE(decoder2->private_data, nullptr); - EXPECT_EQ(decoder->private_data, nullptr); + EXPECT_EQ(decoder->private_data, nullptr); // NOLINT(clang-analyzer-cplusplus.Move) } TEST(NanoarrowIpcHppTest, NanoarrowIpcHppTestUniqueEncoder) { @@ -40,7 +40,7 @@ TEST(NanoarrowIpcHppTest, NanoarrowIpcHppTestUniqueEncoder) { nanoarrow::ipc::UniqueEncoder encoder2 = std::move(encoder); EXPECT_NE(encoder2->private_data, nullptr); - EXPECT_EQ(encoder->private_data, nullptr); + EXPECT_EQ(encoder->private_data, nullptr); // NOLINT(clang-analyzer-cplusplus.Move) } TEST(NanoarrowIpcHppTest, NanoarrowIpcHppTestUniqueInputStream) { @@ -54,7 +54,7 @@ TEST(NanoarrowIpcHppTest, NanoarrowIpcHppTestUniqueInputStream) { nanoarrow::ipc::UniqueInputStream input2 = std::move(input); EXPECT_NE(input2->release, nullptr); - EXPECT_EQ(input->release, nullptr); + EXPECT_EQ(input->release, nullptr); // NOLINT(clang-analyzer-cplusplus.Move) } TEST(NanoarrowIpcHppTest, NanoarrowIpcHppTestUniqueOutputStream) { @@ -68,5 +68,5 @@ TEST(NanoarrowIpcHppTest, NanoarrowIpcHppTestUniqueOutputStream) { nanoarrow::ipc::UniqueOutputStream output2 = std::move(output); EXPECT_NE(output2->release, nullptr); - EXPECT_EQ(output->release, nullptr); + EXPECT_EQ(output->release, nullptr); // NOLINT(clang-analyzer-cplusplus.Move) } From e80beb78562ff3941be7ba03c72af1eaac203475 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 15:22:39 -0500 Subject: [PATCH 08/22] maybe add ci job --- .github/workflows/clang-tidy.yaml | 89 +++++++++++++++++++++++++++++++ ci/scripts/run-clang-tidy.sh | 34 ++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 .github/workflows/clang-tidy.yaml create mode 100755 ci/scripts/run-clang-tidy.sh diff --git a/.github/workflows/clang-tidy.yaml b/.github/workflows/clang-tidy.yaml new file mode 100644 index 000000000..b6c9bcb30 --- /dev/null +++ b/.github/workflows/clang-tidy.yaml @@ -0,0 +1,89 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: clang-tidy + +on: + push: + branches: + - main + pull_request: + branches: + - main + paths: + - 'CMakeLists.txt' + - '.github/workflows/clang-tidy.yaml' + - 'src/nanoarrow/**' + +permissions: + contents: read + +jobs: + test-c: + + runs-on: ubuntu-latest + + name: ${{ matrix.config.label }} + + strategy: + fail-fast: false + matrix: + config: + - {label: common} + - {label: device, cmake_args: "-DNANOARROW_DEVICE=ON"} + - {label: ipc, cmake_args: "-DNANOARROW_IPC=ON"} + + steps: + - uses: actions/checkout@v4 + + - name: Install clang-tidy + if: matrix.config.label == 'default-build' + run: | + sudo apt-get update && sudo apt-get install -y clang-tidy + + - name: Cache Arrow C++ Build + id: cache-arrow-build + uses: actions/cache@v4 + with: + path: arrow + # Bump the number at the end of this line to force a new Arrow C++ build + key: arrow-${{ runner.os }}-${{ runner.arch }}-1 + + - name: Build Arrow C++ + if: steps.cache-arrow-build.outputs.cache-hit != 'true' + shell: bash + run: | + ci/scripts/build-arrow-cpp-minimal.sh 15.0.2 arrow + + - name: Build nanoarrow + run: | + export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:`pwd`/dist/lib + sudo ldconfig + + ARROW_PATH="$(pwd)/arrow" + mkdir build + cd build + + cmake .. -DNANOARROW_BUILD_TESTS=ON -DCMAKE_POSITION_INDEPENDENT_CODE=ON \ + -DCMAKE_BUILD_TYPE=Debug \ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ + -DCMAKE_PREFIX_PATH="${ARROW_PATH}" ${{ matrix.config.cmake_args }} + + cmake --build . + + - name: Run clang-tidy + run: | + ci/scripts/run-clang-tidy . build/ diff --git a/ci/scripts/run-clang-tidy.sh b/ci/scripts/run-clang-tidy.sh new file mode 100755 index 000000000..b50d04448 --- /dev/null +++ b/ci/scripts/run-clang-tidy.sh @@ -0,0 +1,34 @@ +#!/usr/bin/env bash +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +set -e +set -o pipefail + +if [ ${VERBOSE:-0} -gt 0 ]; then + set -x +fi + +main() { + local -r source_dir="${1}" + local -r build_dir="${2}" + + run-clang-tidy -p "${build_dir}" -j$(nproc) -extra-arg=-Wno-unknown-warning-option +} + +main "$@" From cd1996518acd90a3453f84813cb4ea5d6fa73d62 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 15:46:19 -0500 Subject: [PATCH 09/22] typo --- .github/workflows/clang-tidy.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/clang-tidy.yaml b/.github/workflows/clang-tidy.yaml index b6c9bcb30..14d67df44 100644 --- a/.github/workflows/clang-tidy.yaml +++ b/.github/workflows/clang-tidy.yaml @@ -86,4 +86,4 @@ jobs: - name: Run clang-tidy run: | - ci/scripts/run-clang-tidy . build/ + ci/scripts/run-clang-tidy.sh . build/ From d2f6bc1f27d9966e3ee08ac2421e53be85537dc6 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 15:46:54 -0500 Subject: [PATCH 10/22] remove old file --- out.txt | 171 -------------------------------------------------------- 1 file changed, 171 deletions(-) delete mode 100644 out.txt diff --git a/out.txt b/out.txt deleted file mode 100644 index e1e312201..000000000 --- a/out.txt +++ /dev/null @@ -1,171 +0,0 @@ -Enabled checks: - clang-analyzer-apiModeling.StdCLibraryFunctions - clang-analyzer-apiModeling.TrustNonnull - clang-analyzer-apiModeling.google.GTest - clang-analyzer-apiModeling.llvm.CastValue - clang-analyzer-apiModeling.llvm.ReturnValue - clang-analyzer-core.CallAndMessage - clang-analyzer-core.CallAndMessageModeling - clang-analyzer-core.DivideZero - clang-analyzer-core.DynamicTypePropagation - clang-analyzer-core.NonNullParamChecker - clang-analyzer-core.NonnilStringConstants - clang-analyzer-core.NullDereference - clang-analyzer-core.StackAddrEscapeBase - clang-analyzer-core.StackAddressEscape - clang-analyzer-core.UndefinedBinaryOperatorResult - clang-analyzer-core.VLASize - clang-analyzer-core.builtin.BuiltinFunctions - clang-analyzer-core.builtin.NoReturnFunctions - clang-analyzer-core.uninitialized.ArraySubscript - clang-analyzer-core.uninitialized.Assign - clang-analyzer-core.uninitialized.Branch - clang-analyzer-core.uninitialized.CapturedBlockVariable - clang-analyzer-core.uninitialized.UndefReturn - clang-analyzer-cplusplus.InnerPointer - clang-analyzer-cplusplus.Move - clang-analyzer-cplusplus.NewDelete - clang-analyzer-cplusplus.NewDeleteLeaks - clang-analyzer-cplusplus.PlacementNew - clang-analyzer-cplusplus.PureVirtualCall - clang-analyzer-cplusplus.SelfAssignment - clang-analyzer-cplusplus.SmartPtrModeling - clang-analyzer-cplusplus.StringChecker - clang-analyzer-cplusplus.VirtualCallModeling - clang-analyzer-deadcode.DeadStores - clang-analyzer-fuchsia.HandleChecker - clang-analyzer-nullability.NullPassedToNonnull - clang-analyzer-nullability.NullReturnedFromNonnull - clang-analyzer-nullability.NullabilityBase - clang-analyzer-nullability.NullableDereferenced - clang-analyzer-nullability.NullablePassedToNonnull - clang-analyzer-nullability.NullableReturnedFromNonnull - clang-analyzer-optin.cplusplus.UninitializedObject - clang-analyzer-optin.cplusplus.VirtualCall - clang-analyzer-optin.mpi.MPI-Checker - clang-analyzer-optin.osx.OSObjectCStyleCast - clang-analyzer-optin.osx.cocoa.localizability.EmptyLocalizationContextChecker - clang-analyzer-optin.osx.cocoa.localizability.NonLocalizedStringChecker - clang-analyzer-optin.performance.GCDAntipattern - clang-analyzer-optin.performance.Padding - clang-analyzer-optin.portability.UnixAPI - clang-analyzer-osx.API - clang-analyzer-osx.MIG - clang-analyzer-osx.NSOrCFErrorDerefChecker - clang-analyzer-osx.NumberObjectConversion - clang-analyzer-osx.OSObjectRetainCount - clang-analyzer-osx.ObjCProperty - clang-analyzer-osx.SecKeychainAPI - clang-analyzer-osx.cocoa.AtSync - clang-analyzer-osx.cocoa.AutoreleaseWrite - clang-analyzer-osx.cocoa.ClassRelease - clang-analyzer-osx.cocoa.Dealloc - clang-analyzer-osx.cocoa.IncompatibleMethodTypes - clang-analyzer-osx.cocoa.Loops - clang-analyzer-osx.cocoa.MissingSuperCall - clang-analyzer-osx.cocoa.NSAutoreleasePool - clang-analyzer-osx.cocoa.NSError - clang-analyzer-osx.cocoa.NilArg - clang-analyzer-osx.cocoa.NonNilReturnValue - clang-analyzer-osx.cocoa.ObjCGenerics - clang-analyzer-osx.cocoa.RetainCount - clang-analyzer-osx.cocoa.RetainCountBase - clang-analyzer-osx.cocoa.RunLoopAutoreleaseLeak - clang-analyzer-osx.cocoa.SelfInit - clang-analyzer-osx.cocoa.SuperDealloc - clang-analyzer-osx.cocoa.UnusedIvars - clang-analyzer-osx.cocoa.VariadicMethodTypes - clang-analyzer-osx.coreFoundation.CFError - clang-analyzer-osx.coreFoundation.CFNumber - clang-analyzer-osx.coreFoundation.CFRetainRelease - clang-analyzer-osx.coreFoundation.containers.OutOfBounds - clang-analyzer-osx.coreFoundation.containers.PointerSizedValues - clang-analyzer-security.FloatLoopCounter - clang-analyzer-security.insecureAPI.SecuritySyntaxChecker - clang-analyzer-security.insecureAPI.UncheckedReturn - clang-analyzer-security.insecureAPI.bcmp - clang-analyzer-security.insecureAPI.bcopy - clang-analyzer-security.insecureAPI.bzero - clang-analyzer-security.insecureAPI.decodeValueOfObjCType - clang-analyzer-security.insecureAPI.getpw - clang-analyzer-security.insecureAPI.gets - clang-analyzer-security.insecureAPI.mkstemp - clang-analyzer-security.insecureAPI.mktemp - clang-analyzer-security.insecureAPI.rand - clang-analyzer-security.insecureAPI.strcpy - clang-analyzer-security.insecureAPI.vfork - clang-analyzer-unix.API - clang-analyzer-unix.DynamicMemoryModeling - clang-analyzer-unix.Malloc - clang-analyzer-unix.MallocSizeof - clang-analyzer-unix.MismatchedDeallocator - clang-analyzer-unix.Vfork - clang-analyzer-unix.cstring.BadSizeArg - clang-analyzer-unix.cstring.CStringModeling - clang-analyzer-unix.cstring.NullArg - clang-analyzer-valist.CopyToSelf - clang-analyzer-valist.Unterminated - clang-analyzer-valist.ValistBase - clang-analyzer-webkit.NoUncountedMemberChecker - clang-analyzer-webkit.RefCntblBaseVirtualDtor - clang-analyzer-webkit.UncountedLambdaCapturesChecker - -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/array_stream.c -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/reader.c -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/utils.c -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/refmap.c -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/verifier.c -/home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/verifier.c:662:15: warning: Although the value stored to 'vte_type' is used in the enclosing expression, the value is never actually read from 'vte_type' [clang-analyzer-deadcode.DeadStores] - if (0 == (vte_type = read_vt_entry(td, id - 1))) { - ^ ~~~~~~~~~~~~~~~~~~~~~~~~~ -/home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/verifier.c:662:15: note: Although the value stored to 'vte_type' is used in the enclosing expression, the value is never actually read from 'vte_type' - if (0 == (vte_type = read_vt_entry(td, id - 1))) { - ^ ~~~~~~~~~~~~~~~~~~~~~~~~~ -/home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/verifier.c:663:19: warning: Although the value stored to 'vte_table' is used in the enclosing expression, the value is never actually read from 'vte_table' [clang-analyzer-deadcode.DeadStores] - if (0 == (vte_table = read_vt_entry(td, id))) { - ^ ~~~~~~~~~~~~~~~~~~~~~ -/home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/verifier.c:663:19: note: Although the value stored to 'vte_table' is used in the enclosing expression, the value is never actually read from 'vte_table' - if (0 == (vte_table = read_vt_entry(td, id))) { - ^ ~~~~~~~~~~~~~~~~~~~~~ -2 warnings generated. -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/build/_deps/googletest-src/googletest/src/gtest_main.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/emitter.c -/home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/emitter.c:154:28: warning: Value stored to 'p' during its initialization is never read [clang-analyzer-deadcode.DeadStores] - flatcc_emitter_page_t *p = E->front; - ^ ~~~~~~~~ -/home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/emitter.c:154:28: note: Value stored to 'p' during its initialization is never read - flatcc_emitter_page_t *p = E->front; - ^ ~~~~~~~~ -1 warning generated. -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/writer.c -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/build/_deps/googletest-src/googlemock/src/gmock-all.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/build/_deps/googletest-src/googlemock/src/gmock_main.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/build/_deps/googletest-src/googletest/src/gtest-all.cc -1 warning generated. -Suppressed 1 warnings (1 in non-user code). -Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/integration/ipc_integration.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/encoder.c -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/integration/c_data_integration.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/integration/c_data_integration_test.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/encoder_test.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/array_stream_test.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/schema.c -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/ipc_hpp_test.cc -Suppressed 4 warnings (4 NOLINT). -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/thirdparty/flatcc/src/runtime/builder.c -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/files_test.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/writer_test.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/decoder.c -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/array.c -Suppressed 1 warnings (1 NOLINT). -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/testing/testing.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/reader_test.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/utils_test.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/nanoarrow_hpp_test.cc -Suppressed 6 warnings (6 NOLINT). -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/ipc/decoder_test.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/buffer_test.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/testing/testing_test.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/schema_test.cc -clang-tidy-14 --use-color -extra-arg=-Wno-unknown-warning-option -p=build/ /home/dewey/gh/arrow-nanoarrow/src/nanoarrow/common/array_test.cc From d2f5b106e76871bf79daa9950ffa4686ea457c13 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 15:50:49 -0500 Subject: [PATCH 11/22] whoops --- .github/workflows/clang-tidy.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/clang-tidy.yaml b/.github/workflows/clang-tidy.yaml index 14d67df44..443ebac4e 100644 --- a/.github/workflows/clang-tidy.yaml +++ b/.github/workflows/clang-tidy.yaml @@ -79,7 +79,7 @@ jobs: cd build cmake .. -DNANOARROW_BUILD_TESTS=ON -DCMAKE_POSITION_INDEPENDENT_CODE=ON \ - -DCMAKE_BUILD_TYPE=Debug \ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ + -DCMAKE_BUILD_TYPE=Debug -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ -DCMAKE_PREFIX_PATH="${ARROW_PATH}" ${{ matrix.config.cmake_args }} cmake --build . From 39e0316ec4a3888365b7bf2aae16cc2a1401da87 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 15:51:18 -0500 Subject: [PATCH 12/22] better name --- .github/workflows/clang-tidy.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/clang-tidy.yaml b/.github/workflows/clang-tidy.yaml index 443ebac4e..1bf7a2584 100644 --- a/.github/workflows/clang-tidy.yaml +++ b/.github/workflows/clang-tidy.yaml @@ -33,7 +33,7 @@ permissions: contents: read jobs: - test-c: + clang-tidy: runs-on: ubuntu-latest From fbe88d04137dc4b868cb0a1902fc350631d6406f Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 16:02:59 -0500 Subject: [PATCH 13/22] maybe simplify and ensure build errors --- .github/workflows/clang-tidy.yaml | 18 +++--------------- ci/scripts/run-clang-tidy.sh | 4 +++- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/.github/workflows/clang-tidy.yaml b/.github/workflows/clang-tidy.yaml index 1bf7a2584..2e4c4b4cf 100644 --- a/.github/workflows/clang-tidy.yaml +++ b/.github/workflows/clang-tidy.yaml @@ -39,22 +39,9 @@ jobs: name: ${{ matrix.config.label }} - strategy: - fail-fast: false - matrix: - config: - - {label: common} - - {label: device, cmake_args: "-DNANOARROW_DEVICE=ON"} - - {label: ipc, cmake_args: "-DNANOARROW_IPC=ON"} - steps: - uses: actions/checkout@v4 - - name: Install clang-tidy - if: matrix.config.label == 'default-build' - run: | - sudo apt-get update && sudo apt-get install -y clang-tidy - - name: Cache Arrow C++ Build id: cache-arrow-build uses: actions/cache@v4 @@ -78,9 +65,10 @@ jobs: mkdir build cd build - cmake .. -DNANOARROW_BUILD_TESTS=ON -DCMAKE_POSITION_INDEPENDENT_CODE=ON \ + cmake .. -DNANOARROW_DEVICE=ON -DNANOARROW_IPC=ON \ + -DNANOARROW_BUILD_TESTS=ON -DCMAKE_POSITION_INDEPENDENT_CODE=ON \ -DCMAKE_BUILD_TYPE=Debug -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ - -DCMAKE_PREFIX_PATH="${ARROW_PATH}" ${{ matrix.config.cmake_args }} + -DCMAKE_PREFIX_PATH="${ARROW_PATH}" cmake --build . diff --git a/ci/scripts/run-clang-tidy.sh b/ci/scripts/run-clang-tidy.sh index b50d04448..f126c36cf 100755 --- a/ci/scripts/run-clang-tidy.sh +++ b/ci/scripts/run-clang-tidy.sh @@ -28,7 +28,9 @@ main() { local -r source_dir="${1}" local -r build_dir="${2}" - run-clang-tidy -p "${build_dir}" -j$(nproc) -extra-arg=-Wno-unknown-warning-option + run-clang-tidy -p "${build_dir}" -j$(nproc) \ + -extra-arg=-Wno-unknown-warning-option \ + --warnings-as-errors } main "$@" From 099258b3bf60fadcd08bd50c1f2e39ac18904a65 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 16:13:29 -0500 Subject: [PATCH 14/22] see if this does it --- ci/scripts/run-clang-tidy.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/scripts/run-clang-tidy.sh b/ci/scripts/run-clang-tidy.sh index f126c36cf..c27187565 100755 --- a/ci/scripts/run-clang-tidy.sh +++ b/ci/scripts/run-clang-tidy.sh @@ -30,7 +30,7 @@ main() { run-clang-tidy -p "${build_dir}" -j$(nproc) \ -extra-arg=-Wno-unknown-warning-option \ - --warnings-as-errors + -fix } main "$@" From 00fb6adf4ba8f9d8b1f342d5ee3b0f69f77605d3 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 16:27:43 -0500 Subject: [PATCH 15/22] fix device errors --- .clang-tidy | 1 - src/nanoarrow/device/device_hpp_test.cc | 9 +++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 35adac39b..6d48ab986 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -21,4 +21,3 @@ # we also support C99, we can't blindly replace those calls. Checks: '-clang-analyzer-valist.Uninitialized,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling' FormatStyle: google -UseColor: true diff --git a/src/nanoarrow/device/device_hpp_test.cc b/src/nanoarrow/device/device_hpp_test.cc index 1b4c8d8e9..3041e32e8 100644 --- a/src/nanoarrow/device/device_hpp_test.cc +++ b/src/nanoarrow/device/device_hpp_test.cc @@ -27,7 +27,7 @@ TEST(NanoarrowDeviceHpp, UniqueDeviceArray) { ASSERT_NE(array->array.release, nullptr); nanoarrow::device::UniqueDeviceArray array2 = std::move(array); - ASSERT_EQ(array->array.release, nullptr); + ASSERT_EQ(array->array.release, nullptr); // NOLINT(clang-analyzer-cplusplus.Move) ASSERT_NE(array2->array.release, nullptr); } @@ -46,7 +46,7 @@ TEST(NanoarrowDeviceHpp, UniqueDeviceArrayStream) { ASSERT_NE(stream->release, nullptr); nanoarrow::device::UniqueDeviceArrayStream stream2 = std::move(stream); - ASSERT_EQ(stream->release, nullptr); + ASSERT_EQ(stream->release, nullptr); // NOLINT(clang-analyzer-cplusplus.Move) ASSERT_NE(stream2->release, nullptr); } @@ -57,7 +57,7 @@ TEST(NanoarrowDeviceHpp, UniqueDevice) { ArrowDeviceInitCpu(device.get()); nanoarrow::device::UniqueDevice device2 = std::move(device); - ASSERT_EQ(device->release, nullptr); + ASSERT_EQ(device->release, nullptr); // NOLINT(clang-analyzer-cplusplus.Move) ASSERT_NE(device2->release, nullptr); } @@ -71,5 +71,6 @@ TEST(NanoarrowDeviceHpp, UniqueDeviceArrayView) { nanoarrow::device::UniqueDeviceArrayView array_view2 = std::move(array_view); ASSERT_EQ(array_view2->array_view.storage_type, NANOARROW_TYPE_INT32); - ASSERT_EQ(array_view->array_view.storage_type, NANOARROW_TYPE_UNINITIALIZED); + ASSERT_EQ(array_view->array_view.storage_type, // NOLINT(clang-analyzer-cplusplus.Move) + NANOARROW_TYPE_UNINITIALIZED); } From 64f5a4c353da3114a469480b416a14ea6d1115e3 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 16:32:20 -0500 Subject: [PATCH 16/22] see if this errors --- ci/scripts/run-clang-tidy.sh | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/ci/scripts/run-clang-tidy.sh b/ci/scripts/run-clang-tidy.sh index c27187565..8593efb10 100755 --- a/ci/scripts/run-clang-tidy.sh +++ b/ci/scripts/run-clang-tidy.sh @@ -18,19 +18,18 @@ # under the License. set -e -set -o pipefail - -if [ ${VERBOSE:-0} -gt 0 ]; then - set -x -fi main() { local -r source_dir="${1}" local -r build_dir="${2}" + set -x + run-clang-tidy -p "${build_dir}" -j$(nproc) \ -extra-arg=-Wno-unknown-warning-option \ -fix + + set +x } main "$@" From 76370fd7139dac6ffe1d988e153e274709119fad Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 16:56:14 -0500 Subject: [PATCH 17/22] basic level of erroring for errors --- ci/scripts/run-clang-tidy.sh | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ci/scripts/run-clang-tidy.sh b/ci/scripts/run-clang-tidy.sh index 8593efb10..11c7131d0 100755 --- a/ci/scripts/run-clang-tidy.sh +++ b/ci/scripts/run-clang-tidy.sh @@ -26,8 +26,15 @@ main() { set -x run-clang-tidy -p "${build_dir}" -j$(nproc) \ - -extra-arg=-Wno-unknown-warning-option \ - -fix + -extra-arg=-Wno-unknown-warning-option | \ + tee "${build_dir}/clang-tidy-output.txt" + + if grep -e "warning:" -e "error:" clang-tidy-output.txt; then + echo "Warnings or errors found!" + exit 1 + else + echo "No warnings or errors found!" + fi set +x } From 84b555e3b693141479490a6f33a482a400c92bbe Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 16:57:48 -0500 Subject: [PATCH 18/22] fix file --- ci/scripts/run-clang-tidy.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/scripts/run-clang-tidy.sh b/ci/scripts/run-clang-tidy.sh index 11c7131d0..58bf12b4d 100755 --- a/ci/scripts/run-clang-tidy.sh +++ b/ci/scripts/run-clang-tidy.sh @@ -29,7 +29,7 @@ main() { -extra-arg=-Wno-unknown-warning-option | \ tee "${build_dir}/clang-tidy-output.txt" - if grep -e "warning:" -e "error:" clang-tidy-output.txt; then + if grep -e "warning:" -e "error:" "${build_dir}/clang-tidy-output.txt"; then echo "Warnings or errors found!" exit 1 else From f049c122b6d13ca3583aaaa807d1e052dd4cd85b Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 22:16:52 -0500 Subject: [PATCH 19/22] one more from macos --- src/nanoarrow/nanoarrow.hpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/nanoarrow/nanoarrow.hpp b/src/nanoarrow/nanoarrow.hpp index 97c33db0a..b9470bec9 100644 --- a/src/nanoarrow/nanoarrow.hpp +++ b/src/nanoarrow/nanoarrow.hpp @@ -223,7 +223,10 @@ class Unique { } /// \brief Move and take ownership of data - Unique(T* data) { move_pointer(data, &data_); } + Unique(T* data) { + std::memset(&data_, 0, sizeof(data_)); + move_pointer(data, &data_); + } /// \brief Move and take ownership of data wrapped by rhs Unique(Unique&& rhs) : Unique(rhs.get()) {} From f6cabcb757f5d7c45daebac5128d642c6e21dba4 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 22:19:42 -0500 Subject: [PATCH 20/22] fix a metal one --- src/nanoarrow/device/metal_test.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/nanoarrow/device/metal_test.cc b/src/nanoarrow/device/metal_test.cc index 65b1557ee..009d6a755 100644 --- a/src/nanoarrow/device/metal_test.cc +++ b/src/nanoarrow/device/metal_test.cc @@ -75,7 +75,6 @@ TEST(NanoarrowDeviceMetal, DeviceGpuBufferMove) { struct ArrowBufferView view = {data, sizeof(data)}; ASSERT_EQ(ArrowDeviceBufferInit(cpu, view, gpu, &buffer), NANOARROW_OK); - auto mtl_buffer = reinterpret_cast(buffer.data); // GPU -> GPU: just a move uint8_t* old_ptr = buffer.data; From e7d4ef571d24ff0efc310f975cc0de7b25b8578b Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 22:29:32 -0500 Subject: [PATCH 21/22] ensure clang-tidy script works on macos --- ci/scripts/run-clang-tidy.sh | 8 +++++++- src/nanoarrow/device/device.c | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/ci/scripts/run-clang-tidy.sh b/ci/scripts/run-clang-tidy.sh index 58bf12b4d..62a9d77a9 100755 --- a/ci/scripts/run-clang-tidy.sh +++ b/ci/scripts/run-clang-tidy.sh @@ -23,9 +23,15 @@ main() { local -r source_dir="${1}" local -r build_dir="${2}" + if [ $(uname) = "Darwin" ]; then + local -r jobs=$(sysctl -n hw.ncpu) + else + local -r jobs=$(nproc) + fi + set -x - run-clang-tidy -p "${build_dir}" -j$(nproc) \ + run-clang-tidy -p "${build_dir}" -j$jobs \ -extra-arg=-Wno-unknown-warning-option | \ tee "${build_dir}/clang-tidy-output.txt" diff --git a/src/nanoarrow/device/device.c b/src/nanoarrow/device/device.c index 3d2f07af6..8178b9526 100644 --- a/src/nanoarrow/device/device.c +++ b/src/nanoarrow/device/device.c @@ -492,7 +492,7 @@ static ArrowErrorCode ArrowDeviceArrayViewEnsureBufferSizesAsync( NANOARROW_DCHECK(cursor == (buffer.data + buffer.size_bytes)); ArrowBufferReset(&buffer); - return NANOARROW_OK; + return result; } ArrowErrorCode ArrowDeviceArrayViewSetArrayAsync( From b7ce2bf268bced08c5b907179de7e19bd3953175 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Tue, 1 Oct 2024 22:33:23 -0500 Subject: [PATCH 22/22] maybe fix clang-tidy --- thirdparty/flatcc/src/runtime/emitter.c | 2 +- thirdparty/flatcc/src/runtime/verifier.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/thirdparty/flatcc/src/runtime/emitter.c b/thirdparty/flatcc/src/runtime/emitter.c index 089ea00b2..6f7319505 100644 --- a/thirdparty/flatcc/src/runtime/emitter.c +++ b/thirdparty/flatcc/src/runtime/emitter.c @@ -151,7 +151,7 @@ int flatcc_emitter_recycle_page(flatcc_emitter_t *E, flatcc_emitter_page_t *p) void flatcc_emitter_reset(flatcc_emitter_t *E) { - flatcc_emitter_page_t *p = E->front; + flatcc_emitter_page_t *p = E->front; // NOLINT(clang-analyzer-deadcode.DeadStores) if (!E->front) { return; diff --git a/thirdparty/flatcc/src/runtime/verifier.c b/thirdparty/flatcc/src/runtime/verifier.c index 14003c067..4e96a1e40 100644 --- a/thirdparty/flatcc/src/runtime/verifier.c +++ b/thirdparty/flatcc/src/runtime/verifier.c @@ -659,8 +659,8 @@ int flatcc_verify_union_vector_field(flatcc_table_verifier_descriptor_t *td, const utype_t *types; uoffset_t count, base; - if (0 == (vte_type = read_vt_entry(td, id - 1))) { - if (0 == (vte_table = read_vt_entry(td, id))) { + if (0 == (vte_type = read_vt_entry(td, id - 1))) { // NOLINT(clang-analyzer-deadcode.DeadStores) + if (0 == (vte_table = read_vt_entry(td, id))) { // NOLINT(clang-analyzer-deadcode.DeadStores) verify(!required, flatcc_verify_error_type_field_absent_from_required_union_vector_field); } }