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

Support for building Android platforms #90

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
28 changes: 28 additions & 0 deletions android/AndroidFixup.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright 2013, The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#ifndef ANDROID_FIXUP_H
#define ANDROID_FIXUP_H
static inline void *rawmemchr(const void *s, int c) {
const unsigned char *ptr = s;
while (1) {
if (*ptr == c)
return (void *)ptr;
ptr++;
}
}
/* workaround for canonicalize_file_name */
#define canonicalize_file_name(path) realpath(path, NULL)
#endif /* ANDROID_FIXUP_H */
4 changes: 4 additions & 0 deletions android/android.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#pragma once
#define __user
#define __force
typedef unsigned __poll_t;
21 changes: 21 additions & 0 deletions android/argp.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright 2013, The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#ifndef ELFUTILS_ARGP_H
#define ELFUTILS_ARGP_H
// We don't have an implementation, but elfutils unconditionally includes this,
// and relies on its transitive includes in places.
#include <errno.h>
#endif /* ELFUTILS_ARGP_H */
21 changes: 21 additions & 0 deletions android/libintl.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright 2013, The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#ifndef ELFUTILS_LIBINTL_H
#define ELFUTILS_LIBINTL_H
/* no internalization */
#define gettext(x) (x)
#define dgettext(x, y) (y)
#endif /* ELFUTILS_LIBINTL_H */
5 changes: 5 additions & 0 deletions bindings.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,8 @@
#include "libbpf/src/btf.h"
#include "libbpf/src/libbpf.h"
#endif /* __LIBBPF_SYS_NOVENDOR */

#if defined (__aarch64__) || defined (__arm__)
#include <stdarg.h>
typedef va_list __va_list_tag;
#endif
156 changes: 116 additions & 40 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::fs::read_dir;
use std::path;
use std::path::Path;
use std::process;
use std::process::ExitStatus;

use nix::fcntl;

