[PATCH] D71772: [scudo][standalone] Support __BIONIC__

Peter Collingbourne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 20 10:19:49 PST 2019


pcc added inline comments.


================
Comment at: compiler-rt/lib/scudo/standalone/platform.h:18
 
-#if defined(__ANDROID__)
+#if defined(__ANDROID__) || defined(__BIONIC__)
 #define SCUDO_ANDROID 1
----------------
cryptoad wrote:
> cferris wrote:
> > pcc wrote:
> > > cryptoad wrote:
> > > > pcc wrote:
> > > > > Maybe just `#if defined(__BIONIC__)`?
> > > > We still want Scudo to work on Android outside of Bionic (eg: .so or just statically linked to a binary) in which case I don't think __BIONIC__ will be defined.
> > > I don't think that `__BIONIC__` means that we're compiling as part of Bionic, it just means that the libc is Bionic. See:
> > > https://android.googlesource.com/platform/bionic/+/master/docs/defines.md
> > > 
> > > To confuse matters, we also have `_BIONIC`, which, at least within scudo, does mean that we're compiling as part of Bionic. It may be a good idea to rename this macro to something more clear.
> > It's also the case that __BIONIC__ is only defined when you include cdefs.h directly or indirectly. Unfortunately, at this point it's not defined so this doesn't quite solve the problem.
> Right now there is no `#include` in platform.h which I feel is what it should be.
> Should I include cdefs.h and do a `defined(__BIONIC__)` or work around it?
I'm not sure that you can include cdefs.h because it isn't universally available, e.g. the Windows and Fuchsia SDKs don't appear to have it.

One option would be to include a standardized header such as stdint.h which does include cdefs.h on Android.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71772/new/

https://reviews.llvm.org/D71772





More information about the llvm-commits mailing list