[Darwin,Driver,committed] Improve processing of macosx-version-min=

Message ID 84A3B6F4-E7B6-4CAC-B67B-7D08C2412728@sandoe.co.uk
State New
Headers show
Series
  • [Darwin,Driver,committed] Improve processing of macosx-version-min=
Related show

Commit Message

Iain Sandoe June 13, 2019, 7:05 p.m.
For PR target/63810 some improvements were made in the parsing of the version string
at the point it's used to define the built-in
__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__.  
This is fine, but the specs processing also uses the version, and specs version-compare
doesn't like leading zeros on components.  This means that while we succeed in
processing -mmacosx-version-min=010.00002.000099 on compile lines, it fails for any
 other line that uses the value as part of a spec (in particular, link lines fail).

To fix this, we need to apply a bit of clean-up to the version that’s presented to the driver,
and push that back into the command line opts.

The value can come from four places:
 1. User-entered on the command line
 2. User-entered as MACOSX_DEPLOYMENT_TARGET= environment var.
 3. Absent those two
   3a For self-hosting systems, look-up from the kernel
   3b For cross-compilers, as a default supplied at configure time.

We apply the clean-up to all 4 (although it shouldn't really be needed
for the cases under 3).

We also supply a test-case that adapts to the target-version of the
system, so that the link requirements are met by the SDK in use (if you
try to link i686-darwin9 on an x86-64-darwin18 SDK, it will fail).

tested across a range of Darwin boxes, 
applied to mainline,

thanks
Iain

gcc/
2019-06-13  Iain Sandoe  <iain@sandoe.co.uk>

	* config/darwin-driver.c (validate_macosx_version_min): New.
	(darwin_default_min_version): Cleanup and validate supplied version.
	(darwin_driver_init): Likewise and push cleaned version into opts.

gcc/testsuite

2019-06-13  Iain Sandoe  <iain@sandoe.co.uk>

	* gcc.dg/darwin-minversion-link.c: New test.

Patch

diff --git a/gcc/config/darwin-driver.c b/gcc/config/darwin-driver.c
index 6a7c859..01238e2 100644
--- a/gcc/config/darwin-driver.c
+++ b/gcc/config/darwin-driver.c
@@ -26,6 +26,91 @@  along with GCC; see the file COPYING3.  If not see
 #include "opts.h"
 #include "diagnostic-core.h"
 
+/* Validate a version string (either given on the command line or, perhaps
+   as MACOSX_DEPLOYMENT_TARGET).
+
+   The specs %version-compare() function doesn't accept leading '0' on
+   numbers so strip them out.  Do sanity checking here too.
+
+   Return:
+     * original string means it was OK and we didn't want to change it.
+     * new string means it was OK but we rewrote it to avoid possible format
+     problems.
+     * NULL means we didn't like what we saw.
+*/
+
+static const char *
+validate_macosx_version_min (const char *version_str)
+{
+  size_t version_len;
+  unsigned long major, minor, tiny = 0;
+  char *end;
+  const char *old_version = version_str;
+  bool need_rewrite = false;
+
+  version_len = strlen (version_str);
+  if (version_len < 4) /* The minimum would be 10.x  */
+    return NULL;
+
+  /* Version string must consist of digits and periods only.  */
+  if (strspn (version_str, "0123456789.") != version_len)
+    return NULL;
+
+  if (!ISDIGIT (version_str[0]) || !ISDIGIT (version_str[version_len - 1]))
+    return NULL;
+
+  if (version_str[0] == '0')
+    need_rewrite = true;
+
+  major = strtoul (version_str, &end, 10);
+  version_str = end + ((*end == '.') ? 1 : 0);
+
+  if (major != 10) /* So far .. all MacOS 10 ... */
+    return NULL;
+
+  /* Version string components must be present and numeric.  */
+  if (!ISDIGIT (version_str[0]))
+    return NULL;
+
+  /* If we have one or more leading zeros on a component, then rewrite the
+     version string.  */
+  if (version_str[0] == '0' && version_str[1] != '\0'
+      && version_str[1] != '.')
+    need_rewrite = true;
+
+  minor = strtoul (version_str, &end, 10);
+  version_str = end + ((*end == '.') ? 1 : 0);
+  if (minor > 99)
+    return NULL;
+
+  /* If 'tiny' is present it must be numeric.  */
+  if (*end != '\0' && !ISDIGIT (version_str[0]))
+    return NULL;
+
+  /* If we have one or more leading zeros on a component, then rewrite the
+     version string.  */
+  if (*end != '\0' && version_str[0] == '0'
+      && version_str[1] != '\0')
+    need_rewrite = true;
+
+  tiny = strtoul (version_str, &end, 10);
+  if (tiny > 99)
+    return NULL;
+
+  /* Version string must contain no more than three tokens.  */
+  if (*end != '\0')
+    return NULL;
+
+  if (need_rewrite)
+    {
+      char *new_version;
+      asprintf (&new_version, "10.%lu.%lu", minor, tiny);
+      return new_version;
+    }
+
+  return old_version;
+}
+
 #ifndef CROSS_DIRECTORY_STRUCTURE
 #include <sys/sysctl.h>
 #include "xregex.h"