Expand Down Expand Up @@ -70,7 +71,6 @@ fn generate_bindings(src_dir: path::PathBuf) {
.allowlist_function("perf_.+")
.allowlist_function("ring_buffer_.+")
.allowlist_function("user_ring_buffer_.+")
.allowlist_function("vdprintf")
.allowlist_type("bpf_.+")
.allowlist_type("btf_.+")
.allowlist_type("xdp_.+")
Expand All @@ -79,6 +79,10 @@ fn generate_bindings(src_dir: path::PathBuf) {
.allowlist_var("BTF_.+")
.allowlist_var("XDP_.+")
.allowlist_var("PERF_.+")
.allowlist_type("__va_list_tag")
.blocklist_type("vdprintf")
.blocklist_type("libbpf_print_fn_t")
.blocklist_function("libbpf_set_print")
.parse_callbacks(Box::new(ignored_macros))
.header("bindings.h")
.clang_arg(format!("-I{}", src_dir.join("libbpf/include").display()))
Expand Down Expand Up @@ -114,16 +118,18 @@ fn main() {

generate_bindings(src_dir.clone());

let vendored_libbpf = cfg!(feature = "vendored-libbpf");
let vendored_libelf = cfg!(feature = "vendored-libelf");
let vendored_zlib = cfg!(feature = "vendored-zlib");
let android = build_android();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps drop a note to reference rust-lang/cargo#1197, which would allow us to enable different features for Android specifically.

Copy link
Collaborator

Choose a reason for hiding this comment

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

And thinking about this more...I think this will break the header embedding, among potentially others. So at the very least we need to adjust that as well.

A better solution may be something along the lines of what is suggested here in the last answer: https://stackoverflow.com/questions/57972355/enable-a-cargo-feature-by-default-when-the-target-arch-is-wasm

We don't care about dependency resolution because all dependencies are vendored in the repository, so I think this should work well (well, assuming it does, I haven't tried it).

Copy link
Author

Choose a reason for hiding this comment

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

I think it is not necessary now. In my daily use, I always need these dependencies. zlib has static libraries in android ndk (maybe I need to add this?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Huh? But the issue is still present, right?


let vendored_libbpf = cfg!(feature = "vendored-libbpf") || android;
let vendored_libelf = cfg!(feature = "vendored-libelf") || android;
let vendored_zlib = cfg!(feature = "vendored-zlib") || android;
println!("Using feature vendored-libbpf={}", vendored_libbpf);
println!("Using feature vendored-libelf={}", vendored_libelf);
println!("Using feature vendored-zlib={}", vendored_zlib);

let static_libbpf = cfg!(feature = "static-libbpf");
let static_libelf = cfg!(feature = "static-libelf");
let static_zlib = cfg!(feature = "static-zlib");
let static_libbpf = cfg!(feature = "static-libbpf") || android;
let static_libelf = cfg!(feature = "static-libelf") || android;
let static_zlib = cfg!(feature = "static-zlib") || android;
println!("Using feature static-libbpf={}", static_libbpf);
println!("Using feature static-libelf={}", static_libelf);
println!("Using feature static-zlib={}", static_zlib);
Expand All @@ -146,10 +152,10 @@ fn main() {
pkg_check("flex");
pkg_check("bison");
pkg_check("gawk");
pkg_check("aclocal");
Copy link
Collaborator

Choose a reason for hiding this comment

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

What needs aclocal?

}

let (compiler, mut cflags) = if vendored_libbpf || vendored_libelf || vendored_zlib {
pkg_check("make");
pkg_check("pkg-config");

let compiler = cc::Build::new().try_get_compiler().expect(
Expand Down Expand Up @@ -208,45 +214,73 @@ fn main() {
}
}

fn make_zlib(compiler: &cc::Tool, src_dir: &path::Path, out_dir: &path::Path) {
let src_dir = src_dir.join("zlib");
fn make_zlib(compiler: &cc::Tool, src_dir: &path::Path, _: &path::Path) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you don't use a previously present argument, can you remove it?

// lock README such that if two crates are trying to compile
// this at the same time (eg libbpf-rs libbpf-cargo)
// they wont trample each other
let file = std::fs::File::open(src_dir.join("README")).unwrap();
let _lock = fcntl::Flock::lock(file, fcntl::FlockArg::LockExclusive).unwrap();

let status = process::Command::new("./configure")
.arg("--static")
.arg("--prefix")
.arg(".")
.arg("--libdir")
.arg(out_dir)
.env("CC", compiler.path())
.env("CFLAGS", compiler.cflags_env())
.current_dir(&src_dir)
.status()
.expect("could not execute make");
let project_dir = src_dir.join("zlib");

assert!(status.success(), "make failed");

let status = process::Command::new("make")
.arg("install")
.arg("-j")
.arg(&format!("{}", num_cpus()))
.current_dir(&src_dir)
.status()
.expect("could not execute make");
let file = std::fs::File::open(project_dir.join("README")).unwrap();
let _lock = fcntl::Flock::lock(file, fcntl::FlockArg::LockExclusive).unwrap();

assert!(status.success(), "make failed");
let project_dir = project_dir.to_str().unwrap();

let zlib_sources = [
"adler32.c",
"compress.c",
"crc32.c",
"deflate.c",
"gzclose.c",
"gzlib.c",
"gzread.c",
"gzwrite.c",
"infback.c",
"inffast.c",
"inflate.c",
"inftrees.c",
"trees.c",
"uncompr.c",
"zutil.c",
];

// These flags are only used in Android
// ref: https://android.googlesource.com/platform/external/zlib/+/refs/tags/android-11.0.0_r48/Android.bp
let android_cflags = [
// We do support hidden visibility, so turn that on.
"-DHAVE_HIDDEN",
// We do support const, so turn that on.
"-DZLIB_CONST",
// Enable -O3 as per chromium.
"-O3",
// "-Wall",
// "-Werror",
// "-Wno-deprecated-non-prototype",
// "-Wno-unused",
// "-Wno-unused-parameter",
Comment on lines +248 to +258
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where are these flags coming from, all of a sudden? And what does chromium have to do with this?

Copy link
Collaborator

@danielocfb danielocfb Aug 21, 2024

Choose a reason for hiding this comment

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

In general, I kind of fail to understand what we gain from building our own build logic. The general guideline I am aware of is that the original build system should be used. If it provided a value, perhaps going against this recommendation is fine, but what value is that? Is the existing zlib build system incapable, somehow? Perhaps if you finally wrote a proper commit description I wouldn't have to inquire about every single detail and it would even be available for future developers...

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI Android keeps forks of elfutils(https://android.googlesource.com/platform/external/elfutils) and zlib(https://android.googlesource.com/platform/external/zlib/+/refs/heads/main) that uses the soong blueprint + ninja build system.

Clearly some flags are copied from there: https://android.googlesource.com/platform/external/zlib/+/refs/heads/main/Android.bp#20 . (That Android.bp file doesn't have a license header.)

Some of the flags used in the soong blueprint are known to be supported by the compiler prebuilt that the AOSP project distributes, but might not be supported by all compilers. I don't think it is appropriate to set those flags for non Android targets.

Copy link
Author

Choose a reason for hiding this comment

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

FYI Android keeps forks of elfutils(https://android.googlesource.com/platform/external/elfutils) and zlib(https://android.googlesource.com/platform/external/zlib/+/refs/heads/main) that uses the soong blueprint + ninja build system.

Clearly some flags are copied from there: https://android.googlesource.com/platform/external/zlib/+/refs/heads/main/Android.bp#20 . (That Android.bp file doesn't have a license header.)

Some of the flags used in the soong blueprint are known to be supported by the compiler prebuilt that the AOSP project distributes, but might not be supported by all compilers. I don't think it is appropriate to set those flags for non Android targets.

Yes, these flags are only used in Android

Copy link
Collaborator

@danielocfb danielocfb Aug 26, 2024

Choose a reason for hiding this comment

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

The larger question seems unanswered.

In general, I kind of fail to understand what we gain from building our own build logic. The general guideline I am aware of is that the original build system should be used. If it provided a value, perhaps, but what value is that? Is the existing zlib build system incapable, somehow? Perhaps if you finally wrote a proper commit description I wouldn't have to inquire about every single detail and it would even be available for future developers...

Putting that aside, please remove unused flags as well as references to Chromium. Perhaps these make some kind of sense in an Android build system context, but they have no business here.

];

configure(project_dir, vec![]);

let mut builder = cc::Build::new();

builder.include(project_dir).files({
zlib_sources
.iter()
.map(|source| format!("{project_dir}/{source}"))
});

if build_android() {
for flag in android_cflags {
builder.flag(flag);
}
} else {
for flag in compiler.args() {
builder.flag(flag);
}
}

let status = process::Command::new("make")
.arg("distclean")
.current_dir(&src_dir)
.status()
.expect("could not execute make");
builder.flag_if_supported("-w").warnings(false).compile("z");

assert!(status.success(), "make failed");
emit_rerun_directives_for_contents(&src_dir);
}

Expand Down Expand Up @@ -396,3 +430,45 @@ fn make_libbpf(
fn num_cpus() -> usize {
std::thread::available_parallelism().map_or(1, |count| count.get())
}

fn build_android() -> bool {
env::var("CARGO_CFG_TARGET_OS")
.expect("CARGO_CFG_TARGET_OS not set")
.eq("android")
}

fn configure<'a, P, A>(project_dir: P, args: A)
where
P: AsRef<str>,
A: IntoIterator<Item = &'a str>,
{
let project_dir = project_dir.as_ref();

let prog = format!("{project_dir}/configure");

unsafe {
let prog = std::ffi::CString::new(prog.as_bytes()).unwrap();
nix::libc::chmod(prog.as_ptr(), 0o755);
}

let status = subproc(prog, project_dir, args);

assert!(
status.success(),
"configure({}) failed: {}",
project_dir,
status
);
}

fn subproc<'a, P, A>(prog: P, workdir: &str, args: A) -> ExitStatus
where
P: AsRef<str>,
A: IntoIterator<Item = &'a str>,
{
process::Command::new(prog.as_ref())
.current_dir(workdir)
.args(args)
.status()
.expect(&format!("could not execute `{}`", prog.as_ref()))
}
18 changes: 0 additions & 18 deletions src/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5487,7 +5487,6 @@ extern "C" {
) -> ::std::os::raw::c_int;
}
pub type va_list = __builtin_va_list;
pub type __gnuc_va_list = __builtin_va_list;
#[repr(C)]
#[derive(Debug, Default, Copy, Clone)]
pub struct btf_header {
Expand Down Expand Up @@ -6046,13 +6045,6 @@ extern "C" {
opts: *const btf_dump_type_data_opts,
) -> ::std::os::raw::c_int;
}
extern "C" {
pub fn vdprintf(
__fd: ::std::os::raw::c_int,
__fmt: *const ::std::os::raw::c_char,
__arg: *mut __va_list_tag,
) -> ::std::os::raw::c_int;
}
pub type pid_t = __pid_t;
extern "C" {
pub fn libbpf_major_version() -> __u32;
Expand Down Expand Up @@ -6086,16 +6078,6 @@ pub const LIBBPF_WARN: libbpf_print_level = 0;
pub const LIBBPF_INFO: libbpf_print_level = 1;
pub const LIBBPF_DEBUG: libbpf_print_level = 2;
pub type libbpf_print_level = ::std::os::raw::c_uint;
pub type libbpf_print_fn_t = ::std::option::Option<
unsafe extern "C" fn(
level: libbpf_print_level,
arg1: *const ::std::os::raw::c_char,
ap: *mut __va_list_tag,
) -> ::std::os::raw::c_int,
>;
extern "C" {
pub fn libbpf_set_print(fn_: libbpf_print_fn_t) -> libbpf_print_fn_t;
}
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct bpf_object_open_opts {
Expand Down
20 changes: 20 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,26 @@ macro_rules! header {
};
}

pub type libbpf_print_fn_t = ::std::option::Option<
unsafe extern "C" fn(
level: libbpf_print_level,
arg1: *const ::std::os::raw::c_char,
ap: *mut __va_list_tag,
) -> ::std::os::raw::c_int,
>;


extern "C" {

pub fn vdprintf(
__fd: ::std::os::raw::c_int,
__fmt: *const ::std::os::raw::c_char,
__args: *mut __va_list_tag,
) -> ::std::os::raw::c_int;

pub fn libbpf_set_print(fn_: libbpf_print_fn_t) -> libbpf_print_fn_t;
}

/// Vendored libbpf headers
///
/// Tuple format is: (header filename, header contents)
Expand Down