Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NDK_ANALYZE=1 is broken #362

Closed
andreya108 opened this issue Apr 11, 2017 · 7 comments
Closed

NDK_ANALYZE=1 is broken #362

andreya108 opened this issue Apr 11, 2017 · 7 comments
Assignees
Milestone

Comments

@andreya108
Copy link

andreya108 commented Apr 11, 2017

Description

NDK_ANALYZE feature is completely broken in ndk r14

It uses clang static code analyzer tool and scan-build utility.
But all invocation scripts are out of date.

  1. android-ndk-r14b/build/ndk-build does not even try to setup C++ analyzer, it always assume C and gcc as toolchain
  2. clang toolchain's setup.mk overrides TARGET_CXX, so there are no chances for scan-build to get any results
    ...

Environment Details

  • NDK Version: r14b
  • Build sytem: ndk-build
  • Host OS: Ubuntu 16.04
  • Compiler: clang
  • ABI: arm64-v8a
@andreya108
Copy link
Author

andreya108 commented Apr 11, 2017

I've made patches fixing NDK_ANALYZE:

Every clang toolchain's setup.mk should be patched like this:

--- a/build/core/toolchains/aarch64-linux-android-clang/setup.mk	2017-04-11 17:56:17.813103033 +0300
+++ b/build/core/toolchains/aarch64-linux-android-clang/setup.mk	2017-04-11 17:58:09.925105404 +0300
@@ -36,7 +36,11 @@
 TOOLCHAIN_PREFIX := $(TOOLCHAIN_ROOT)/bin/$(TOOLCHAIN_NAME)-

 TARGET_CC := $(LLVM_TOOLCHAIN_PREFIX)clang$(HOST_EXEEXT)
+ifneq ($(NDK_ANALYZE),1)
 TARGET_CXX := $(LLVM_TOOLCHAIN_PREFIX)clang++$(HOST_EXEEXT)
+else
+TARGET_CXX := $(CXX)
+endif

 LLVM_TRIPLE := aarch64-none-linux-android

And ndk-build script should be patched like this:

--- a/build/ndk-build	2017-04-11 18:06:34.000000000 +0300
+++ b/build/ndk-build	2017-04-11 18:06:24.005115849 +0300
@@ -264,10 +264,11 @@
         TOOLCHAIN_PREFIX=`get_build_var_for_abi TOOLCHAIN_PREFIX $ABI`
         LLVM_PATH=$ANDROID_NDK_ROOT/toolchains/llvm/prebuilt/$HOST_TAG
         perl $LLVM_PATH/tools/scan-build/bin/scan-build \
-            --use-analyzer $LLVM_PATH/bin/${ABI}/analyzer \
-            --use-cc ${TOOLCHAIN_PREFIX}gcc \
-            --use-c++ ${TOOLCHAIN_PREFIX}g++ \
+            --use-analyzer $LLVM_PATH/bin/${ABI}/analyzer++ \
+            --use-cc $LLVM_PATH/bin/clang \
+            --use-c++ $LLVM_PATH/bin/clang++ \
             --status-bugs \
+            -v -o clang-analyzer-report/$ABI \
             $GNUMAKE -f $PROGDIR/core/build-local.mk "$@" APP_ABI=$ABI
     done
 else

Also report dir is changed here from /tmp to <CURRENT_BUILD_DIR>/clang-analyzer-report and splitted by ABI.

@DanAlbert DanAlbert self-assigned this Apr 18, 2017
@DanAlbert DanAlbert added this to the r15 milestone Apr 18, 2017
@DanAlbert
Copy link
Member

Thanks for the report (and thanks even more for the patches!). I'm still catching up on the past couple weeks, but I'll take a look at this some time before r15 goes to stable.

@andreya108
Copy link
Author

I've noticed than NDK_CCACHE feature works fine (and saves a lot of my time). It seems that NDK_ANALYZE implementation can be reworked to be very similar and less complicated... Just an idea...

@DanAlbert
Copy link
Member

Finally had a chance to look at this. Did some cleanup while I was at it:

https://android-review.googlesource.com/405992 Remove Clang toolchain path from setup.mk.
https://android-review.googlesource.com/405993 Make NDK_ANALYZE work with both GCC and Clang.
https://android-review.googlesource.com/405994 Add NDK_ANALYZER_OUT, expand the test.

Now our test will actually catch issues here, and the analyzer will work with both Clang and GCC.

I don't think I'll be cherry-picking these into r15. We're very late in the release cycle and this isn't a regression. This actually works as well as it ever has, it's just that it never got updated to allow compilation using Clang.

@DanAlbert DanAlbert modified the milestones: r16, r15 May 31, 2017
@DanAlbert
Copy link
Member

Merged into r16.

miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018
Not sure why this was ever hard wired to GCC. We can support both
easily enough.

Test: ndk-build NDK_ANALYZE=1 V=1
Test: ndk-build NDK_ANALYZE=1 V=1 NDK_TOOLCHAIN_VERSION=4.9
Bug: android/ndk#362
Change-Id: I378b95f76225ac09d0e0abe89d4d6022e74c8bd7
miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018
Allow analyzer output to be sent somewhere other than /tmp with
NDK_ANALYZER_OUT. Now that we can predict the output, improve the
test (since it clearly wasn't catching anything).

Also, remove the test config saying the test was clang specific. It
actually works with both Clang and GCC.

Test: ./run_tests.py --filter NDK_ANALYZE
Bug: android/ndk#362
Change-Id: I7226e6fba1190573280e8b0c21b8550fb5636005
@wustwg
Copy link

wustwg commented Sep 3, 2019

do you have any android cmake projects using NDK_ANALYZE=1 demos? Now our company has migarate our project from ndk-build to cmake ,but I can't find any samples or docs about how to use analyzer in cmake , can you help me ? thanks!!!!

@DanAlbert
Copy link
Member

Responded on #1072

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants