From 3bb168f10d8a8e7b4c0f2aa5840c21638a3d1fda Mon Sep 17 00:00:00 2001
From: Dan Willemsen <dwillemsen@google.com>
Date: Tue, 19 Jan 2016 20:07:08 -0800
Subject: [PATCH] Order objects based on LOCAL_SRC_FILES and
 LOCAL_GENERATED_SOURCES

We have been reordering objects to the linker based on how they were
generated. In soong, they're ordered based on the order listed in the
src_files.

Keep track of which source files created which object files so that we
can create the ordered list. Optionally change the order, based on
BINARY_OBJECTS_ORDER. That way we can compare make and soong builds.

Since we're keeping track of the used source files, warn when an entry
in LOCAL_SRC_FILES is not used. (whether it is an unused file like a
header, or a typo)

LOCAL_GENERATED_SOURCES is not verified, since it is valid to add
headers and other files in that list (to set up dependencies).

Change-Id: I1dfbbb3aa570c11c1db3b7133e46ed0b8c3b8989
---
 core/binary.mk            | 101 +++++++++++++++++++++++++++++++-------
 core/definitions.mk       |  36 ++++++++++++++
 core/prebuilt_internal.mk |   3 ++
 3 files changed, 122 insertions(+), 18 deletions(-)

diff --git a/core/binary.mk b/core/binary.mk
index 5f7ba663c4..d8b681630f 100644
--- a/core/binary.mk
+++ b/core/binary.mk
@@ -502,6 +502,34 @@ else
   debug_cflags :=
 endif
 
+####################################################
+## Keep track of src -> obj mapping
+####################################################
+
+my_tracked_gen_files :=
+my_tracked_src_files :=
+
+###########################################################
+## Stuff source generated from one-off tools
+###########################################################
+$(my_generated_sources): PRIVATE_MODULE := $(my_register_name)
+
+my_gen_sources_copy := $(patsubst $(generated_sources_dir)/%,$(intermediates)/%,$(filter $(generated_sources_dir)/%,$(my_generated_sources)))
+
+$(my_gen_sources_copy): $(intermediates)/% : $(generated_sources_dir)/% | $(ACP)
+	@echo "Copy: $@"
+	$(copy-file-to-target)
+
+my_generated_sources := $(patsubst $(generated_sources_dir)/%,$(intermediates)/%,$(my_generated_sources))
+
+# Generated sources that will actually produce object files.
+# Other files (like headers) are allowed in LOCAL_GENERATED_SOURCES,
+# since other compiled sources may depend on them, and we set up
+# the dependencies.
+my_gen_src_files := $(filter %.c %$(LOCAL_CPP_EXTENSION) %.S %.s %.o,$(my_generated_sources))
+
+ALL_GENERATED_SOURCES += $(my_generated_sources)
+
 ####################################################
 ## Compile RenderScript with reflected C++
 ####################################################
@@ -570,6 +598,8 @@ rs_generated_cpps := $(addprefix \
     $(renderscript_intermediate)/ScriptC_,$(patsubst %.fs,%.cpp, $(patsubst %.rs,%.cpp, \
     $(notdir $(renderscript_sources)))))
 
+$(call track-src-file-gen,$(renderscript_sources),$(rs_generated_cpps))
+
 # This is just a dummy rule to make sure gmake doesn't skip updating the dependents.
 $(rs_generated_cpps) : $(RenderScript_file_stamp)
 	@echo "Updated RS generated cpp file $@."
@@ -581,21 +611,6 @@ my_generated_sources += $(rs_generated_cpps)
 endif
 
 
-###########################################################
-## Stuff source generated from one-off tools
-###########################################################
-$(my_generated_sources): PRIVATE_MODULE := $(my_register_name)
-
-my_gen_sources_copy := $(patsubst $(generated_sources_dir)/%,$(intermediates)/%,$(filter $(generated_sources_dir)/%,$(my_generated_sources)))
-
-$(my_gen_sources_copy): $(intermediates)/% : $(generated_sources_dir)/% | $(ACP)
-	@echo "Copy: $@"
-	$(copy-file-to-target)
-
-my_generated_sources := $(patsubst $(generated_sources_dir)/%,$(intermediates)/%,$(my_generated_sources))
-
-ALL_GENERATED_SOURCES += $(my_generated_sources)
-
 ###########################################################
 ## Compile the .proto files to .cc (or .c) and then to .o
 ###########################################################
@@ -625,6 +640,7 @@ proto_generated_sources := $(addprefix $(proto_generated_sources_dir)/, \
 proto_generated_headers := $(patsubst %.pb$(my_proto_source_suffix),%.pb.h, $(proto_generated_sources))
 proto_generated_objects := $(addprefix $(proto_generated_obj_dir)/, \
     $(patsubst %.proto,%.pb.o,$(proto_sources_fullpath)))
+$(call track-src-file-obj,$(proto_sources),$(proto_generated_objects))
 
 # Ensure the transform-proto-to-cc rule is only defined once in multilib build.
 ifndef $(my_prefix)_$(LOCAL_MODULE_CLASS)_$(LOCAL_MODULE)_proto_defined
@@ -685,6 +701,9 @@ dbus_definition_paths := $(addprefix $(LOCAL_PATH)/,$(dbus_definitions))
 dbus_service_config := $(filter %dbus-service-config.json,$(my_src_files))
 dbus_service_config_path := $(addprefix $(LOCAL_PATH)/,$(dbus_service_config))
 
+# Mark these source files as not producing objects
+$(call track-src-file-obj,$(dbus_definitions) $(dbus_service_config),)
+
 dbus_gen_dir := $(generated_sources_dir)/dbus_bindings
 
 ifdef LOCAL_DBUS_PROXY_PREFIX
@@ -743,7 +762,7 @@ $(foreach s,$(aidl_src),\
     $(eval $(call define-aidl-cpp-rule,$(s),$(aidl_gen_cpp_root),aidl_gen_cpp)))
 $(foreach cpp,$(aidl_gen_cpp), \
     $(call include-depfile,$(addsuffix .aidl.P,$(basename $(cpp))),$(cpp)))
-
+$(call track-src-file-gen,$(aidl_src),$(aidl_gen_cpp))
 
 $(aidl_gen_cpp) : PRIVATE_MODULE := $(LOCAL_MODULE)
 $(aidl_gen_cpp) : PRIVATE_HEADER_OUTPUT_DIR := $(aidl_gen_include_root)
@@ -769,6 +788,7 @@ $(y_yacc_cs): $(intermediates)/%.c: \
     $(TOPDIR)$(LOCAL_PATH)/%.y \
     $(my_additional_dependencies)
 	$(call transform-y-to-c-or-cpp)
+$(call track-src-file-gen,$(y_yacc_sources),$(y_yacc_cs))
 
 my_generated_sources += $(y_yacc_cs)
 endif
@@ -781,6 +801,7 @@ $(yy_yacc_cpps): $(intermediates)/%$(LOCAL_CPP_EXTENSION): \
     $(TOPDIR)$(LOCAL_PATH)/%.yy \
     $(my_additional_dependencies)
 	$(call transform-y-to-c-or-cpp)
+$(call track-src-file-gen,$(yy_yacc_sources),$(yy_yacc_cpps))
 
 my_generated_sources += $(yy_yacc_cpps)
 endif
@@ -796,6 +817,7 @@ ifneq ($(l_lex_cs),)
 $(l_lex_cs): $(intermediates)/%.c: \
     $(TOPDIR)$(LOCAL_PATH)/%.l
 	$(transform-l-to-c-or-cpp)
+$(call track-src-file-gen,$(l_lex_sources),$(l_lex_cs))
 
 my_generated_sources += $(l_lex_cs)
 endif
@@ -807,6 +829,7 @@ ifneq ($(ll_lex_cpps),)
 $(ll_lex_cpps): $(intermediates)/%$(LOCAL_CPP_EXTENSION): \
     $(TOPDIR)$(LOCAL_PATH)/%.ll
 	$(transform-l-to-c-or-cpp)
+$(call track-src-file-gen,$(ll_lex_sources),$(ll_lex_cpps))
 
 my_generated_sources += $(ll_lex_cpps)
 endif
@@ -821,6 +844,7 @@ cpp_arm_sources := $(patsubst %$(LOCAL_CPP_EXTENSION).arm,%$(LOCAL_CPP_EXTENSION
 dotdot_arm_sources := $(filter ../%,$(cpp_arm_sources))
 cpp_arm_sources := $(filter-out ../%,$(cpp_arm_sources))
 cpp_arm_objects := $(addprefix $(intermediates)/,$(cpp_arm_sources:$(LOCAL_CPP_EXTENSION)=.o))
+$(call track-src-file-obj,$(patsubst %,%.arm,$(cpp_arm_sources)),$(cpp_arm_objects))
 
 # For source files starting with ../, we remove all the ../ in the object file path,
 # to avoid object file escaping the intermediate directory.
@@ -829,6 +853,7 @@ $(foreach s,$(dotdot_arm_sources),\
   $(eval $(call compile-dotdot-cpp-file,$(s),\
   $(yacc_cpps) $(proto_generated_headers) $(my_additional_dependencies),\
   dotdot_arm_objects)))
+$(call track-src-file-obj,$(patsubst %,%.arm,$(dotdot_arm_sources)),$(dotdot_arm_objects))
 
 dotdot_sources := $(filter ../%$(LOCAL_CPP_EXTENSION),$(my_src_files))
 dotdot_objects :=
@@ -836,9 +861,11 @@ $(foreach s,$(dotdot_sources),\
   $(eval $(call compile-dotdot-cpp-file,$(s),\
     $(yacc_cpps) $(proto_generated_headers) $(my_additional_dependencies),\
     dotdot_objects)))
+$(call track-src-file-obj,$(dotdot_sources),$(dotdot_objects))
 
 cpp_normal_sources := $(filter-out ../%,$(filter %$(LOCAL_CPP_EXTENSION),$(my_src_files)))
 cpp_normal_objects := $(addprefix $(intermediates)/,$(cpp_normal_sources:$(LOCAL_CPP_EXTENSION)=.o))
+$(call track-src-file-obj,$(cpp_normal_sources),$(cpp_normal_objects))
 
 $(dotdot_arm_objects) $(cpp_arm_objects): PRIVATE_ARM_MODE := $(arm_objects_mode)
 $(dotdot_arm_objects) $(cpp_arm_objects): PRIVATE_ARM_CFLAGS := $(arm_objects_cflags)
@@ -864,6 +891,7 @@ cpp_objects += $(dotdot_arm_objects) $(dotdot_objects)
 
 gen_cpp_sources := $(filter %$(LOCAL_CPP_EXTENSION),$(my_generated_sources))
 gen_cpp_objects := $(gen_cpp_sources:%$(LOCAL_CPP_EXTENSION)=%.o)
+$(call track-gen-file-obj,$(gen_cpp_sources),$(gen_cpp_objects))
 
 ifneq ($(strip $(gen_cpp_objects)),)
 # Compile all generated files as thumb.
@@ -884,6 +912,7 @@ endif
 
 gen_S_sources := $(filter %.S,$(my_generated_sources))
 gen_S_objects := $(gen_S_sources:%.S=%.o)
+$(call track-gen-file-obj,$(gen_S_sources),$(gen_S_objects))
 
 ifneq ($(strip $(gen_S_sources)),)
 $(gen_S_objects): $(intermediates)/%.o: $(intermediates)/%.S \
@@ -894,6 +923,7 @@ endif
 
 gen_s_sources := $(filter %.s,$(my_generated_sources))
 gen_s_objects := $(gen_s_sources:%.s=%.o)
+$(call track-gen-file-obj,$(gen_s_sources),$(gen_s_objects))
 
 ifneq ($(strip $(gen_s_objects)),)
 $(gen_s_objects): $(intermediates)/%.o: $(intermediates)/%.s \
@@ -919,6 +949,7 @@ c_arm_sources := $(patsubst %.c.arm,%.c,$(filter %.c.arm,$(my_src_files)))
 dotdot_arm_sources := $(filter ../%,$(c_arm_sources))
 c_arm_sources := $(filter-out ../%,$(c_arm_sources))
 c_arm_objects := $(addprefix $(intermediates)/,$(c_arm_sources:.c=.o))
+$(call track-src-file-obj,$(patsubst %,%.arm,$(c_arm_sources)),$(c_arm_objects))
 
 # For source files starting with ../, we remove all the ../ in the object file path,
 # to avoid object file escaping the intermediate directory.
@@ -927,6 +958,7 @@ $(foreach s,$(dotdot_arm_sources),\
   $(eval $(call compile-dotdot-c-file,$(s),\
     $(yacc_cpps) $(proto_generated_headers) $(my_additional_dependencies),\
     dotdot_arm_objects)))
+$(call track-src-file-obj,$(patsubst %,%.arm,$(dotdot_arm_sources)),$(dotdot_arm_objects))
 
 dotdot_sources := $(filter ../%.c, $(my_src_files))
 dotdot_objects :=
@@ -934,9 +966,11 @@ $(foreach s, $(dotdot_sources),\
   $(eval $(call compile-dotdot-c-file,$(s),\
     $(yacc_cpps) $(proto_generated_headers) $(my_additional_dependencies),\
     dotdot_objects)))
+$(call track-src-file-obj,$(dotdot_sources),$(dotdot_objects))
 
 c_normal_sources := $(filter-out ../%,$(filter %.c,$(my_src_files)))
 c_normal_objects := $(addprefix $(intermediates)/,$(c_normal_sources:.c=.o))
+$(call track-src-file-obj,$(c_normal_sources),$(c_normal_objects))
 
 $(dotdot_arm_objects) $(c_arm_objects): PRIVATE_ARM_MODE := $(arm_objects_mode)
 $(dotdot_arm_objects) $(c_arm_objects): PRIVATE_ARM_CFLAGS := $(arm_objects_cflags)
@@ -960,6 +994,7 @@ c_objects += $(dotdot_arm_objects) $(dotdot_objects)
 
 gen_c_sources := $(filter %.c,$(my_generated_sources))
 gen_c_objects := $(gen_c_sources:%.c=%.o)
+$(call track-gen-file-obj,$(gen_c_sources),$(gen_c_objects))
 
 ifneq ($(strip $(gen_c_objects)),)
 # Compile all generated files as thumb.
@@ -1008,12 +1043,14 @@ asm_sources_S := $(filter %.S,$(my_src_files))
 dotdot_sources := $(filter ../%,$(asm_sources_S))
 asm_sources_S := $(filter-out ../%,$(asm_sources_S))
 asm_objects_S := $(addprefix $(intermediates)/,$(asm_sources_S:.S=.o))
+$(call track-src-file-obj,$(asm_sources_S),$(asm_objects_S))
 
 dotdot_objects_S :=
 $(foreach s,$(dotdot_sources),\
   $(eval $(call compile-dotdot-s-file,$(s),\
     $(my_additional_dependencies),\
     dotdot_objects_S)))
+$(call track-src-file-obj,$(dotdot_sources),$(dotdot_objects_S))
 
 ifneq ($(strip $(asm_objects_S)),)
 $(asm_objects_S): $(intermediates)/%.o: $(TOPDIR)$(LOCAL_PATH)/%.S \
@@ -1026,12 +1063,14 @@ asm_sources_s := $(filter %.s,$(my_src_files))
 dotdot_sources := $(filter ../%,$(asm_sources_s))
 asm_sources_s := $(filter-out ../%,$(asm_sources_s))
 asm_objects_s := $(addprefix $(intermediates)/,$(asm_sources_s:.s=.o))
+$(call track-src-file-obj,$(asm_sources_s),$(asm_objects_s))
 
 dotdot_objects_s :=
 $(foreach s,$(dotdot_sources),\
   $(eval $(call compile-dotdot-s-file-no-deps,$(s),\
     $(my_additional_dependencies),\
     dotdot_objects_s)))
+$(call track-src-file-obj,$(dotdot_sources),$(dotdot_objects_s))
 
 ifneq ($(strip $(asm_objects_s)),)
 $(asm_objects_s): $(intermediates)/%.o: $(TOPDIR)$(LOCAL_PATH)/%.s \
@@ -1050,6 +1089,7 @@ asm_objects_asm := $(addprefix $(intermediates)/,$(asm_sources_asm:.asm=.o))
 $(asm_objects_asm): $(intermediates)/%.o: $(TOPDIR)$(LOCAL_PATH)/%.asm \
     $(my_additional_dependencies)
 	$(transform-asm-to-o)
+$(call track-src-file-obj,$(asm_sources_asm),$(asm_objects_asm))
 
 asm_objects += $(asm_objects_asm)
 endif
@@ -1104,6 +1144,11 @@ endif
 ## Common object handling.
 ###########################################################
 
+my_unused_src_files := $(filter-out $(logtags_sources) $(my_tracked_src_files),$(my_src_files) $(my_gen_src_files))
+ifneq ($(my_unused_src_files),)
+  $(warning $(LOCAL_MODULE_MAKEFILE): $(LOCAL_MODULE): Unused source files: $(my_unused_src_files))
+endif
+
 # some rules depend on asm_objects being first.  If your code depends on
 # being first, it's reasonable to require it to be assembly
 normal_objects := \
@@ -1115,11 +1160,31 @@ normal_objects := \
     $(gen_c_objects) \
     $(objc_objects) \
     $(objcpp_objects) \
-    $(proto_generated_objects) \
-    $(addprefix $(TOPDIR)$(LOCAL_PATH)/,$(LOCAL_PREBUILT_OBJ_FILES))
+    $(proto_generated_objects)
+
+new_order_normal_objects := $(foreach f,$(my_src_files),$(my_src_file_obj_$(f)))
+new_order_normal_objects += $(foreach f,$(my_gen_src_files),$(my_src_file_obj_$(f)))
+
+ifneq ($(sort $(normal_objects)),$(sort $(new_order_normal_objects)))
+$(warning $(LOCAL_MODULE_MAKEFILE) Internal build system warning: New object list does not match old)
+$(info Only in old: $(filter-out $(new_order_normal_objects),$(sort $(normal_objects))))
+$(info Only in new: $(filter-out $(normal_objects),$(sort $(new_order_normal_objects))))
+endif
+
+ifeq ($(BINARY_OBJECTS_ORDER),soong)
+normal_objects := $(new_order_normal_objects)
+endif
+
+normal_objects += $(addprefix $(TOPDIR)$(LOCAL_PATH)/,$(LOCAL_PREBUILT_OBJ_FILES))
 
 all_objects := $(normal_objects) $(gen_o_objects)
 
+# Cleanup file tracking
+$(foreach f,$(my_tracked_gen_files),$(eval my_src_file_gen_$(s):=))
+my_tracked_gen_files :=
+$(foreach f,$(my_tracked_src_files),$(eval my_src_file_obj_$(s):=))
+my_tracked_src_files :=
+
 my_c_includes += $(TOPDIR)$(LOCAL_PATH) $(intermediates) $(generated_sources_dir)
 
 ifndef LOCAL_SDK_VERSION
diff --git a/core/definitions.mk b/core/definitions.mk
index 78b607b631..a0b2976c88 100644
--- a/core/definitions.mk
+++ b/core/definitions.mk
@@ -904,6 +904,42 @@ $(eval -include $1)
 endef
 endif
 
+###########################################################
+## Track source files compiled to objects
+###########################################################
+# $(1): list of sources
+# $(2): list of matching objects
+define track-src-file-obj
+$(eval $(call _track-src-file-obj,$(1)))
+endef
+define _track-src-file-obj
+i := w
+$(foreach s,$(1),
+my_tracked_src_files += $(s)
+my_src_file_obj_$(s) := $$(word $$(words $$(i)),$$(2))
+i += w)
+endef
+
+# $(1): list of sources
+# $(2): list of matching generated sources
+define track-src-file-gen
+$(eval $(call _track-src-file-gen,$(2)))
+endef
+define _track-src-file-gen
+i := w
+$(foreach s,$(1),
+my_tracked_gen_files += $(s)
+my_src_file_gen_$(s) := $$(word $$(words $$(i)),$$(1))
+i += w)
+endef
+
+# $(1): list of generated sources
+# $(2): list of matching objects
+define track-gen-file-obj
+$(call track-src-file-obj,$(foreach f,$(1),\
+  $(or $(my_src_file_gen_$(f)),$(f))),$(2))
+endef
+
 ###########################################################
 ## Commands for running lex
 ###########################################################
diff --git a/core/prebuilt_internal.mk b/core/prebuilt_internal.mk
index 601a79c431..6d9ded42d4 100644
--- a/core/prebuilt_internal.mk
+++ b/core/prebuilt_internal.mk
@@ -26,11 +26,14 @@ ifdef LOCAL_PREBUILT_MODULE_FILE
 else
   ifdef LOCAL_SRC_FILES_$($(my_prefix)$(LOCAL_2ND_ARCH_VAR_PREFIX)ARCH)
     my_prebuilt_src_file := $(LOCAL_PATH)/$(LOCAL_SRC_FILES_$($(my_prefix)$(LOCAL_2ND_ARCH_VAR_PREFIX)ARCH))
+    LOCAL_SRC_FILES_$($(my_prefix)$(LOCAL_2ND_ARCH_VAR_PREFIX)ARCH)) :=
   else
     ifdef LOCAL_SRC_FILES_$(my_32_64_bit_suffix)
       my_prebuilt_src_file := $(LOCAL_PATH)/$(LOCAL_SRC_FILES_$(my_32_64_bit_suffix))
+      LOCAL_SRC_FILES_$(my_32_64_bit_suffix) :=
     else
       my_prebuilt_src_file := $(LOCAL_PATH)/$(LOCAL_SRC_FILES)
+      LOCAL_SRC_FILES :=
     endif
   endif
 endif