Skip to content

Commit 323b020

Browse files
magicusmagicus
committed
8367034: [REDO] Protect ExecuteWithLog from running with redirection without a subshell
Reviewed-by: erikj
1 parent 48831c6 commit 323b020

File tree

5 files changed

+39
-30
lines changed

5 files changed

+39
-30
lines changed

‎make/RunTests.gmk‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ define SetupRunGtestTestBody
509509
$$(call LogWarn)
510510
$$(call LogWarn, Running test '$$($1_TEST)')
511511
$$(call MakeDir, $$($1_TEST_RESULTS_DIR) $$($1_TEST_SUPPORT_DIR))
512-
$$(call ExecuteWithLog, $$($1_TEST_SUPPORT_DIR)/gtest, ( \
512+
$$(call ExecuteWithLog, $$($1_TEST_SUPPORT_DIR)/gtest, \
513513
$$(CD) $$($1_TEST_SUPPORT_DIR) && \
514514
$$(FIXPATH) $$(TEST_IMAGE_DIR)/hotspot/gtest/$$($1_VARIANT)/gtestLauncher \
515515
-jdk $(JDK_UNDER_TEST) $$($1_GTEST_FILTER) \
@@ -520,7 +520,7 @@ define SetupRunGtestTestBody
520520
> >($(TEE) $$($1_TEST_RESULTS_DIR)/gtest.txt) \
521521
&& $$(ECHO) $$$$? > $$($1_EXITCODE) \
522522
|| $$(ECHO) $$$$? > $$($1_EXITCODE) \
523-
))
523+
)
524524

525525
$1_RESULT_FILE := $$($1_TEST_RESULTS_DIR)/gtest.txt
526526

@@ -644,7 +644,7 @@ define SetupRunMicroTestBody
644644
$$(call LogWarn)
645645
$$(call LogWarn, Running test '$$($1_TEST)')
646646
$$(call MakeDir, $$($1_TEST_RESULTS_DIR) $$($1_TEST_SUPPORT_DIR))
647-
$$(call ExecuteWithLog, $$($1_TEST_SUPPORT_DIR)/micro, ( \
647+
$$(call ExecuteWithLog, $$($1_TEST_SUPPORT_DIR)/micro, \
648648
$$(CD) $$(TEST_IMAGE_DIR) && \
649649
$$(FIXPATH) $$($1_MICRO_TEST_JDK)/bin/java $$($1_MICRO_JAVA_OPTIONS) \
650650
-jar $$($1_MICRO_BENCHMARKS_JAR) \
@@ -655,7 +655,7 @@ define SetupRunMicroTestBody
655655
> >($(TEE) $$($1_TEST_RESULTS_DIR)/micro.txt) \
656656
&& $$(ECHO) $$$$? > $$($1_EXITCODE) \
657657
|| $$(ECHO) $$$$? > $$($1_EXITCODE) \
658-
))
658+
)
659659

660660
$1_RESULT_FILE := $$($1_TEST_RESULTS_DIR)/micro.txt
661661

@@ -758,34 +758,34 @@ define SetupAOTBody
758758
ifeq ($$($1_TRAINING), onestep)
759759

760760
$$(call LogWarn, AOT: Create AOT cache $$($1_AOT_JDK_CACHE) in one step with flags: $$($1_VM_OPTIONS)) \
761-
$$(call ExecuteWithLog, $$($1_AOT_JDK_OUTPUT_DIR), ( \
761+
$$(call ExecuteWithLog, $$($1_AOT_JDK_OUTPUT_DIR), \
762762
cd $$($1_AOT_JDK_OUTPUT_DIR); \
763763
$(JAR) --extract --file $(TEST_IMAGE_DIR)/setup_aot/TestSetupAOT.jar; \
764764
$$(FIXPATH) $(JDK_UNDER_TEST)/bin/java $$($1_VM_OPTIONS) \
765-
-Xlog:class+load,aot,aot+class=debug:file=$$($1_AOT_JDK_CACHE).log -Xlog:cds*=error -Xlog:aot*=error \
765+
-Xlog:class+load$$(COMMA)aot$$(COMMA)aot+class=debug:file=$$($1_AOT_JDK_CACHE).log -Xlog:cds*=error -Xlog:aot*=error \
766766
-XX:AOTMode=record -XX:AOTCacheOutput=$$($1_AOT_JDK_CACHE) \
767767
TestSetupAOT $$($1_AOT_JDK_OUTPUT_DIR) > $$($1_AOT_JDK_LOG) \
768-
))
768+
)
769769

770770
else
771771

