Skip to content

Fix size test build on gcc11#17867

Closed
lucylq wants to merge 1 commit intomainfrom
binary-size
Closed

Fix size test build on gcc11#17867
lucylq wants to merge 1 commit intomainfrom
binary-size

Conversation

@lucylq
Copy link
Contributor

@lucylq lucylq commented Mar 5, 2026

Summary

size_test on gcc11 fails with

In static member function ‘static constexpr std::size_t std::char_traits<char>::length(const char_type*)’,
    inlined from ‘constexpr std::basic_string_view<_CharT, _Traits>::basic_string_view(const _CharT*) [with _CharT = char; _Traits = std::char_traits<char>]’ at /usr/include/c++/11/string_view:132:35,
    inlined from ‘executorch::runtime::Result<void*> executorch::runtime::deserialization::getTensorDataPtr(const executorch_flatbuffer::Tensor*, const executorch::runtime::Program*, size_t, executorch::runtime::HierarchicalAllocator*, const executorch::runtime::NamedDataMap*, executorch::runtime::Span<executorch::runtime::deserialization::NamedData>)’ at /data/users/lfq/binary-size/executorch/runtime/executor/tensor_parser_exec_aten.cpp:218:41:
/usr/include/c++/11/bits/char_traits.h:399:32: error: ‘long unsigned int __builtin_strlen(const char*)’ reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread]
  399 |         return __builtin_strlen(__s);
      |                ~~~~~~~~~~~~~~~~^~~~~
In file included from /data/users/lfq/binary-size/executorch/../executorch/runtime/executor/tensor_parser.h:21,
                 from /data/users/lfq/binary-size/executorch/runtime/executor/tensor_parser_exec_aten.cpp:9:
/data/users/lfq/binary-size/executorch/cmake-out/schema/include/executorch/schema/program_generated.h: In function ‘executorch::runtime::Result<void*> executorch::runtime::deserialization::getTensorDataPtr(const executorch_flatbuffer::Tensor*, const executorch::runtime::Program*, size_t, executorch::runtime::HierarchicalAllocator*, const executorch::runtime::NamedDataMap*, executorch::runtime::Span<executorch::runtime::deserialization::NamedData>)’:
/data/users/lfq/binary-size/executorch/cmake-out/schema/include/executorch/schema/program_generated.h:568:8: note: at offset 5 into source object ‘executorch_flatbuffer::ExtraTensorInfo::<anonymous>’ of size 1
  568 | struct ExtraTensorInfo FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table {

Test plan

Flatbuffer should be safe - add -Wno-stringop-overread to suppress the error.

size results after stripping

sh test/build_size_test.sh 

(executorch) [lfq@devvm11764.nha0 /data/users/lfq/binary-size/executorch (binary-size)]$ ls -la cmake-out/test/size_test_all_ops
-rwxr-xr-x 1 lfq users 1670416 Mar  4 22:37 cmake-out/test/size_test_all_ops

(executorch) [lfq@devvm11764.nha0 /data/users/lfq/binary-size/executorch (binary-size)]$ ls -la cmake-out/test/size_test
-rwxr-xr-x 1 lfq users 56240 Mar  4 22:38 cmake-out/test/size_test

@pytorch-bot
Copy link

pytorch-bot bot commented Mar 5, 2026

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/17867

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 Awaiting Approval, 1 New Failure, 3 Unrelated Failures

As of commit 31c6aa8 with merge base 75f5a76 (image):

AWAITING APPROVAL - The following workflow needs approval before CI can run:

NEW FAILURE - The following job has failed:

FLAKY - The following job failed but was likely due to flakiness present on trunk:

BROKEN TRUNK - The following jobs failed but was present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 5, 2026
@github-actions
Copy link

github-actions bot commented Mar 5, 2026

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@lucylq lucylq marked this pull request as ready for review March 5, 2026 06:38
Copilot AI review requested due to automatic review settings March 5, 2026 06:38
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a build failure in the size test when compiled with GCC 11. GCC 11 produces a false positive -Wstringop-overread warning in Flatbuffers-generated code (program_generated.h), which is promoted to an error by -Werror. The fix suppresses this specific warning via -Wno-stringop-overread.

Changes:

  • Added -Wno-stringop-overread to COMMON_CXXFLAGS in the size test build script to suppress a false positive GCC 11 warning in Flatbuffers-generated code.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -15,7 +15,7 @@ EXTRA_BUILD_ARGS="${@:-}"
# TODO(#8357): Remove -Wno-int-in-bool-context
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing -Wno-int-in-bool-context flag has a TODO comment (line 15) explaining why it's there and that it should eventually be removed. Consider adding a similar TODO comment for -Wno-stringop-overread to document that it's a workaround for a GCC 11 false positive in Flatbuffers-generated code, and can potentially be removed when GCC 11 is no longer a supported compiler.

Suggested change
# TODO(#8357): Remove -Wno-int-in-bool-context
# TODO(#8357): Remove -Wno-int-in-bool-context
# TODO: Remove -Wno-stringop-overread once GCC 11 is no longer supported.
# This flag works around a GCC 11 false positive in Flatbuffers-generated code.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@@ -15,7 +15,7 @@ EXTRA_BUILD_ARGS="${@:-}"
# TODO(#8357): Remove -Wno-int-in-bool-context
# TODO: Replace -ET_HAVE_PREAD=0 with a CMake option.
# FileDataLoader used in the size_test breaks baremetal builds with pread when missing.
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a TODO comment for -Wno-stringop-overread, similar to the existing # TODO(#8357): Remove -Wno-int-in-bool-context on line 15, to track when this suppression can be removed (e.g., when the FlatBuffers version is updated or the GCC11 false positive is resolved). This helps prevent warning suppressions from accumulating without tracking.

Suggested change
# FileDataLoader used in the size_test breaks baremetal builds with pread when missing.
# FileDataLoader used in the size_test breaks baremetal builds with pread when missing.
# TODO: Remove -Wno-stringop-overread once the GCC11 FlatBuffers false positive is resolved or FlatBuffers is updated.

Copilot uses AI. Check for mistakes.
@lucylq
Copy link
Contributor Author

lucylq commented Mar 6, 2026

duplicate of #17905, sorry @kirklandsign 😅

@lucylq lucylq closed this Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants