If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

B2G's Android (Gonk) build doesn't use CFLAGS and CXXFLAGS from .userconfig

RESOLVED FIXED

Status

Firefox OS
GonkIntegration
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jld, Unassigned)

Tracking

unspecified
x86_64
Linux

Firefox Tracking Flags

(b2g18 fixed)

Details

(Reporter)

Description

5 years ago
I've been working on getting frame pointers for profiler stack walking (bug 831611 and bug 810526), and it would help to be able to set additional compiler flags for the Android build.  In the limit this can be accomplished with wrapper scripts or even by replacing parts of the prebuilt toolchain in place (the latter is what I'm currently doing), but it would be good if there were a less invasive way.  The flags in .userconfig work for the Gecko build, but not the Android bits, and I need the entire system to follow the frame-pointer ABI to get unbroken stacks.
(Reporter)

Updated

5 years ago
Summary: Android build doesn't use CFLAGS and CXXFLAGS from .userconfig → B2G's Android (Gonk) build doesn't use CFLAGS and CXXFLAGS from .userconfig
Mike, Dave, any suggestions on how to move forward here?
I took a quick look and for android, it looks like you might be able to use

TARGET_GLOBAL_CFLAGS and TARGET_GLOBAL_CPPFLAGS

I've added mwu in case he has any better ideas
Flags: needinfo?(mwu)
(Reporter)

Comment 3

5 years ago
TARGET_GLOBAL_CFLAGS does not seem to be it.  I built using a -include flag to force-include module-level asm to put a known string in the .rodata (to be reasonably unambiguous about whether my flags are actually being used), which works for Gecko via CFLAGS/CXXFLAGS, but nothing else.
TARGET_GLOBAL_CFLAGS would only apply to .c files
TARGET_GLOBAL_CPPFLAGS applies to .cpp files.

I added:

export TARGET_GLOBAL_CPPFLAGS=-DDAVE_HYLANDS

to my .userconfig, and then did:

touch system/vold/VoldCommand.cpp
./build.sh showcommands

and I see:

prebuilt/linux-x86/ccache/ccache prebuilt/linux-x86/toolchain/arm-linux-androideabi-4.4.x/bin/arm-linux-androideabi-g++ -I bionic/libc/kernel/common -I bionic/libc/kernel/arch-arm -I system/extras/ext4_utils -I external/openssl/include -I system/vold -I out/target/product/unagi/obj/EXECUTABLES/vold_intermediates -I dalvik/libnativehelper/include/nativehelper -isystem system/core/include -isystem hardware/libhardware/include -isystem hardware/libhardware_legacy/include -isystem hardware/ril/include -isystem dalvik/libnativehelper/include -isystem frameworks/base/include -isystem frameworks/base/opengl/include -isystem frameworks/base/native/include -isystem external/skia/include -isystem out/target/product/unagi/obj/include -isystem bionic/libc/arch-arm/include -isystem bionic/libc/include -isystem bionic/libstdc++/include -isystem bionic/libm/include -isystem bionic/libm/include/arm -isystem bionic/libthread_db/include -c  -fno-exceptions -Wno-multichar -msoft-float -fpic -ffunction-sections -fdata-sections -funwind-tables -fstack-protector -Wa,--noexecstack -Werror=format-security -fno-short-enums -march=armv7-a -mfloat-abi=softfp -mfpu=neon -include system/core/include/arch/linux-arm/AndroidConfig.h -I system/core/include/arch/linux-arm/ -Wno-psabi -mthumb-interwork -DANDROID -fmessage-length=0 -W -Wall -Wno-unused -Winit-self -Wpointer-arith -Werror=return-type -Werror=non-virtual-dtor -Werror=address -Werror=sequence-point -DNDEBUG -g -Wstrict-aliasing=2 -fgcse-after-reload -frerun-cse-after-loop -frename-registers -DNDEBUG -UDEBUG -DDAVE_HYLANDS -fvisibility-inlines-hidden -DANDROID -fmessage-length=0 -W -Wall -Wno-unused -Winit-self -Wpointer-arith -Wsign-promo -Werror=return-type -Werror=non-virtual-dtor -Werror=address -Werror=sequence-point -DNDEBUG -UDEBUG -mthumb -Os -fomit-frame-pointer -fno-strict-aliasing  -fno-rtti      -MD -MF out/target/product/unagi/obj/EXECUTABLES/vold_intermediates/VoldCommand.d -o out/target/product/unagi/obj/EXECUTABLES/vold_intermediates/VoldCommand.o system/vold/VoldCommand.cpp

So it seems to be working. I noticed that it doesn't rebuild the world when you do this.

- make sure you export TARGET_GLOBAL_CPPFLAGS (and TARGET_LOCAL_CFLAGS) from .userconfig
- do "rm -rf out" to force everything to be rebuilt.

The showcommands target is a psuedo-target which will cause the android build commands to be shown.
Hmm. I tried setting both using:

export TARGET_GLOBAL_CFLAGS=-DDAVE_HYLANDS_C
export TARGET_GLOBAL_CPPFLAGS=-DDAVE_HYLANDS_CPP

and then touched system/core/sdcard/sdcard.c and I don't see the cflags added.
(Reporter)

Comment 6

5 years ago
From a cursory reading of the makefiles, I think that CPPFLAGS in this case might mean C preprocessor flags, as it usually does.
(Reporter)

Comment 7

5 years ago
So, I ran the build under strace at -j1, and I can see that the top-level make process is forking and execing things with TARGET_GLOBAL_CFLAGS in the environment... and then the execve calls stop having TARGET_GLOBAL_CFLAGS in them.

Recursive grep on the makefiles isn't enlightening; nor is ltrace.
It seems that androids build/core/select.mk contains these lines:

$(combo_target)GLOBAL_CFLAGS := -fno-exceptions -Wno-multichar
$(combo_target)RELEASE_CFLAGS := -O2 -g -fno-strict-aliasing
$(combo_target)GLOBAL_LDFLAGS :=
$(combo_target)GLOBAL_ARFLAGS := crsP

which causes anything which sets TARGET_GLOBAL_CFLAGS in the environment to be wiped out
So I think that if you wanted to add something that could be controlled via .userconfig then you would need to create a new variable and modify build/core/combo/TARGET_linux-arm.mk to add that new variable to TARGET_GLOBAL_CFLAGS

As far as I can tell TARGET_GLOBAL_CFLAGS is passed to compile both C and C++ files, whereas TARGET_GLOBAL_CPPFLAGS is only passed to C++ files. See build/core/definitions.mk and search for transform-cpp-to-o and transform-c-or-s-to-o-no-deps

It appears that this file has already had modifications by us, so I would assume its ok to add some more.
I went ahead and merged this:
https://github.com/mozilla-b2g/platform_build/pull/19
https://github.com/mozilla-b2g/platform_build/commit/7fce578c221291b1cbd3ccaae2838bf57a39cd7c

and afterwards, I realized it was v1-train which I think requires some type of approval which this bug doesn't have.

This is one of those extremely low-risk changes, so I'll ask for approval after the fact.
tracking-b2g18: --- → ?
Jed - can you prepare a PR for master as well?
status-b2g18: --- → fixed
Component: General → Builds
tracking-b2g18: ? → -
tracking-b2g18: - → ---
(Reporter)

Comment 12

5 years ago
Done: https://github.com/mozilla-b2g/platform_build/pull/20
Flags: needinfo?(mwu)
ok - merged into master:
https://github.com/mozilla-b2g/platform_build/commit/f9f0e383f25088d279e356ea4c36d60aa31fd46a
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.