772772
$$(call LogWarn, AOT: Create cache configuration) \
773-
$$(call ExecuteWithLog, $$($1_AOT_JDK_OUTPUT_DIR), ( \
773+
$$(call ExecuteWithLog, $$($1_AOT_JDK_OUTPUT_DIR), \
774774
cd $$($1_AOT_JDK_OUTPUT_DIR); \
775775
$(JAR) --extract --file $(TEST_IMAGE_DIR)/setup_aot/TestSetupAOT.jar; \
776776
$$(FIXPATH) $(JDK_UNDER_TEST)/bin/java $$($1_VM_OPTIONS) \
777-
-Xlog:class+load,aot,aot+class=debug:file=$$($1_AOT_JDK_CONF).log -Xlog:cds*=error -Xlog:aot*=error \
777+
-Xlog:class+load$$(COMMA)aot$$(COMMA)aot+class=debug:file=$$($1_AOT_JDK_CONF).log -Xlog:cds*=error -Xlog:aot*=error \
778778
-XX:AOTMode=record -XX:AOTConfiguration=$$($1_AOT_JDK_CONF) \
779779
TestSetupAOT $$($1_AOT_JDK_OUTPUT_DIR) > $$($1_AOT_JDK_LOG) \
780-
))
780+
)
781781

782782
$$(call LogWarn, AOT: Generate AOT cache $$($1_AOT_JDK_CACHE) with flags: $$($1_VM_OPTIONS))
783-
$$(call ExecuteWithLog, $$($1_AOT_JDK_OUTPUT_DIR), ( \
783+
$$(call ExecuteWithLog, $$($1_AOT_JDK_OUTPUT_DIR), \
784784
$$(FIXPATH) $(JDK_UNDER_TEST)/bin/java \
785-
$$($1_VM_OPTIONS) -Xlog:aot,aot+class=debug:file=$$($1_AOT_JDK_CACHE).log -Xlog:cds*=error -Xlog:aot*=error \
785+
$$($1_VM_OPTIONS) -Xlog:aot$$(COMMA)aot+class=debug:file=$$($1_AOT_JDK_CACHE).log -Xlog:cds*=error -Xlog:aot*=error \
786786
-XX:ExtraSharedClassListFile=$(JDK_UNDER_TEST)/lib/classlist \
787787
-XX:AOTMode=create -XX:AOTConfiguration=$$($1_AOT_JDK_CONF) -XX:AOTCache=$$($1_AOT_JDK_CACHE) \
788-
))
788+
)
789789

790790
endif
791791

@@ -1085,9 +1085,9 @@ define SetupRunJtregTestBody
10851085
$$(call LogWarn, Running test '$$($1_TEST)')
10861086
$$(call MakeDir, $$($1_TEST_RESULTS_DIR) $$($1_TEST_SUPPORT_DIR) \
10871087
$$($1_TEST_TMP_DIR))
1088-
$$(call ExecuteWithLog, $$($1_TEST_SUPPORT_DIR)/jtreg, ( \
1088+
$$(call ExecuteWithLog, $$($1_TEST_SUPPORT_DIR)/jtreg, \
10891089
$$(COV_ENVIRONMENT) $$($1_COMMAND_LINE) \
1090-
))
1090+
)
10911091

10921092
$1_RESULT_FILE := $$($1_TEST_RESULTS_DIR)/text/stats.txt
10931093

@@ -1204,12 +1204,12 @@ define SetupRunSpecialTestBody
12041204
$$(call LogWarn)
12051205
$$(call LogWarn, Running test '$$($1_TEST)')
12061206
$$(call MakeDir, $$($1_TEST_RESULTS_DIR) $$($1_TEST_SUPPORT_DIR))
1207-
$$(call ExecuteWithLog, $$($1_TEST_SUPPORT_DIR)/test-execution, ( \
1207+
$$(call ExecuteWithLog, $$($1_TEST_SUPPORT_DIR)/test-execution, \
12081208
$$($1_TEST_COMMAND_LINE) \
12091209
> >($(TEE) $$($1_TEST_RESULTS_DIR)/test-output.txt) \
12101210
&& $$(ECHO) $$$$? > $$($1_EXITCODE) \
12111211
|| $$(ECHO) $$$$? > $$($1_EXITCODE) \
1212-
))
1212+
)
12131213

12141214
# We can not parse the various "special" tests.
12151215
parse-test-$1: run-test-$1

‎make/StaticLibs.gmk‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ else ifeq ($(call isTargetOs, aix), true)
111111
INFO := Generating export list for $(notdir $(lib)), \
112112
DEPS := $(lib), \
113113
OUTPUT_FILE := $(lib).exp, \
114-
COMMAND := ( $(AR) $(ARFLAGS) -w $(lib) | $(GREP) -v '^\.' | $(AWK) '{print $$1}' | $(SORT) -u > $(lib).exp ), \
114+
COMMAND := $(AR) $(ARFLAGS) -w $(lib) | $(GREP) -v '^\.' | $(AWK) '{print $$1}' | $(SORT) -u > $(lib).exp, \
115115
)) \
116116
$(eval STATIC_LIB_EXPORT_FILES += $(lib).exp) \
117117
)

‎make/common/MakeBase.gmk‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -284,28 +284,36 @@ else
284284
LogCmdlines =
285285
endif
286286

