Repository navigation
macOS: Fix macOS 27 os_log compile errors, improve TRACE_VSYNC - #191238
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
github-actions
Bot
added
engine
Suppress `-Wignored-attributes` triggered when compiling macOS-specific logging and vsync tracing code. In the macOS 27 SDK that's part of Xcode 27, Apple appears to have added an `OS_LOG_UNINITIALIZED` macro inside their logging and signposting headers (`os/log.h`, `os/signpost.h`, `os/trace_base.h`). This macro expands to include: `__attribute__((stack_protector_ignore))`. Because of this, we skip stack protection checks on local formatting buffers generated by logging macros such as `os_log_with_type` and `os_signpost_event_emit`. However, in Flutter, we compile the engine with `-fstack-protector-all`, which causes the build to error out complaining the `stack_protector_ignore` attribute is being ignored: error: 'stack_protector_ignore' attribute ignored due to '-fstack-protector-all' option [-Werror,-Wignored-attributes] This wraps these usages in ignore pragmas to avoid disabling it across the board, which is the same as the approach that was taken in: * dart-lang/sdk@6bea1b7 * google/angle@918b93f This also creates the vsync log once and caches it statically as Apple recommends. `os_log` is already multi-tenant safe.
8d40c15 to
b3ea25a
Compare
cbracken
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the TRACE_VSYNC macro in FlutterVSyncWaiter.mm to declare the log variable as static, preventing repeated calls to os_log_create. Additionally, it wraps the os_signpost_event_emit call with compiler pragmas to suppress -Wignored-attributes diagnostics. There are no review comments, and I have no feedback to provide.
Suppress
-Wignored-attributestriggered when compiling macOS-specific logging and vsync tracing code.In the macOS 27 SDK that's part of Xcode 27, Apple appears to have added an
OS_LOG_UNINITIALIZEDmacro inside their logging and signposting headers (os/log.h,os/signpost.h,os/trace_base.h). This macro expands to include:__attribute__((stack_protector_ignore)).Because of this, we skip stack protection checks on local formatting buffers generated by logging macros such as
os_log_with_typeandos_signpost_event_emit. However, in Flutter, we compile the engine with-fstack-protector-all, which causes the build to error out complaining thestack_protector_ignoreattribute is being ignored:error: 'stack_protector_ignore' attribute ignored due to '-fstack-protector-all' option [-Werror,-Wignored-attributes]
This wraps these usages in ignore pragmas to avoid disabling it across the board, which is the same as the approach that was taken in:
This also creates the vsync log once and caches it statically as Apple recommends.
os_logis already multi-tenant safe.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.