From 4073526571266dd5c5d2a26ea5b648b53bed6507 Mon Sep 17 00:00:00 2001 From: David Freese Date: Wed, 9 Apr 2025 20:28:02 +0000 Subject: [PATCH 1/3] Create sanitized response file instead of expanding arguments The cc_wrapper.sh scripts inspects response files and sanitizes each line within them. How the logic is structured currently causes all of these arguments to the be directly used later in the script. This can run into argument overflows. It can also run into problems where quoting in response files and from args is treated differently. This addresses some of the comments in #430, namely cleaning up files and not overwriting the input files. I have not tried to apply a threshold here as that is best left up to the tooling that previously decided to use response files, which can cause difficult to debug changes in behavior. This conflicts with #479, but is trying to address the same underlying problem of quoted arguments in response files as the result of golang's link actions. Fixes #421 --- toolchain/cc_wrapper.sh.tpl | 26 ++++++++++++++++++++------ toolchain/osx_cc_wrapper.sh.tpl | 20 +++++++++++++++----- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/toolchain/cc_wrapper.sh.tpl b/toolchain/cc_wrapper.sh.tpl index e972d57e6..0f1067d39 100644 --- a/toolchain/cc_wrapper.sh.tpl +++ b/toolchain/cc_wrapper.sh.tpl @@ -29,6 +29,16 @@ set -euo pipefail +CLEANUP_FILES=() + +function cleanup() { + if [[ ${#CLEANUP_FILES[@]} -gt 0 ]]; then + rm -f "${CLEANUP_FILES[@]}" + fi +} + +trap cleanup EXIT + # See note in toolchain/internal/configure.bzl where we define # `wrapper_bin_prefix` for why this wrapper is needed. @@ -59,15 +69,19 @@ function sanitize_option() { } cmd=() +tmpfiles=() for ((i = 0; i <= $#; i++)); do - if [[ ${!i} == @* ]]; then + if [[ ${!i} == @* && -r "${i:1}" ]]; then + # Create a new, sanitized file. + tmpfile=$(mktemp) + CLEANUP_FILES+=("${tmpfile}") while IFS= read -r opt; do - opt="$( + echo "$( set -e sanitize_option "${opt}" - )" - cmd+=("${opt}") - done <"${!i:1}" + )" >> "${tmpfile}" + done <"${!i:1}" + cmd+=("@${tmpfile}") else opt="$( set -e @@ -78,4 +92,4 @@ for ((i = 0; i <= $#; i++)); do done # Call the C++ compiler. -exec "${cmd[@]}" +"${cmd[@]}" diff --git a/toolchain/osx_cc_wrapper.sh.tpl b/toolchain/osx_cc_wrapper.sh.tpl index 31122784b..285bb2d10 100755 --- a/toolchain/osx_cc_wrapper.sh.tpl +++ b/toolchain/osx_cc_wrapper.sh.tpl @@ -35,6 +35,15 @@ LIBS= LIB_DIRS= RPATHS= OUTPUT= +CLEANUP_FILES=() + +function cleanup() { + if [[ ${#CLEANUP_FILES[@]} -gt 0 ]]; then + rm -f "${CLEANUP_FILES[@]}" + fi +} + +trap cleanup EXIT function parse_option() { local -r opt="$1" @@ -87,17 +96,18 @@ function sanitize_option() { cmd=() for ((i = 0; i <= $#; i++)); do if [[ ${!i} == @* && -r "${i:1}" ]]; then + tmpfile=$(mktemp) + CLEANUP_FILES+=("${tmpfile}") while IFS= read -r opt; do if [[ ${opt} == "-fuse-ld=ld64.lld" ]]; then - cmd+=("-fuse-ld=lld") + echo "-fuse-ld=lld" >> ${tmpfile} fi - opt="$( + parse_option "$( set -e sanitize_option "${opt}" - )" - parse_option "${opt}" - cmd+=("${opt}") + )" >> ${tmpfile} done <"${!i:1}" + cmd+=("@${tmpfile}") else opt="$( set -e From 0d1ef10e3e2bbe9cd93545276d8c3b5e78491065 Mon Sep 17 00:00:00 2001 From: David Freese Date: Wed, 9 Apr 2025 21:58:10 +0000 Subject: [PATCH 2/3] remove unused tmpfiles variable and format --- toolchain/cc_wrapper.sh.tpl | 5 ++--- toolchain/osx_cc_wrapper.sh.tpl | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/toolchain/cc_wrapper.sh.tpl b/toolchain/cc_wrapper.sh.tpl index 0f1067d39..7acc14580 100644 --- a/toolchain/cc_wrapper.sh.tpl +++ b/toolchain/cc_wrapper.sh.tpl @@ -69,7 +69,6 @@ function sanitize_option() { } cmd=() -tmpfiles=() for ((i = 0; i <= $#; i++)); do if [[ ${!i} == @* && -r "${i:1}" ]]; then # Create a new, sanitized file. @@ -79,8 +78,8 @@ for ((i = 0; i <= $#; i++)); do echo "$( set -e sanitize_option "${opt}" - )" >> "${tmpfile}" - done <"${!i:1}" + )" >>"${tmpfile}" + done <"${!i:1}" cmd+=("@${tmpfile}") else opt="$( diff --git a/toolchain/osx_cc_wrapper.sh.tpl b/toolchain/osx_cc_wrapper.sh.tpl index 285bb2d10..828f0b202 100755 --- a/toolchain/osx_cc_wrapper.sh.tpl +++ b/toolchain/osx_cc_wrapper.sh.tpl @@ -100,12 +100,12 @@ for ((i = 0; i <= $#; i++)); do CLEANUP_FILES+=("${tmpfile}") while IFS= read -r opt; do if [[ ${opt} == "-fuse-ld=ld64.lld" ]]; then - echo "-fuse-ld=lld" >> ${tmpfile} + echo "-fuse-ld=lld" >>${tmpfile} fi parse_option "$( set -e sanitize_option "${opt}" - )" >> ${tmpfile} + )" >>${tmpfile} done <"${!i:1}" cmd+=("@${tmpfile}") else From 4e2cca3eebe13c14025fdf826559b18e8a04f495 Mon Sep 17 00:00:00 2001 From: David Freese Date: Thu, 10 Apr 2025 11:32:49 +0000 Subject: [PATCH 3/3] fix quoting and shellcheck errors --- toolchain/cc_wrapper.sh.tpl | 5 +++-- toolchain/osx_cc_wrapper.sh.tpl | 7 ++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/toolchain/cc_wrapper.sh.tpl b/toolchain/cc_wrapper.sh.tpl index 7acc14580..b3cb23386 100644 --- a/toolchain/cc_wrapper.sh.tpl +++ b/toolchain/cc_wrapper.sh.tpl @@ -75,10 +75,11 @@ for ((i = 0; i <= $#; i++)); do tmpfile=$(mktemp) CLEANUP_FILES+=("${tmpfile}") while IFS= read -r opt; do - echo "$( + opt="$( set -e sanitize_option "${opt}" - )" >>"${tmpfile}" + )" + echo "${opt}" >>"${tmpfile}" done <"${!i:1}" cmd+=("@${tmpfile}") else diff --git a/toolchain/osx_cc_wrapper.sh.tpl b/toolchain/osx_cc_wrapper.sh.tpl index 828f0b202..a005caa30 100755 --- a/toolchain/osx_cc_wrapper.sh.tpl +++ b/toolchain/osx_cc_wrapper.sh.tpl @@ -100,12 +100,13 @@ for ((i = 0; i <= $#; i++)); do CLEANUP_FILES+=("${tmpfile}") while IFS= read -r opt; do if [[ ${opt} == "-fuse-ld=ld64.lld" ]]; then - echo "-fuse-ld=lld" >>${tmpfile} + echo "-fuse-ld=lld" >>"${tmpfile}" fi - parse_option "$( + opt="$( set -e sanitize_option "${opt}" - )" >>${tmpfile} + )" + parse_option "${opt}" >>"${tmpfile}" done <"${!i:1}" cmd+=("@${tmpfile}") else