Fix link_type checking

This was printing "error:", but not actually triggering an error.
Instead of trying to write a single line bash script to handle this,
move the actual check into python. This allows us to print all of the
errors for a single module before triggering the failure.

Also updates the warning format and the warn.py script to properly parse
these warning. Many of the java:sdk -> java:platform warnings are false
positives due to the lack of LOCAL_SDK_VERSION markings on prebuilts.

Individual tags can be marked as warnings now, which lets us check for
system libraries linking against vendor libraries (which won't work on
AOSP). I'm not sure this is a completely valid check, which one reason
that it's just a warning.

Test: m all_link_types (with some missing libs commented out)
Change-Id: I333e418c9a4511b7c7e826891ae481da08fbf6f9
This commit is contained in:
Dan Willemsen 2016-09-14 21:38:29 -07:00
parent 029ced528b
commit 121e284b46
9 changed files with 164 additions and 46 deletions

View File

@ -1365,13 +1365,17 @@ endif
####################################################
my_link_type := $(intermediates)/link_type
all_link_types: $(my_link_type)
ifdef LOCAL_SDK_VERSION
$(my_link_type): PRIVATE_LINK_TYPE := ndk
$(my_link_type): PRIVATE_ALLOWED_TYPES := ndk
$(my_link_type): PRIVATE_LINK_TYPE := native:ndk
$(my_link_type): PRIVATE_WARN_TYPES :=
$(my_link_type): PRIVATE_ALLOWED_TYPES := native:ndk
else
$(my_link_type): PRIVATE_LINK_TYPE := platform
$(my_link_type): PRIVATE_ALLOWED_TYPES := (ndk|platform)
$(my_link_type): PRIVATE_LINK_TYPE := native:platform
$(my_link_type): PRIVATE_WARN_TYPES :=
$(my_link_type): PRIVATE_ALLOWED_TYPES := native:ndk native:platform
endif
$(eval $(call link-type-partitions,$(my_link_type)))
my_link_type_deps := $(strip \
$(foreach l,$(my_whole_static_libraries) $(my_static_libraries), \
$(call intermediates-dir-for,STATIC_LIBRARIES,$(l),$(my_kind),,$(LOCAL_2ND_ARCH_VAR_PREFIX),$(my_host_cross))/link_type))
@ -1383,16 +1387,9 @@ endif
$(my_link_type): PRIVATE_DEPS := $(my_link_type_deps)
$(my_link_type): PRIVATE_MODULE := $(LOCAL_MODULE)
$(my_link_type): PRIVATE_MAKEFILE := $(LOCAL_MODULE_MAKEFILE)
$(my_link_type): $(my_link_type_deps)
$(my_link_type): $(my_link_type_deps) $(CHECK_LINK_TYPE)
@echo Check module type: $@
$(hide) mkdir -p $(dir $@) && rm -f $@
ifdef my_link_type_deps
$(hide) for f in $(PRIVATE_DEPS); do \
grep -qE '^$(PRIVATE_ALLOWED_TYPES)$$' $$f || \
($(call echo-error,"$(PRIVATE_MAKEFILE): $(PRIVATE_MODULE) ($(PRIVATE_LINK_TYPE)) should not link to $$(basename $${f%_intermediates/link_type}) ($$(cat $$f))"); exit 1) \
done
endif
$(hide) echo $(PRIVATE_LINK_TYPE) >$@
$(check-link-type)
###########################################################

View File

@ -586,6 +586,7 @@ TUNE2FS := $(HOST_OUT_EXECUTABLES)/tune2fs$(HOST_EXECUTABLE_SUFFIX)
JARJAR := $(HOST_OUT_JAVA_LIBRARIES)/jarjar.jar
DATA_BINDING_COMPILER := $(HOST_OUT_JAVA_LIBRARIES)/databinding-compiler.jar
FAT16COPY := build/tools/fat16copy.py
CHECK_LINK_TYPE := build/tools/check_link_type.py
ifneq ($(ANDROID_JACK_EXTRA_ARGS),)
JACK_DEFAULT_ARGS :=

View File

