From 946f2dead352a3708e70391701556a5f0437eeb7 Mon Sep 17 00:00:00 2001 From: Connor O'Brien Date: Mon, 22 Nov 2021 18:02:36 -0800 Subject: DO NOT MERGE Rename libbpf to libbpf_bcc This is to prevent a name collision with "upstream" libbpf (external/libbpf) which is built using the same name. Bug: 203823368 Test: build cuttlefish Signed-off-by: Connor O'Brien Change-Id: I8fa2b21d331ecdeabd0e0a1a9f495f29ff967623 --- libs/cputimeinstate/Android.bp | 4 ++-- services/gpuservice/gpumem/Android.bp | 2 +- services/gpuservice/tests/unittests/Android.bp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libs/cputimeinstate/Android.bp b/libs/cputimeinstate/Android.bp index 570af71d9a..1fd2c62772 100644 --- a/libs/cputimeinstate/Android.bp +++ b/libs/cputimeinstate/Android.bp @@ -12,7 +12,7 @@ cc_library { srcs: ["cputimeinstate.cpp"], shared_libs: [ "libbase", - "libbpf", + "libbpf_bcc", "libbpf_android", "liblog", "libnetdutils" @@ -31,7 +31,7 @@ cc_test { srcs: ["testtimeinstate.cpp"], shared_libs: [ "libbase", - "libbpf", + "libbpf_bcc", "libbpf_android", "libtimeinstate", "libnetdutils", diff --git a/services/gpuservice/gpumem/Android.bp b/services/gpuservice/gpumem/Android.bp index 830e53d534..24087ac9e6 100644 --- a/services/gpuservice/gpumem/Android.bp +++ b/services/gpuservice/gpumem/Android.bp @@ -28,7 +28,7 @@ cc_library_shared { ], shared_libs: [ "libbase", - "libbpf", + "libbpf_bcc", "libbpf_android", "libcutils", "liblog", diff --git a/services/gpuservice/tests/unittests/Android.bp b/services/gpuservice/tests/unittests/Android.bp index 6d87c45921..5b69f960a2 100644 --- a/services/gpuservice/tests/unittests/Android.bp +++ b/services/gpuservice/tests/unittests/Android.bp @@ -34,7 +34,7 @@ cc_test { ], shared_libs: [ "libbase", - "libbpf", + "libbpf_bcc", "libbpf_android", "libcutils", "libgfxstats", -- cgit v1.2.3 From ec7e4d9a5cc4dfc2937bb80d51cdb85193f5d72f Mon Sep 17 00:00:00 2001 From: Garfield Tan Date: Mon, 19 Jul 2021 12:02:16 -0700 Subject: Skip creating external textures that exceeds size limit Skia always accept an Android buffer and wraps it around with a texture even if its size exceeds the limit GL exposes. Therefore let's skip creating the texture in SurfaceFlinger and outputs an error log to logcat. I chose to do it in SurfaceFlinger rather than RenderEngine is because the external texture mapping is designed to be asynchronous, so it'd be better to keep that way. The limit is also exposed out of RenderEngine so SurfaceFlinger can check it before creating external textures as well. Bug: 190399306 Bug: 204316511 Test: The test mentioned in the bug fails instead of crashing SurfaceFlinger. Test: atest SurfaceFlinger_test Test: atest libsurfaceflinger_unittest Change-Id: I52d253ed5a10f0e4ade372048721913405ed668a (cherry picked from commit 9c9c19134593655c36fe70aaa45a91ad4f75e36f) Merged-In: I52d253ed5a10f0e4ade372048721913405ed668a --- services/surfaceflinger/BufferQueueLayer.cpp | 7 ++-- services/surfaceflinger/SurfaceFlinger.cpp | 42 ++++++++++++++-------- services/surfaceflinger/SurfaceFlinger.h | 8 +++-- .../surfaceflinger/tests/ScreenCapture_test.cpp | 18 +++++++++- .../tests/unittests/CompositionTest.cpp | 4 +-- .../tests/unittests/TestableSurfaceFlinger.h | 1 + 6 files changed, 56 insertions(+), 24 deletions(-) diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp index 6b6d43425d..99e470dfe6 100644 --- a/services/surfaceflinger/BufferQueueLayer.cpp +++ b/services/surfaceflinger/BufferQueueLayer.cpp @@ -515,13 +515,10 @@ void BufferQueueLayer::onFirstRef() { } status_t BufferQueueLayer::setDefaultBufferProperties(uint32_t w, uint32_t h, PixelFormat format) { - uint32_t const maxSurfaceDims = - std::min(mFlinger->getMaxTextureSize(), mFlinger->getMaxViewportDims()); - // never allow a surface larger than what our underlying GL implementation // can handle. - if ((uint32_t(w) > maxSurfaceDims) || (uint32_t(h) > maxSurfaceDims)) { - ALOGE("dimensions too large %u x %u", uint32_t(w), uint32_t(h)); + if (mFlinger->exceedsMaxRenderTargetSize(w, h)) { + ALOGE("dimensions too large %" PRIu32 " x %" PRIu32, w, h); return BAD_VALUE; } diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 230810c936..5ab5032276 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -792,6 +792,8 @@ void SurfaceFlinger::init() { ? renderengine::RenderEngine::ContextPriority::REALTIME : renderengine::RenderEngine::ContextPriority::MEDIUM) .build())); + mMaxRenderTargetSize = + std::min(getRenderEngine().getMaxTextureSize(), getRenderEngine().getMaxViewportDims()); // Set SF main policy after initializing RenderEngine which has its own policy. if (!SetTaskProfiles(0, {"SFMainPolicy"})) { @@ -879,14 +881,6 @@ void SurfaceFlinger::startBootAnim() { } } -size_t SurfaceFlinger::getMaxTextureSize() const { - return getRenderEngine().getMaxTextureSize(); -} - -size_t SurfaceFlinger::getMaxViewportDims() const { - return getRenderEngine().getMaxViewportDims(); -} - // ---------------------------------------------------------------------------- bool SurfaceFlinger::authenticateSurfaceTexture( @@ -4218,17 +4212,30 @@ uint32_t SurfaceFlinger::setClientStateLocked( } bool bufferChanged = what & layer_state_t::eBufferChanged; bool cacheIdChanged = what & layer_state_t::eCachedBufferChanged; + bool bufferSizeExceedsLimit = false; std::shared_ptr buffer; if (bufferChanged && cacheIdChanged && s.buffer != nullptr) { - ClientCache::getInstance().add(s.cachedBuffer, s.buffer); - buffer = ClientCache::getInstance().get(s.cachedBuffer); + bufferSizeExceedsLimit = + exceedsMaxRenderTargetSize(s.buffer->getWidth(), s.buffer->getHeight()); + if (!bufferSizeExceedsLimit) { + ClientCache::getInstance().add(s.cachedBuffer, s.buffer); + buffer = ClientCache::getInstance().get(s.cachedBuffer); + } } else if (cacheIdChanged) { buffer = ClientCache::getInstance().get(s.cachedBuffer); } else if (bufferChanged && s.buffer != nullptr) { - buffer = std::make_shared< - renderengine::ExternalTexture>(s.buffer, getRenderEngine(), - renderengine::ExternalTexture::Usage::READABLE); - } + bufferSizeExceedsLimit = + exceedsMaxRenderTargetSize(s.buffer->getWidth(), s.buffer->getHeight()); + if (!bufferSizeExceedsLimit) { + buffer = std::make_shared< + renderengine::ExternalTexture>(s.buffer, getRenderEngine(), + renderengine::ExternalTexture::Usage::READABLE); + } + } + ALOGE_IF(bufferSizeExceedsLimit, + "Attempted to create an ExternalTexture for layer %s that exceeds render target size " + "limit.", + layer->getDebugName()); if (buffer) { const bool frameNumberChanged = what & layer_state_t::eFrameNumberChanged; const uint64_t frameNumber = frameNumberChanged @@ -6198,6 +6205,13 @@ status_t SurfaceFlinger::captureScreenCommon(RenderAreaFuture renderAreaFuture, const sp& captureListener) { ATRACE_CALL(); + if (exceedsMaxRenderTargetSize(bufferSize.getWidth(), bufferSize.getHeight())) { + ALOGE("Attempted to capture screen with size (%" PRId32 ", %" PRId32 + ") that exceeds render target size limit.", + bufferSize.getWidth(), bufferSize.getHeight()); + return BAD_VALUE; + } + // Loop over all visible layers to see whether there's any protected layer. A protected layer is // typically a layer with DRM contents, or have the GRALLOC_USAGE_PROTECTED set on the buffer. // A protected layer has no implication on whether it's secure, which is explicitly set by diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 380f444221..95c251c721 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -931,8 +931,9 @@ private: void readPersistentProperties(); - size_t getMaxTextureSize() const; - size_t getMaxViewportDims() const; + bool exceedsMaxRenderTargetSize(uint32_t width, uint32_t height) const { + return width > mMaxRenderTargetSize || height > mMaxRenderTargetSize; + } int getMaxAcquiredBufferCountForCurrentRefreshRate(uid_t uid) const; @@ -1381,6 +1382,9 @@ private: SurfaceFlingerBE mBE; std::unique_ptr mCompositionEngine; + // mMaxRenderTargetSize is only set once in init() so it doesn't need to be protected by + // any mutex. + size_t mMaxRenderTargetSize{1}; const std::string mHwcServiceName; diff --git a/services/surfaceflinger/tests/ScreenCapture_test.cpp b/services/surfaceflinger/tests/ScreenCapture_test.cpp index 6912fcf219..ab2064efd0 100644 --- a/services/surfaceflinger/tests/ScreenCapture_test.cpp +++ b/services/surfaceflinger/tests/ScreenCapture_test.cpp @@ -515,7 +515,8 @@ TEST_F(ScreenCaptureTest, CaptureSize) { } TEST_F(ScreenCaptureTest, CaptureInvalidLayer) { - sp redLayer = createLayer(String8("Red surface"), 60, 60, 0); + sp redLayer = createLayer(String8("Red surface"), 60, 60, + ISurfaceComposerClient::eFXSurfaceBufferState); ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(redLayer, Color::RED, 60, 60)); @@ -532,6 +533,21 @@ TEST_F(ScreenCaptureTest, CaptureInvalidLayer) { ASSERT_EQ(NAME_NOT_FOUND, ScreenCapture::captureLayers(args, captureResults)); } +TEST_F(ScreenCaptureTest, CaptureTooLargeLayer) { + sp redLayer = createLayer(String8("Red surface"), 60, 60); + ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(redLayer, Color::RED, 60, 60)); + + Transaction().show(redLayer).setLayer(redLayer, INT32_MAX).apply(true); + + LayerCaptureArgs captureArgs; + captureArgs.layerHandle = redLayer->getHandle(); + captureArgs.frameScaleX = INT32_MAX / 60; + captureArgs.frameScaleY = INT32_MAX / 60; + + ScreenCaptureResults captureResults; + ASSERT_EQ(BAD_VALUE, ScreenCapture::captureLayers(captureArgs, captureResults)); +} + TEST_F(ScreenCaptureTest, CaptureSecureLayer) { sp redLayer = createLayer(String8("Red surface"), 60, 60, ISurfaceComposerClient::eFXSurfaceBufferState); diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp index 560f139719..52a36a2719 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -108,6 +108,8 @@ public: mComposer = new Hwc2::mock::Composer(); mFlinger.setupComposer(std::unique_ptr(mComposer)); + + mFlinger.mutableMaxRenderTargetSize() = 16384; } ~CompositionTest() { @@ -519,8 +521,6 @@ struct BaseLayerProperties { static void setupLatchedBuffer(CompositionTest* test, sp layer) { // TODO: Eliminate the complexity of actually creating a buffer - EXPECT_CALL(*test->mRenderEngine, getMaxTextureSize()).WillOnce(Return(16384)); - EXPECT_CALL(*test->mRenderEngine, getMaxViewportDims()).WillOnce(Return(16384)); status_t err = layer->setDefaultBufferProperties(LayerProperties::WIDTH, LayerProperties::HEIGHT, LayerProperties::FORMAT); diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index cf67593174..3802e0d617 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -435,6 +435,7 @@ public: auto& mutableTransactionFlags() { return mFlinger->mTransactionFlags; } auto& mutablePowerAdvisor() { return mFlinger->mPowerAdvisor; } auto& mutableDebugDisableHWC() { return mFlinger->mDebugDisableHWC; } + auto& mutableMaxRenderTargetSize() { return mFlinger->mMaxRenderTargetSize; } auto& mutableHwcDisplayData() { return getHwComposer().mDisplayData; } auto& mutableHwcPhysicalDisplayIdMap() { return getHwComposer().mPhysicalDisplayIdMap; } -- cgit v1.2.3 From 5c4dd9ffb3338a578a17e5772c649cc4bc4667ac Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Thu, 23 Dec 2021 23:26:48 +0000 Subject: Obviate libbinder_ndk_host_user. After this, it is no longer required in order to use libbinder_ndk on host. Following this change, all users will be cleaned up and the module itself will be removed. Bug: 211908498 Test: build Change-Id: I7136472677ada11cc2c0b726cd849cac98d3ee4e Merged-In: I7136472677ada11cc2c0b726cd849cac98d3ee4e (cherry picked from commit 619a02a6c35ddf13754e5bab764ee8eb76b46fd9) --- libs/binder/ndk/Android.bp | 10 +--------- libs/binder/ndk/include_ndk/android/binder_status.h | 19 +++++++++++++++++-- libs/binder/rust/Android.bp | 8 -------- libs/binder/tests/parcel_fuzzer/Android.bp | 6 ++++-- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/libs/binder/ndk/Android.bp b/libs/binder/ndk/Android.bp index b03e24cd1a..23162d2e91 100644 --- a/libs/binder/ndk/Android.bp +++ b/libs/binder/ndk/Android.bp @@ -32,17 +32,10 @@ license { ], } +// TODO(b/211908498): remove this cc_defaults { name: "libbinder_ndk_host_user", target: { - host: { - cflags: [ - "-D__INTRODUCED_IN(n)=", - "-D__assert(a,b,c)=", - // We want all the APIs to be available on the host. - "-D__ANDROID_API__=10000", - ], - }, darwin: { enabled: false, }, @@ -52,7 +45,6 @@ cc_defaults { cc_library { name: "libbinder_ndk", - defaults: ["libbinder_ndk_host_user"], host_supported: true, llndk: { diff --git a/libs/binder/ndk/include_ndk/android/binder_status.h b/libs/binder/ndk/include_ndk/android/binder_status.h index 6f1fdfcd20..76c7aacb7c 100644 --- a/libs/binder/ndk/include_ndk/android/binder_status.h +++ b/libs/binder/ndk/include_ndk/android/binder_status.h @@ -32,11 +32,26 @@ __BEGIN_DECLS +#ifndef __BIONIC__ + +#ifndef __INTRODUCED_IN +#define __INTRODUCED_IN(n) +#endif + +#ifndef __assert +#define __assert(a, b, c) \ + do { \ + syslog(LOG_ERR, a ": " c); \ + abort(); \ + } while (false) +#endif + #ifndef __ANDROID_API__ -#error Android builds must be compiled against a specific API. If this is an \ - android platform host build, you must use libbinder_ndk_host_user. +#define __ANDROID_API__ 10000 #endif +#endif // __BIONIC__ + /** * Low-level status types for use in binder. This is the least preferable way to * return an error for binder services (where binder_exception_t should be used, diff --git a/libs/binder/rust/Android.bp b/libs/binder/rust/Android.bp index 49d3401a4f..416616031b 100644 --- a/libs/binder/rust/Android.bp +++ b/libs/binder/rust/Android.bp @@ -85,14 +85,6 @@ rust_bindgen { // Currently necessary for host builds // TODO(b/31559095): bionic on host should define this target: { - host: { - cflags: [ - "-D__INTRODUCED_IN(n)=", - "-D__assert(a,b,c)=", - // We want all the APIs to be available on the host. - "-D__ANDROID_API__=10000", - ], - }, darwin: { enabled: false, }, diff --git a/libs/binder/tests/parcel_fuzzer/Android.bp b/libs/binder/tests/parcel_fuzzer/Android.bp index 74b8eb8d93..4b1bd2f591 100644 --- a/libs/binder/tests/parcel_fuzzer/Android.bp +++ b/libs/binder/tests/parcel_fuzzer/Android.bp @@ -9,7 +9,6 @@ package { cc_fuzz { name: "binder_parcel_fuzzer", - defaults: ["libbinder_ndk_host_user"], host_supported: true, fuzz_config: { @@ -51,6 +50,9 @@ cc_fuzz { "libbinder", ], }, + darwin: { + enabled: false, + }, }, // This flag enables verbose output in the fuzz target, and is very useful // for debugging a failure. If you are trying to diagnose how a crash was @@ -64,7 +66,7 @@ cc_library_static { target: { darwin: { enabled: false, - } + }, }, srcs: [ "random_fd.cpp", -- cgit v1.2.3