Skip to content

Conversation

@oldnewthing
Copy link
Member

@oldnewthing oldnewthing commented Jan 26, 2021

At a minimum, avoid build breaks on non-MSVC compilers.

Turns out that gcc, clang, icc, djgpp, ellcc, and zapcc all define the symbol __GNUC__ and support the gcc return address extraction extensions. So we need only test two symbols: _MSC_VER for MSVC, and __GNUC__ for everyone else.

clang and icc implement the gcc extensions for compatibility. djgpp is built on top of gcc, so it inherits the gcc extensions. ellcc and zapcc are built on top of clang, so they inherit the gcc extensions via clang.

For MSVC, you must include intrin.h before using _ReturnAddress(), so we do that explicitly. rather than relying on the kindness of strangers (in this case, relying on apps having the WindowsNumerics.impl.h header file installed and leaving _XM_NO_INTRINSICS_ undefined, so that directxmath.h will include intrin.h for us).

I opted against making this a caller-overridable thing, to avoid odr problems. If somebody tries to onboard a new compiler toolchain, the custom_error_logger test will fail (null return address), and they can make a PR to this repo to add return address support for it.

At a minimum, avoid build breaks on non-MSVC compilers.

Turns out that gcc, clang, icc, djgpp, ellcc, and zapcc
all define the symbol `__GNUC__` and support the gcc return address
extraction extensions. So we need only test two symbols:
`_MSC_VER` for MSVC, and `__GNUC__` for everyone else.

(clang and icc implement the gcc extensions for compatibility.
djgpp is built on top of gcc, so it inherits the gcc extensions.
ellcc and zapcc are built on top of clang, so they inherit the
gcc extensions via clang.)
Copy link
Collaborator

@kennykerr kennykerr left a comment

Choose a reason for hiding this comment

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

Thanks!

@kennykerr kennykerr merged commit fbf0543 into microsoft:master Jan 27, 2021
@oldnewthing oldnewthing deleted the returnaddress branch February 16, 2021 15:31
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.

2 participants