287+
# Check if the command line contains redirection, that is <, > or >>,
288+
# and if so, return a value that is interpreted as true in a make $(if)
289+
# construct.
290+
is_redirect = \
291+
$(if $(filter < > >>, $1), true)
292+
287293
################################################################################
288294
# ExecuteWithLog will run a command and log the output appropriately. This is
289295
# meant to be used by commands that do "real" work, like a compilation.
290296
# The output is stored in a specified log file, which is displayed at the end
291297
# of the build in case of failure. The command line itself is stored in a file,
292298
# and also logged to stdout if the LOG=cmdlines option has been given.
293299
#
294-
# NOTE: If the command redirects stdout, the caller needs to wrap it in a
295-
# subshell (by adding parentheses around it), otherwise the redirect to the
296-
# subshell tee process will create a race condition where the target file may
297-
# not be fully written when the make recipe is done.
298-
#
299300
# Param 1 - The path to base the name of the log file / command line file on
300301
# Param 2 - The command to run
301302
ExecuteWithLog = \
302-
$(call LogCmdlines, Executing: [$(strip $2)]) \
303+
$(call LogCmdlines, Executing: \
304+
[$(if $(call is_redirect, $2),$(LEFT_PAREN) )$(strip $2)$(if $(call \
305+
is_redirect, $2), $(RIGHT_PAREN))]) \
303306
$(call MakeDir, $(dir $(strip $1)) $(MAKESUPPORT_OUTPUTDIR)/failure-logs) \
304307
$(call WriteFile, $2, $(strip $1).cmdline) \
305-
( $(RM) $(strip $1).log && $(strip $2) > >($(TEE) -a $(strip $1).log) 2> >($(TEE) -a $(strip $1).log >&2) || \
308+
( $(RM) $(strip $1).log && \
309+
$(if $(call is_redirect, $2),$(LEFT_PAREN) )$(strip $2)$(if $(call \
310+
is_redirect, $2), $(RIGHT_PAREN)) \
311+
> >($(TEE) -a $(strip $1).log) 2> >($(TEE) -a $(strip $1).log >&2) || \
306312
( exitcode=$(DOLLAR)? && \
307-
$(CP) $(strip $1).log $(MAKESUPPORT_OUTPUTDIR)/failure-logs/$(subst /,_,$(patsubst $(OUTPUTDIR)/%,%,$(strip $1))).log && \
308-
$(CP) $(strip $1).cmdline $(MAKESUPPORT_OUTPUTDIR)/failure-logs/$(subst /,_,$(patsubst $(OUTPUTDIR)/%,%,$(strip $1))).cmdline && \
313+
$(CP) $(strip $1).log $(MAKESUPPORT_OUTPUTDIR)/failure-logs/$(subst \
314+
/,_,$(patsubst $(OUTPUTDIR)/%,%,$(strip $1))).log && \
315+
$(CP) $(strip $1).cmdline $(MAKESUPPORT_OUTPUTDIR)/failure-logs/$(subst \
316+
/,_,$(patsubst $(OUTPUTDIR)/%,%,$(strip $1))).cmdline && \
309317
exit $(DOLLAR)exitcode ) )
310318

311319
################################################################################

‎make/common/ProcessMarkdown.gmk‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ define ProcessMarkdown
109109
$$(call LogInfo, Post-processing markdown file $2)
110110
$$(call MakeDir, $$(SUPPORT_OUTPUTDIR)/markdown $$($1_$2_TARGET_DIR))
111111
$$(call ExecuteWithLog, $$(SUPPORT_OUTPUTDIR)/markdown/$$($1_$2_MARKER)_post, \
112-
( $$($1_POST_PROCESS) $$($1_$2_PANDOC_OUTPUT) > $$($1_$2_OUTPUT_FILE) ) )
112+
$$($1_POST_PROCESS) $$($1_$2_PANDOC_OUTPUT) > $$($1_$2_OUTPUT_FILE) )
113113
endif
114114

115115
$1 += $$($1_$2_OUTPUT_FILE)

‎make/hotspot/gensrc/GensrcDtrace.gmk‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,9 @@ ifeq ($(call check-jvm-feature, dtrace), true)
4747
$(call LogInfo, Generating dtrace header file $(@F))
4848
$(call MakeDir, $(@D) $(DTRACE_SUPPORT_DIR))
4949
$(call ExecuteWithLog, $(DTRACE_SUPPORT_DIR)/$(@F).d, \
50-
($(CPP) $(DTRACE_CPP_FLAGS) $(SYSROOT_CFLAGS) $< > $(DTRACE_SUPPORT_DIR)/$(@F).d))
51-
$(call ExecuteWithLog, $@, $(DTRACE) $(DTRACE_FLAGS) -h -o $@ -s $(DTRACE_SUPPORT_DIR)/$(@F).d)
50+
$(CPP) $(DTRACE_CPP_FLAGS) $(SYSROOT_CFLAGS) $< > $(DTRACE_SUPPORT_DIR)/$(@F).d)
51+
$(call ExecuteWithLog, $@, \
52+
$(DTRACE) $(DTRACE_FLAGS) -h -o $@ -s $(DTRACE_SUPPORT_DIR)/$(@F).d)
5253

5354
# Process all .d files in DTRACE_SOURCE_DIR. They are:
5455
# hotspot_jni.d hotspot.d hs_private.d

0 commit comments

Comments
 (0)