Skip to content

Disable sigaltstack on Apple platforms#18251

Merged
ggreenway merged 2 commits intoenvoyproxy:mainfrom
keith:ks/disable-sigaltstack-on-apple-platforms
Sep 27, 2021
Merged

Disable sigaltstack on Apple platforms#18251
ggreenway merged 2 commits intoenvoyproxy:mainfrom
keith:ks/disable-sigaltstack-on-apple-platforms

Conversation

@keith
Copy link
Member

@keith keith commented Sep 24, 2021

According to https://reviews.llvm.org/D28265 using sigaltstack with
backtrace() (which absiel uses internally to get stack trace info) is
not compatible. Using them together results in crashes in tests not
printing backtraces only on macOS

envoyproxy/envoy-mobile#1827

@repokitteh-read-only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #18251 was opened by keith.

see: more, trace.

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Awesome thanks for fixing. cc @rojkov who is probably our best bet for a reviewer who understands all of this. :)

According to https://reviews.llvm.org/D28265 using `sigaltstack` with
`backtrace()` (which absiel uses internally to get stack trace info) is
not compatible. This results in crashes in tests not printing backtraces
only on macOS

envoyproxy/envoy-mobile#1827
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith keith force-pushed the ks/disable-sigaltstack-on-apple-platforms branch from 39ab2b4 to 2505144 Compare September 24, 2021 16:25
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith keith marked this pull request as ready for review September 24, 2021 16:44
@keith
Copy link
Member Author

keith commented Sep 24, 2021

a 404 from brew 🤔

@keith
Copy link
Member Author

keith commented Sep 24, 2021

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #18251 (comment) was created by @keith.

see: more, trace.

Copy link
Member

@rojkov rojkov left a comment

Choose a reason for hiding this comment

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

Thanks for fixing! The consequence is that stack overflows become difficult to debug on Apple, but given sigaltstack has been already non-functional on the platform it's better to go this way and to get meaningful backtraces for other cases at least.

@ggreenway ggreenway merged commit d77ac0a into envoyproxy:main Sep 27, 2021
junr03 added a commit that referenced this pull request Sep 28, 2021
Commit Message: signal action - fully disable sigaltstack on Apple
Additional Description: follow up from #18251
Risk Level: low - follow up on previous PR
Platform Specific Features: only affects apple platforms

Signed-off-by: Jose Nino <jnino@lyft.com>
@keith keith deleted the ks/disable-sigaltstack-on-apple-platforms branch September 29, 2021 19:13
@keith
Copy link
Member Author

keith commented Sep 29, 2021

Thanks!

soulxu pushed a commit to soulxu/envoy that referenced this pull request Oct 16, 2021
According to https://reviews.llvm.org/D28265 using `sigaltstack` with
`backtrace()` (which absiel uses internally to get stack trace info) is
not compatible. This results in crashes in tests not printing backtraces
only on macOS

envoyproxy/envoy-mobile#1827
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants