diff mbox series

genmultilib: Add sanity check

Message ID 20221102131004.3816486-1-christophe.lyon@arm.com
State Accepted
Headers
Series genmultilib: Add sanity check |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Christophe Lyon Nov. 2, 2022, 1:10 p.m. UTC 
  When a list of dirnames is provided to genmultilib, its length is
expected to match the number of options.  If this is not the case, the
build fails later for reasons not obviously related to this mistake.
This patch adds a sanity check to help diagnose such cases.

Tested by adding an option to t-aarch64 and no corresponding dirname,
with both bash and dash.

OK for trunk?

gcc/ChangeLog:

	* genmultilib: Add sanity check.
---
 gcc/genmultilib | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Joseph Myers Nov. 2, 2022, 5:29 p.m. UTC | #1 
On Wed, 2 Nov 2022, Christophe Lyon via Gcc-patches wrote:

> +# Sanity check: make sure we have as many dirnames as options
> +if [ -n "${dirnames}" ]; then
> +    options_arr=($options)

This is an sh script; arrays are a bash feature.  Building GCC isn't 
supposed to need bash (or to rely on $(SHELL) being bash, even when bash 
is available - many GNU/Linux systems use dash for /bin/sh), only a POSIX 
shell.
Christophe Lyon Nov. 2, 2022, 5:57 p.m. UTC | #2 
On 11/2/22 18:29, Joseph Myers wrote:
> On Wed, 2 Nov 2022, Christophe Lyon via Gcc-patches wrote:
> 
>> +# Sanity check: make sure we have as many dirnames as options
>> +if [ -n "${dirnames}" ]; then
>> +    options_arr=($options)
> 
> This is an sh script; arrays are a bash feature.  Building GCC isn't
> supposed to need bash (or to rely on $(SHELL) being bash, even when bash
> is available - many GNU/Linux systems use dash for /bin/sh), only a POSIX
> shell.
> 

That's what I feared, and I did "try to try" to build with dash, but I 
realize now that changing SHELL in the generated gcc/Makefile is not 
enough since it's defined by the higher level Makefile/config.status. 
Indeed rebuilding from scratch with CONFIG_SHELL=/bin/dash fails with my 
patch.

We have lived with that behavior for years, so it's not that bad anyway :-)

Thanks,

Christophe
diff mbox series

Patch

diff --git a/gcc/genmultilib b/gcc/genmultilib
index 1e387fb1589..ef121e77d17 100644
--- a/gcc/genmultilib
+++ b/gcc/genmultilib
@@ -141,6 +141,20 @@  multiarch=$9
 multilib_reuse=${10}
 enable_multilib=${11}
 
+# Sanity check: make sure we have as many dirnames as options
+if [ -n "${dirnames}" ]; then
+    options_arr=($options)
+    dirnames_arr=($dirnames)
+    nboptions=${#options_arr[@]}
+    nbdirnames=${#dirnames_arr[@]}
+    if [ $nbdirnames -ne $nboptions ]; then
+	echo 1>&2 "Error calling $0: Number of dirnames ($nbdirnames) does not match number of options ($nboptions)"
+	echo 1>&2 "options: ${options}"
+	echo 1>&2 "dirnames: ${dirnames}"
+	exit 1
+    fi
+fi
+
 echo "static const char *const multilib_raw[] = {"
 
 mkdir tmpmultilib.$$ || exit 1