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.

Sign up for GitHub

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

Jump to bottom

Test Android in CI #730

Closed
wants to merge 3 commits into from
Closed

Test Android in CI #730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f03e99a
android: Use standard __ANDROID__ preprocessor macro
fmeum Apr 25, 2023
727197b
Revert "Add change to .bazelrc to build for Java 8 on Android"
fmeum Apr 25, 2023
78568a6
bazel: Unconditionally set Android flags
fmeum Apr 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions .bazelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,8 @@ build --tool_java_runtime_version=remotejdk_17
build --experimental_java_classpath=bazel

# Android
build:android_arm --incompatible_enable_android_toolchain_resolution
build:android_arm --android_platforms=//:android_arm64
build:android_arm --copt=-D_ANDROID
build:android_arm --java_runtime_version=8
Copy link
Contributor

Choose a reason for hiding this comment

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

When I tested locally, everything worked except this value still needed to be "=local_jdk" to find the correct toolchain

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for checking. That probably means that this is blocked onhttps://github.com/bazelbuild/bazel/pull/18262. I will keep it open and we can test it again after that change has landed in Bazel.

Copy link
Contributor

Choose a reason for hiding this comment

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

@fmeum Thanks so much for following up with this, we tried many toolchain workarounds for --java_runtime_version flag but couldn't get it to work. Hopefully the above PR will unblock this feature.

In the meantime, we are still hoping to add a basic CI test to ensure the critical targets that we have will continue to build.

Currently we rely heavily on the following two commands:

bazelisk build launcher/android:jazzer_android --config=android_arm
bazelisk build src/main/java/com/code_intelligence/jazzer/android:jazzer_standalone_android --config=android_arm

Can you please offer some guidance on how we can incorporate these commands into the current pipeline? Apologies for the rudimentary question, but I don't have much bazel testing experience yet.

We really just need to make sure that those two commands pass to ensure that Android is in a good state in Jazzer. I think it is important to note that even though the below test commands still execute as expected and the test passes, trying to build these targets individually will still fail:

bazelisk test --config=ci --remote_header=x-buildbuddy-api-key=*** --java_runtime_version=local_jdk_8 --disk_cache=/home/runner/.cache/bazel-disk //launcher/android:jazzer_android --build_tag_filters="-no-linux-jdk8,-no-jdk8" --test_tag_filters="-no-linux-jdk8,-no-jdk8" //...

Thanks in advance, I am happy to implement the test, just need some guidance on where to start.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@marktefftech Could you try adding --config=android_arm as well as the missing label to

jazzer/.github/workflows/run-all-tests.yml

Line 42 in 73e7641

bazel_args: "//launcher/android:jazzer_android"
?

build --incompatible_enable_android_toolchain_resolution
build --android_platforms=//:android_arm64

# Windows
# Only compiles with clang on Windows.
Expand Down
12 changes: 6 additions & 6 deletions launcher/jvm_tooling.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

#include "jvm_tooling.h"

#if defined(_ANDROID)
#if defined(__ANDROID__)
#include <dlfcn.h>
#elif defined(__APPLE__)
#include <mach-o/dyld.h>
Expand Down Expand Up @@ -67,7 +67,7 @@ std::string getExecutablePath() {
if (_NSGetExecutablePath(buf, &buf_size) != 0) {
#elif defined(_WIN32)
if (GetModuleFileNameA(NULL, buf, sizeof(buf)) == 0) {
#elif defined(_ANDROID)
#elif defined(__ANDROID__)
if (true) {
#else // Assume Linux
if (readlink("/proc/self/exe", buf, sizeof(buf)) == -1) {
Expand Down Expand Up @@ -155,7 +155,7 @@ std::vector<std::string> splitEscaped(const std::string &str) {

namespace jazzer {

#if defined(_ANDROID)
#if defined(__ANDROID__)
typedef jint (*JNI_CreateJavaVM_t)(JavaVM **, JNIEnv **, void *);
JNI_CreateJavaVM_t LoadAndroidVMLibs() {
std::cout << "Loading Android libraries" << std::endl;
Expand Down Expand Up @@ -221,7 +221,7 @@ JVM::JVM() {
options.push_back(
JavaVMOption{.optionString = const_cast<char *>(class_path.c_str())});

#if !defined(_ANDROID)
#if !defined(__ANDROID__)
// Set the maximum heap size to a value that is slightly smaller than
// libFuzzer's default rss_limit_mb. This prevents erroneous oom reports.
options.push_back(JavaVMOption{.optionString = (char *)"-Xmx1800m"});
Expand Down Expand Up @@ -272,7 +272,7 @@ JVM::JVM() {
}
}

#if !defined(_ANDROID)
#if !defined(__ANDROID__)
jint jni_version = JNI_VERSION_1_8;
#else
jint jni_version = JNI_VERSION_1_6;
Expand All @@ -283,7 +283,7 @@ JVM::JVM() {
.options = options.data(),
.ignoreUnrecognized = JNI_FALSE};

#if !defined(_ANDROID)
#if !defined(__ANDROID__)
int ret = JNI_CreateJavaVM(&jvm_, (void **)&env_, &jvm_init_args);
#else
JNI_CreateJavaVM_t CreateArtVM = LoadAndroidVMLibs();
Expand Down
2 changes: 1 addition & 1 deletion src/main/native/com/code_intelligence/jazzer/driver/init_jazzer_preload.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

#include <cstdlib>

#if defined(_ANDROID)
#if defined(__ANDROID__)
#define __jni_version__ JNI_VERSION_1_6
#else
#define __jni_version__ JNI_VERSION_1_8
Expand Down