@ -3115,6 +3115,35 @@ $(strip $(if $(LOCAL_RECORDED_MODULE_TYPE),,
$(error Invalid module type: $(1))))))
endef
###########################################################
# Link type checking
###########################################################
define check-link-type
$(hide) mkdir -p $(dir $@) && rm -f $@
$(hide) $(CHECK_LINK_TYPE) --makefile $(PRIVATE_MAKEFILE) --module $(PRIVATE_MODULE) \
--type "$(PRIVATE_LINK_TYPE)" $(addprefix --allowed ,$(PRIVATE_ALLOWED_TYPES)) \
$(addprefix --warn ,$(PRIVATE_WARN_TYPES)) $(PRIVATE_DEPS)
$(hide) echo "$(PRIVATE_LINK_TYPE)" >$@
endef
define link-type-partitions
ifndef LOCAL_IS_HOST_MODULE
ifeq (true,$(LOCAL_PROPRIETARY_MODULE))
$(1): PRIVATE_LINK_TYPE += partition:vendor
$(1): PRIVATE_ALLOWED_TYPES += partition:vendor partition:oem partition:odm
else ifeq (true,$(LOCAL_OEM_MODULE))
$(1): PRIVATE_LINK_TYPE += partition:oem
$(1): PRIVATE_ALLOWED_TYPES += partition:vendor partition:oem partition:odm
else ifeq (true,$(LOCAL_ODM_MODULE))
$(1): PRIVATE_LINK_TYPE += partition:odm
$(1): PRIVATE_ALLOWED_TYPES += partition:vendor partition:oem partition:odm
else
# TODO: Mark libraries in /data
$(1): PRIVATE_WARN_TYPES += partition:vendor partition:oem partition:odm
endif
endif
endef
###########################################################
## Other includes
###########################################################

View File

@ -102,28 +102,26 @@ endif # outer my_prebuilt_jni_libs
# Verify that all included libraries are built against the NDK
ifneq ($(strip $(LOCAL_JNI_SHARED_LIBRARIES)),)
my_link_type := $(call intermediates-dir-for,APPS,$(LOCAL_MODULE))/$(my_2nd_arch_prefix)jni_link_type
all_link_types: $(my_link_type)
my_link_type_deps := $(strip \
$(foreach l,$(LOCAL_JNI_SHARED_LIBRARIES),\
$(call intermediates-dir-for,SHARED_LIBRARIES,$(l),,,$(my_2nd_arch_prefix))/link_type))
ifneq ($(LOCAL_SDK_VERSION),)
$(my_link_type): PRIVATE_LINK_TYPE := sdk
$(my_link_type): PRIVATE_ALLOWED_TYPES := ndk
$(my_link_type): PRIVATE_LINK_TYPE := app:sdk
$(my_link_type): PRIVATE_WARN_TYPES := native:platform
$(my_link_type): PRIVATE_ALLOWED_TYPES := native:ndk
else
$(my_link_type): PRIVATE_LINK_TYPE := platform
$(my_link_type): PRIVATE_ALLOWED_TYPES := (ndk|platform)
$(my_link_type): PRIVATE_LINK_TYPE := app:platform
$(my_link_type): PRIVATE_WARN_TYPES :=
$(my_link_type): PRIVATE_ALLOWED_TYPES := native:ndk native:platform
endif
$(eval $(call link-type-partitions,$(my_link_type)))
$(my_link_type): PRIVATE_DEPS := $(my_link_type_deps)
$(my_link_type): PRIVATE_MODULE := $(LOCAL_MODULE)
$(my_link_type): PRIVATE_MAKEFILE := $(LOCAL_MODULE_MAKEFILE)
$(my_link_type): $(my_link_type_deps)
$(my_link_type): $(my_link_type_deps) $(CHECK_LINK_TYPE)
@echo Check JNI module types: $@
$(hide) mkdir -p $(dir $@)
$(hide) rm -f $@
$(hide) for f in $(PRIVATE_DEPS); do \
grep -qE '^$(PRIVATE_ALLOWED_TYPES)$$' $$f || \
$(call echo-warning,"$(PRIVATE_MAKEFILE): $(PRIVATE_MODULE) ($(PRIVATE_LINK_TYPE)) should not link to $$(basename $${f%_intermediates/link_type}) ($$(cat $$f))"); \
done
$(hide) touch $@
$(check-link-type)
$(LOCAL_BUILT_MODULE): | $(my_link_type)

View File

