Make sure size_test CI uses -Wall -Werror

Author: mergennachin

CONTRIBUTING.md

@@ -215,6 +215,11 @@ must work with threading**
 
 ## Testing
 
+### Running Tests
+
+The `sh test/build_size_test.sh` script will compile the runtime along with portable kernels.
+The `test/run_oss_cpp_tests.sh` script will build and run C++ tests locally.
+
 ### Writing Tests
 To help keep code quality high, ExecuTorch uses a combination of unit tests and
 end-to-end (e2e) tests. If you add a new feature or fix a bug, please add tests
@@ -229,8 +234,6 @@ If it's not clear how to add a test for your PR, take a look at the blame for
 the code you're modifying and find an author who has more context. Ask them
 for their help in the PR comments.
 
-The `test/run_oss_cpp_tests.sh` script will build and run C++ tests locally.
-
 ### Continuous Integration
 See https://hud.pytorch.org/hud/pytorch/executorch/main for the current state of
 the CI (continuous integration) jobs. If `main` is broken, consider rebasing

kernels/portable/cpu/op_full.cpp

@@ -38,7 +38,8 @@ Tensor& full_out(
 
   ET_SWITCH_SCALAR_OBJ_TYPES(val_type, ctx, name, CTYPE_VAL, [&] {
     CTYPE_VAL val;
-    utils::extract_scalar(fill_value, &val);
+    ET_KERNEL_CHECK(
+        ctx, utils::extract_scalar(fill_value, &val), InvalidArgument, );
 
     ET_SWITCH_REALHBBF16_TYPES(out_type, ctx, name, CTYPE_OUT, [&] {
       CTYPE_OUT val_casted = static_cast<CTYPE_OUT>(val);

kernels/portable/cpu/op_full_like.cpp

@@ -54,7 +54,8 @@ Tensor& full_like_out(
 
   ET_SWITCH_REALB_TYPES(val_type, ctx, name, CTYPE_VAL, [&] {
     CTYPE_VAL val;
-    utils::extract_scalar(fill_value, &val);
+    ET_KERNEL_CHECK(
+        ctx, utils::extract_scalar(fill_value, &val), InvalidArgument, );
 
     ET_SWITCH_REALHBBF16_TYPES(out_type, ctx, name, CTYPE_OUT, [&] {
       CTYPE_OUT val_casted = static_cast<CTYPE_OUT>(val);

kernels/portable/cpu/op_scatter.cpp

@@ -156,7 +156,7 @@ Tensor& scatter_value_out(
 
   ET_SWITCH_SCALAR_OBJ_TYPES(val_type, ctx, name, CTYPE_VAL, [&] {
     CTYPE_VAL val;
-    utils::extract_scalar(value, &val);
+    ET_KERNEL_CHECK(ctx, utils::extract_scalar(value, &val), InvalidArgument, );
 
     ET_SWITCH_REALHBBF16_TYPES(in.scalar_type(), ctx, name, CTYPE, [&]() {
       scatter_value_helper<CTYPE>(in, dim, index, val, out);

kernels/portable/cpu/util/broadcast_util.cpp

@@ -215,18 +215,16 @@ ET_NODISCARD Error get_broadcast_target_size(
     size_t* out_dim) {
   if ET_UNLIKELY (!tensors_are_broadcastable_between(a_size, b_size)) {
 #ifdef ET_LOG_ENABLED
-    const auto a_shape_str = tensor_shape_to_c_string(
-        executorch::runtime::Span<const Tensor::SizesType>(
-            a_size.data(), a_size.size()));
-    const auto b_shape_str = tensor_shape_to_c_string(
-        executorch::runtime::Span<const Tensor::SizesType>(
-            b_size.data(), b_size.size()));
-#endif
+    executorch::runtime::Span<const Tensor::SizesType> a_size_span(
+        a_size.data(), a_size.size());
+    executorch::runtime::Span<const Tensor::SizesType> b_size_span(
+        b_size.data(), b_size.size());
     ET_LOG(
         Error,
         "Two input tensors should be broadcastable but got shapes %s and %s.",
-        a_shape_str.data(),
-        b_shape_str.data());
+        tensor_shape_to_c_string(a_size_span).data(),
+        tensor_shape_to_c_string(b_size_span).data());
+#endif
     return executorch::runtime::Error::InvalidArgument;
   }
 

runtime/core/portable_type/tensor_impl.cpp

@@ -96,20 +96,18 @@ Error TensorImpl::internal_resize_contiguous(ArrayRef<SizesType> new_sizes) {
     case TensorShapeDynamism::STATIC:
       if (!std::equal(sizes_, sizes_ + dim_, new_sizes.begin())) {
 #ifdef ET_LOG_ENABLED
-        auto old_sizes_str = executorch::runtime::tensor_shape_to_c_string(
-            executorch::runtime::Span<const SizesType>(
-                sizes().data(), sizes().size()));
-        auto new_sizes_str = executorch::runtime::tensor_shape_to_c_string(
-            executorch::runtime::Span<const SizesType>(
-                new_sizes.data(), new_sizes.size()));
-#endif
-
-        ET_CHECK_OR_RETURN_ERROR(
-            false,
-            NotSupported,
+        executorch::runtime::Span<const SizesType> sizes_span(
+            sizes().data(), sizes().size());
+        executorch::runtime::Span<const SizesType> new_sizes_span(
+            new_sizes.data(), new_sizes.size());
+        ET_LOG(
+            Error,
             "Attempted to resize a static tensor. Expected shape %s, but received %s.",
-            old_sizes_str.data(),
-            new_sizes_str.data())
+            executorch::runtime::tensor_shape_to_c_string(sizes_span).data(),
+            executorch::runtime::tensor_shape_to_c_string(new_sizes_span)
+                .data());
+#endif
+        return ::executorch::runtime::Error::NotSupported;
       }
 
       break;

runtime/kernel/operator_registry.cpp

@@ -135,7 +135,7 @@ void make_kernel_key_string(Span<const TensorMeta> key, char* buf) {
     // If no tensor is present in an op, kernel key does not apply
     return;
   }
-  strncpy(buf, "v1/", 3);
+  strcpy(buf, "v1/");
   buf += 3;
   for (size_t i = 0; i < key.size(); i++) {
     auto& meta = key[i];

test/build_size_test.sh

@@ -11,11 +11,14 @@ set -e
 # shellcheck source=/dev/null
 source "$(dirname "${BASH_SOURCE[0]}")/../.ci/scripts/utils.sh"
 
+# TODO(https://github.com/pytorch/executorch/issues/814): Remove -Wno-sign-compare
+COMMON_CXXFLAGS="-fno-exceptions -fno-rtti -Wall -Werror -Wno-sign-compare -Wno-unknown-pragmas"
+
 cmake_install_executorch_lib() {
   echo "Installing libexecutorch.a"
   clean_executorch_install_folders
-
-  CXXFLAGS="-fno-exceptions -fno-rtti" retry cmake -DBUCK2="$BUCK2" \
+  
+  CXXFLAGS="$COMMON_CXXFLAGS" retry cmake -DBUCK2="$BUCK2" \
           -DCMAKE_CXX_STANDARD_REQUIRED=ON \
           -DCMAKE_INSTALL_PREFIX=cmake-out \
           -DCMAKE_BUILD_TYPE=Release \
@@ -27,7 +30,7 @@ cmake_install_executorch_lib() {
 }
 
 test_cmake_size_test() {
-    CXXFLAGS="-fno-exceptions -fno-rtti" retry cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=cmake-out -Bcmake-out/test test
+    CXXFLAGS="$COMMON_CXXFLAGS" retry cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=cmake-out -Bcmake-out/test test
 
     echo "Build size test"
     cmake --build cmake-out/test -j9 --config Release