Build fails with strict-aliasing violations

[eli-schwartz]

I tried to build with the following *FLAGS to optimize the build: -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing

The -Werror=* flags are important to detect cases where the compiler can try to optimize based on assuming UB cannot happen, and miscompile code that has UB in it. strict-aliasing issues are always bad but LTO can make them even worse.

I got this error:

[4/5] /usr/bin/x86_64-pc-linux-gnu-g++ -DNDEBUG -DSTATIC_ANDROIDFW_FOR_TOOLS -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -D_LARGEFILE64_SOURCE -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1_build/vendor -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/base/libs/androidfw/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/base/libs/androidfw/include_pathutils -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/core/libcutils/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/logging/liblog/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/core/libsystem/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/core/libutils/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/libbase/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/native/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/libziparchive/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/incremental_delivery/incfs/util/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/fmtlib/include  -pipe -march=native -fstack-protector-all -O2 -fdiagnostics-color=always -frecord-gcc-switches -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing -Werror=lto-type-mismatch  -Wformat -Werror=format-security -ftrivial-auto-var-init=zero -std=gnu++20 -Wno-attributes -MD -MT vendor/CMakeFiles/libandroidfw.dir/base/libs/androidfw/ResourceTypes.cpp.o -MF vendor/CMakeFiles/libandroidfw.dir/base/libs/androidfw/ResourceTypes.cpp.o.d -o vendor/CMakeFiles/libandroidfw.dir/base/libs/androidfw/ResourceTypes.cpp.o -c /var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/base/libs/androidfw/ResourceTypes.cpp
FAILED: vendor/CMakeFiles/libandroidfw.dir/base/libs/androidfw/ResourceTypes.cpp.o 
/usr/bin/x86_64-pc-linux-gnu-g++ -DNDEBUG -DSTATIC_ANDROIDFW_FOR_TOOLS -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -D_LARGEFILE64_SOURCE -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1_build/vendor -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/base/libs/androidfw/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/base/libs/androidfw/include_pathutils -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/core/libcutils/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/logging/liblog/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/core/libsystem/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/core/libutils/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/libbase/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/native/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/libziparchive/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/incremental_delivery/incfs/util/include -I/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/fmtlib/include  -pipe -march=native -fstack-protector-all -O2 -fdiagnostics-color=always -frecord-gcc-switches -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing -Werror=lto-type-mismatch  -Wformat -Werror=format-security -ftrivial-auto-var-init=zero -std=gnu++20 -Wno-attributes -MD -MT vendor/CMakeFiles/libandroidfw.dir/base/libs/androidfw/ResourceTypes.cpp.o -MF vendor/CMakeFiles/libandroidfw.dir/base/libs/androidfw/ResourceTypes.cpp.o.d -o vendor/CMakeFiles/libandroidfw.dir/base/libs/androidfw/ResourceTypes.cpp.o -c /var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/base/libs/androidfw/ResourceTypes.cpp
/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/base/libs/androidfw/ResourceTypes.cpp: In member function ‘int android::ResTable_config::diff(const android::ResTable_config&) const’:
/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/base/libs/androidfw/ResourceTypes.cpp:2234:52: warning: bitwise operation between different enumeration types ‘android::ResTable_config::<unnamed enum>’ and ‘android::ResTable_config::<unnamed enum>’ is deprecated [-Wdeprecated-enum-enum-conversion]
 2234 |     if (((inputFlags^o.inputFlags)&(MASK_KEYSHIDDEN|MASK_NAVHIDDEN)) != 0)
      |                                     ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/base/libs/androidfw/ResourceTypes.cpp: In static member function ‘static bool android::ResTable::stringToFloat(const char16_t*, size_t, android::Res_value*)’:
/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/base/libs/androidfw/ResourceTypes.cpp:5588:14: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
 5588 |             *(float*)(&outValue->data) = f;
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~
/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/base/libs/androidfw/ResourceTypes.cpp: In member function ‘void android::ResTable::print_value(const Package*, const android::Res_value&) const’:
/var/tmp/portage/dev-util/android-tools-35.0.1/work/android-tools-35.0.1/vendor/base/libs/androidfw/ResourceTypes.cpp:7619:33: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
 7619 |         printf("(float) %g\n", *(const float*)&value.data);
      |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: some warnings being treated as errors

Attached is a full build log. There are other errors too. build.log

There's a tracking issue downstream at https://bugs.gentoo.org/858311

...

Okay, so now you may ask, why am I reporting an error in Android source code to this wrapper project? Good question.

The answer is that I have looked everywhere in the codebase and I cannot tell where the libandroidfw target is ever used. It was added in commit be1a63d3986257b53c7482e5deb3837f140382ee but that commit never made use of it. Helpfully, the commit message is highly detailed and explains the whys and wherefores of why it was added:

commit be1a63d3986257b53c7482e5deb3837f140382ee
Author:     Đoàn Trần Công Danh <congdanhqx@gmail.com>
AuthorDate: Tue Nov 10 09:13:23 2020
Commit:     Đoàn Trần Công Danh <congdanhqx@gmail.com>
CommitDate: Tue Nov 10 09:53:38 2020

    vendor: add libandroidfw

It is never mentioned in build.ninja other than as part of the default "all" target. Using a sed to delete:

vendor/CMakeLists.txt:include(CMakeLists.libandroidfw.txt)

results in android-tools building and installing correctly (without LTO, anyway). So my assumption is that by dropping this unused dependency, google's source code bugs become "not our problem". Also ~25 fewer build edges means faster builds -- and 16mb less unpacked source code size (did not measure effects on compressed tarballs).

[Biswa96]

It was added in commit be1a63d3986257b53c7482e5deb3837f140382ee but that commit never made use of it.

@sgn Could you explain why libandroidfw was added in that commit?

[sgn]

It was added in commit be1a63d but that commit never made use of it.

@sgn Could you explain why libandroidfw was added in that commit?

TBH, I don't remember. I think there was a build failure at that time and it require libandroidfw. Please keep in mind that I started the development with 30.0.4 instead of 30.0.5. I guess it was required for 30.0.4 and dropped when it moved to 30.0.5.

[anatol]

As libandroidfw is not used by the android tools then it should be dropped from the codebase.

[eli-schwartz]

I think there was a build failure at that time and it require libandroidfw.

I even checked the history and could not tell any time when the cmake target defined by libandroidfw was used by another cmake target, so it surprises me that it could fix a build failure if it was still never used.

[Biswa96]

There was another native submodule which was added in e8f73d65571801f495f89529c07f5671a7612a18 commit is not used anymore.

After all these removals, the source tarball size reduces to 16.2 MB from 20.6 MB.