@@ -114,12 +199,13 @@  darwin_default_min_version (void)
 
   if (new_flag != NULL)
     {
-      size_t len = strlen (new_flag);
-      if (len > 128) { /* Arbitrary limit, number should be like xx.yy.zz */
-	warning (0, "couldn%'t understand version %s\n", new_flag);
-	return NULL;
-      }
-      new_flag = xstrndup (new_flag, len);
+      const char *checked = validate_macosx_version_min (new_flag);
+      if (checked == NULL)
+	{
+	  warning (0, "couldn%'t understand version %s\n", new_flag);
+	  return NULL;
+	}
+      new_flag = xstrndup (checked, strlen (checked));
     }
   return new_flag;
 }
@@ -209,7 +295,24 @@  darwin_driver_init (unsigned int *decoded_options_count,
 
 	case OPT_mmacosx_version_min_:
 	  seen_version_min = true;
-	  vers_string = xstrndup ((*decoded_options)[i].arg, 32);
+	  vers_string =
+	    validate_macosx_version_min ((*decoded_options)[i].arg);
+	  if (vers_string == NULL)
+	    warning (0, "%qs is not valid for %<mmacosx-version-min%>\n",
+		     (*decoded_options)[i].arg);
+	  else if (vers_string == (*decoded_options)[i].arg)
+	    vers_string = xstrndup ((*decoded_options)[i].arg, 32);
+	  /* Now we've examined it, and verified/re-written, put it to
+	     one side and append later.  */
+	  if (*decoded_options_count > i) {
+	    memmove (*decoded_options + i,
+		     *decoded_options + i + 1,
+		     ((*decoded_options_count - i)
+		      * sizeof (struct cl_decoded_option)));
+	  }
+	  --i;
+	  --*decoded_options_count;
+	  break;
 
 	default:
 	  break;
@@ -276,22 +379,20 @@  darwin_driver_init (unsigned int *decoded_options_count,
      so that we can figure out the mechanism and source for the sysroot to
      be used.  */
   if (! seen_version_min && *decoded_options_count > 1)
-    {
-      /* Not set by the User, try to figure it out.  */
-      vers_string = darwin_default_min_version ();
-      if (vers_string != NULL)
-	{
-	  ++*decoded_options_count;
-	  *decoded_options = XRESIZEVEC (struct cl_decoded_option,
-					 *decoded_options,
-					 *decoded_options_count);
-	  generate_option (OPT_mmacosx_version_min_, vers_string, 1, CL_DRIVER,
-			  &(*decoded_options)[*decoded_options_count - 1]);
-	}
-    }
-  /* Create and push the major version for assemblers that need it.  */
+    /* Not set by the User, try to figure it out.  */
+    vers_string = darwin_default_min_version ();
+
+  /* Create and push a cleaned up version, plus the major version for
+     assemblers and other cases that need it.  */
   if (vers_string != NULL)
     {
+       ++*decoded_options_count;
+       *decoded_options = XRESIZEVEC (struct cl_decoded_option,
+				      *decoded_options,
+				      *decoded_options_count);
+      generate_option (OPT_mmacosx_version_min_, vers_string, 1, CL_DRIVER,
+		       &(*decoded_options)[*decoded_options_count - 1]);
+
       char *asm_major = NULL;
       const char *first_period = strchr(vers_string, '.');
       if (first_period != NULL)

diff --git a/gcc/testsuite/gcc.dg/darwin-minversion-link.c b/gcc/testsuite/gcc.dg/darwin-minversion-link.c
new file mode 100644
index 0000000..0a80048
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/darwin-minversion-link.c
@@ -0,0 +1,26 @@ 
+/* Test that we can handle leading-zeros on mmacosx-version-min for invocations
+   including linking (so that spec processing works).  To make sure that any
+   necessary target libs are present we make this specific to the target version
+   being built.  */
+/* { dg-do link { target *-*-darwin* } } */
+/* { dg-additional-options "-mmacosx-version-min=010.04.11 -DCHECK=1049" { target *-*-darwin8* } } */
+/* { dg-additional-options "-mmacosx-version-min=010.05.08 -DCHECK=1058" { target *-*-darwin9* } } */
+/* { dg-additional-options "-mmacosx-version-min=010.06.08 -DCHECK=1068" { target *-*-darwin10* } } */
+/* { dg-additional-options "-mmacosx-version-min=010.07.05 -DCHECK=1075" { target *-*-darwin11* } } */
+/* { dg-additional-options "-mmacosx-version-min=010.08.05 -DCHECK=1085" { target *-*-darwin12* } } */
+/* { dg-additional-options "-mmacosx-version-min=010.09.05 -DCHECK=1095" { target *-*-darwin13* } } */
+/* { dg-additional-options "-mmacosx-version-min=010.010.03 -DCHECK=101003" { target *-*-darwin14* } } */
+/* { dg-additional-options "-mmacosx-version-min=010.011.06 -DCHECK=101106" { target *-*-darwin15* } } */
+/* { dg-additional-options "-mmacosx-version-min=010.012.06 -DCHECK=101206" { target *-*-darwin16* } } */
+/* { dg-additional-options "-mmacosx-version-min=010.013.06 -DCHECK=101306" { target *-*-darwin17* } } */
+/* This next test covers 10.18 and (currently unreleased) 10.19 for now. */  
+/* { dg-additional-options "-mmacosx-version-min=010.014.05 -DCHECK=101405" { target *-*-darwin1[89]* } } */
+
+int
+main ()
+{
+#if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ != CHECK
+  fail me;
+#endif
+  return 0;
+}