@ -379,6 +379,7 @@ endif # need_compile_java
###########################################################
ifndef LOCAL_IS_HOST_MODULE
my_link_type := $(intermediates.COMMON)/link_type
all_link_types: $(my_link_type)
my_link_type_deps := $(strip \
$(foreach lib,$(LOCAL_STATIC_JAVA_LIBRARIES),\
$(call intermediates-dir-for, \
@ -387,27 +388,25 @@ my_link_type_deps := $(strip \
$(call intermediates-dir-for, \
APPS,$(lib),,COMMON)/link_type))
ifeq ($(LOCAL_SDK_VERSION),system_current)
$(my_link_type): PRIVATE_LINK_TYPE := system
$(my_link_type): PRIVATE_ALLOWED_TYPES := (sdk|system)
$(my_link_type): PRIVATE_LINK_TYPE := java:system
$(my_link_type): PRIVATE_WARN_TYPES := java:platform
$(my_link_type): PRIVATE_ALLOWED_TYPES := java:sdk java:system
else ifneq ($(LOCAL_SDK_VERSION),)
$(my_link_type): PRIVATE_LINK_TYPE := sdk
$(my_link_type): PRIVATE_ALLOWED_TYPES := sdk
$(my_link_type): PRIVATE_LINK_TYPE := java:sdk
$(my_link_type): PRIVATE_WARN_TYPES := java:system java:platform
$(my_link_type): PRIVATE_ALLOWED_TYPES := java:sdk
else
$(my_link_type): PRIVATE_LINK_TYPE := platform
$(my_link_type): PRIVATE_ALLOWED_TYPES := (sdk|system|platform)
$(my_link_type): PRIVATE_LINK_TYPE := java:platform
$(my_link_type): PRIVATE_WARN_TYPES :=
$(my_link_type): PRIVATE_ALLOWED_TYPES := java:sdk java:system java:platform
endif
$(eval $(call link-type-partitions,$(my_link_type)))
$(my_link_type): PRIVATE_DEPS := $(my_link_type_deps)
$(my_link_type): PRIVATE_MODULE := $(LOCAL_MODULE)
$(my_link_type): PRIVATE_MAKEFILE := $(LOCAL_MODULE_MAKEFILE)
$(my_link_type): $(my_link_type_deps)
$(my_link_type): $(my_link_type_deps) $(CHECK_LINK_TYPE)
@echo Check Java library module types: $@
$(hide) mkdir -p $(dir $@)
$(hide) rm -f $@
$(hide) for f in $(PRIVATE_DEPS); do \
grep -qE '^$(PRIVATE_ALLOWED_TYPES)$$' $$f || \
$(call echo-warning,"$(PRIVATE_MAKEFILE): $(PRIVATE_MODULE) ($(PRIVATE_LINK_TYPE)) should not link to $$(basename $${f%_intermediates/link_type}) ($$(cat $$f))"); \
done
$(hide) echo $(PRIVATE_LINK_TYPE) >$@
$(check-link-type)
$(LOCAL_BUILT_MODULE): $(my_link_type)
endif # !LOCAL_IS_HOST_MODULE

View File

@ -1137,4 +1137,7 @@ tidy_only:
ndk: $(SOONG_OUT_DIR)/ndk.timestamp
.PHONY: ndk
.PHONY: all_link_types
all_link_types:
endif # KATI

View File

@ -138,11 +138,12 @@ else
endif
my_link_type := $(intermediates)/link_type
$(my_link_type): PRIVATE_LINK_TYPE := $(if $(LOCAL_SDK_VERSION),ndk,platform)
$(my_link_type): PRIVATE_LINK_TYPE := native:$(if $(LOCAL_SDK_VERSION),ndk,platform)
$(eval $(call link-type-partitions,$(my_link_type)))
$(my_link_type):
@echo Check module type: $@
$(hide) mkdir -p $(dir $@) && rm -f $@
$(hide) echo $(PRIVATE_LINK_TYPE) >$@
$(hide) echo "$(PRIVATE_LINK_TYPE)" >$@
$(LOCAL_BUILT_MODULE) : | $(export_includes) $(my_link_type)
endif # prebuilt_module_is_a_library
@ -397,16 +398,17 @@ $(common_classes_jar) $(common_javalib_jar): PRIVATE_MODULE := $(LOCAL_MODULE)
my_link_type := $(intermediates.COMMON)/link_type
ifeq ($(LOCAL_SDK_VERSION),system_current)
$(my_link_type): PRIVATE_LINK_TYPE := system
$(my_link_type): PRIVATE_LINK_TYPE := java:system
else ifneq ($(LOCAL_SDK_VERSION),)
$(my_link_type): PRIVATE_LINK_TYPE := sdk
$(my_link_type): PRIVATE_LINK_TYPE := java:sdk
else
$(my_link_type): PRIVATE_LINK_TYPE := platform
$(my_link_type): PRIVATE_LINK_TYPE := java:platform
endif
$(eval $(call link-type-partitions,$(my_link_type)))
$(my_link_type):
@echo Check module type: $@
$(hide) mkdir -p $(dir $@) && rm -f $@
$(hide) echo $(PRIVATE_LINK_TYPE) >$@
$(hide) echo "$(PRIVATE_LINK_TYPE)" >$@
$(LOCAL_BUILT_MODULE): $(my_link_type)
ifeq ($(prebuilt_module_is_dex_javalib),true)

80
tools/check_link_type.py Executable file
View File

@ -0,0 +1,80 @@
#!/usr/bin/env python
#
# Copyright (C) 2016 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.
"""Utility to verify modules link against acceptable module types"""
from __future__ import print_function
import argparse
import os
import sys
WARNING_MSG = ('\033[1m%(makefile)s: \033[35mwarning:\033[0m\033[1m '
'%(module)s (%(type)s) should not link to %(dep_name)s (%(dep_type)s)'
'\033[0m')
ERROR_MSG = ('\033[1m%(makefile)s: \033[31merror:\033[0m\033[1m '
'%(module)s (%(type)s) should not link to %(dep_name)s (%(dep_type)s)'
'\033[0m')
def parse_args():
"""Parse commandline arguments."""
parser = argparse.ArgumentParser(description='Check link types')
parser.add_argument('--makefile', help='Makefile defining module')
parser.add_argument('--module', help='The module being checked')
parser.add_argument('--type', help='The link type of module')
parser.add_argument('--allowed', help='Allow deps to use these types',
action='append', default=[], metavar='TYPE')
parser.add_argument('--warn', help='Warn if deps use these types',
action='append', default=[], metavar='TYPE')
parser.add_argument('deps', help='The dependencies to check',
metavar='DEP', nargs='*')
return parser.parse_args()
def print_msg(msg, args, dep_name, dep_type):
"""Print a warning or error message"""
print(msg % {
"makefile": args.makefile,
"module": args.module,
"type": args.type,
"dep_name": dep_name,
"dep_type": dep_type}, file=sys.stderr)
def main():
"""Program entry point."""
args = parse_args()
failed = False
for dep in args.deps:
dep_name = os.path.basename(os.path.dirname(dep))
if dep_name.endswith('_intermediates'):
dep_name = dep_name[:len(dep_name)-len('_intermediates')]
with open(dep, 'r') as dep_file:
dep_types = dep_file.read().strip().split(' ')
for dep_type in dep_types:
if dep_type in args.allowed:
continue
if dep_type in args.warn:
print_msg(WARNING_MSG, args, dep_name, dep_type)
else:
print_msg(ERROR_MSG, args, dep_name, dep_type)
failed = True
if failed:
sys.exit(1)
if __name__ == '__main__':
main()

View File

@ -91,9 +91,18 @@ warnpatterns = [
'description':'make: overriding commands/ignoring old commands',
'patterns':[r".*: warning: overriding commands for target .+",
r".*: warning: ignoring old commands for target .+"] },
{ 'category':'make', 'severity':severity.HIGH, 'members':[], 'option':'',
{ 'category':'make', 'severity':severity.HIGH, 'members':[], 'option':'',
'description':'make: LOCAL_CLANG is false',
'patterns':[r".*: warning: LOCAL_CLANG is set to false"] },
{ 'category':'make', 'severity':severity.HIGH, 'members':[], 'option':'',
'description':'SDK App using platform shared library',
'patterns':[r".*: warning: .+ \(.*app:sdk.*\) should not link to .+ \(native:platform\)"] },
{ 'category':'make', 'severity':severity.HIGH, 'members':[], 'option':'',
'description':'System module linking to a vendor module',
'patterns':[r".*: warning: .+ \(.+\) should not link to .+ \(partition:.+\)"] },
{ 'category':'make', 'severity':severity.MEDIUM, 'members':[], 'option':'',
'description':'Invalid SDK/NDK linking',
'patterns':[r".*: warning: .+ \(.+\) should not link to .+ \(.+\)"] },
{ 'category':'C/C++', 'severity':severity.HIGH, 'members':[], 'option':'-Wimplicit-function-declaration',
'description':'Implicit function declaration',
'patterns':[r".*: warning: implicit declaration of function .+",