[PATCH v2] ld: Make library member file suffix comparisons case insensitive when cross compiling too

[PATCH v2] ld: Make library member file suffix comparisons case insensitive when cross compiling too

[Martin Storsjö]

On Windows, filename_cmp is case insensitive, but when cross compiling
with similar libraries that may contain members with uppercase file
names, we should keep those comparisons case insensitive when running
the build tools on other OSes too.
---
v2: Fix a missed == 0 in pep.em, which was accidentally removed in
the first version of the patch.
---
 ld/emultempl/pe.em  | 26 ++++++++++++++++++++++----
 ld/emultempl/pep.em | 25 +++++++++++++++++++++----
 2 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/ld/emultempl/pe.em b/ld/emultempl/pe.em
index 2fd4ff4acaf..59bda7da968 100644
--- a/ld/emultempl/pe.em
+++ b/ld/emultempl/pe.em
@@ -171,6 +171,24 @@ static int is_underscoring (void)
   return pe_leading_underscore;
 }
 
+/* Hardcoded case insensitive comparison. filename_cmp is insensitive
+ * when running on Windows, but when cross compiling to Windows, we
+ * also want similar comparisons to be case insensitive. */
+static int stricmp (const char *s1, const char *s2)
+{
+  for (;;)
+    {
+      int c1 = TOLOWER (*s1++);
+      int c2 = TOLOWER (*s2++);
+
+      if (c1 != c2)
+        return (c1 - c2);
+
+      if (c1 == '\0')
+        return 0;
+    }
+}
+
 static void
 gld${EMULATION_NAME}_before_parse (void)
 {
@@ -1660,7 +1678,7 @@ gld${EMULATION_NAME}_after_open (void)
 		       extension, and use that for the remainder of the
 		       comparisons.  */
 		    pnt = strrchr (bfd_get_filename (is3->the_bfd), '.');
-		    if (pnt != NULL && filename_cmp (pnt, ".dll") == 0)
+		    if (pnt != NULL && stricmp (pnt, ".dll") == 0)
 		      break;
 		  }
 
@@ -1677,7 +1695,7 @@ gld${EMULATION_NAME}_after_open (void)
 			/* Skip static members, ie anything with a .obj
 			   extension.  */
 			pnt = strrchr (bfd_get_filename (is2->the_bfd), '.');
-			if (pnt != NULL && filename_cmp (pnt, ".obj") == 0)
+			if (pnt != NULL && stricmp (pnt, ".obj") == 0)
 			  continue;
 
 			if (filename_cmp (bfd_get_filename (is3->the_bfd),
@@ -1695,7 +1713,7 @@ gld${EMULATION_NAME}_after_open (void)
 	       then leave the filename alone.  */
 	    pnt = strrchr (bfd_get_filename (is->the_bfd), '.');
 
-	    if (is_ms_arch && (filename_cmp (pnt, ".dll") == 0))
+	    if (is_ms_arch && (stricmp (pnt, ".dll") == 0))
 	      {
 		int idata2 = 0, reloc_count=0;
 		asection *sec;
@@ -1854,7 +1872,7 @@ gld${EMULATION_NAME}_unrecognized_file (lang_input_statement_type *entry ATTRIBU
 #ifdef DLL_SUPPORT
   const char *ext = entry->filename + strlen (entry->filename) - 4;
 
-  if (filename_cmp (ext, ".def") == 0 || filename_cmp (ext, ".DEF") == 0)
+  if (stricmp (ext, ".def") == 0)
     {
       pe_def_file = def_file_parse (entry->filename, pe_def_file);
 
diff --git a/ld/emultempl/pep.em b/ld/emultempl/pep.em
index e68d1e69f17..0c2f6159ac9 100644
--- a/ld/emultempl/pep.em
+++ b/ld/emultempl/pep.em
@@ -181,6 +181,23 @@ static int is_underscoring (void)
   return pep_leading_underscore;
 }
 
+/* Hardcoded case insensitive comparison. filename_cmp is insensitive
+ * when running on Windows, but when cross compiling to Windows, we
+ * also want similar comparisons to be case insensitive. */
+static int stricmp (const char *s1, const char *s2)
+{
+  for (;;)
+    {
+      int c1 = TOLOWER (*s1++);
+      int c2 = TOLOWER (*s2++);
+
+      if (c1 != c2)
+        return (c1 - c2);
+
+      if (c1 == '\0')
+        return 0;
+    }
+}
 
 static void
 gld${EMULATION_NAME}_before_parse (void)
@@ -1624,7 +1641,7 @@ gld${EMULATION_NAME}_after_open (void)
 		       extension, and use that for the remainder of the
 		       comparisons.  */
 		    pnt = strrchr (bfd_get_filename (is3->the_bfd), '.');
-		    if (pnt != NULL && filename_cmp (pnt, ".dll") == 0)
+		    if (pnt != NULL && stricmp (pnt, ".dll") == 0)
 		      break;
 		  }
 
@@ -1641,7 +1658,7 @@ gld${EMULATION_NAME}_after_open (void)
 			/* Skip static members, ie anything with a .obj
 			   extension.  */
 			pnt = strrchr (bfd_get_filename (is2->the_bfd), '.');
-			if (pnt != NULL && filename_cmp (pnt, ".obj") == 0)
+			if (pnt != NULL && stricmp (pnt, ".obj") == 0)
 			  continue;
 
 			if (filename_cmp (bfd_get_filename (is3->the_bfd),
@@ -1659,7 +1676,7 @@ gld${EMULATION_NAME}_after_open (void)
 	       then leave the filename alone.  */
 	    pnt = strrchr (bfd_get_filename (is->the_bfd), '.');
 
-	    if (is_ms_arch && (filename_cmp (pnt, ".dll") == 0))
+	    if (is_ms_arch && (stricmp (pnt, ".dll") == 0))
 	      {
 		int idata2 = 0, reloc_count=0;
 		asection *sec;
@@ -1724,7 +1741,7 @@ gld${EMULATION_NAME}_unrecognized_file (lang_input_statement_type *entry ATTRIBU
 #ifdef DLL_SUPPORT
   const char *ext = entry->filename + strlen (entry->filename) - 4;
 
-  if (filename_cmp (ext, ".def") == 0 || filename_cmp (ext, ".DEF") == 0)
+  if (stricmp (ext, ".def") == 0)
     {
       pep_def_file = def_file_parse (entry->filename, pep_def_file);
 
-- 
2.25.1

Re: [PATCH v2] ld: Make library member file suffix comparisons case insensitive when cross compiling too

[Nick Clifton]

Hi Martin,

> +/* Hardcoded case insensitive comparison. filename_cmp is insensitive
> + * when running on Windows, but when cross compiling to Windows, we
> + * also want similar comparisons to be case insensitive. */
> +static int stricmp (const char *s1, const char *s2)
> +{
> +  for (;;)
> +    {
> +      int c1 = TOLOWER (*s1++);
> +      int c2 = TOLOWER (*s2++);
> +
> +      if (c1 != c2)
> +        return (c1 - c2);
> +
> +      if (c1 == '\0')
> +        return 0;
> +    }
> +}

The implementation of filename_cmp() in libiberty also treats forward
slashes and backward slashes as the same, on DOS based filesystems.
Shouldn't this be carried over to stricmp() as well ?

Cheers
   Nick

Re: [PATCH v2] ld: Make library member file suffix comparisons case insensitive when cross compiling too

[Martin Storsjö]

On Tue, 23 Aug 2022, Nick Clifton wrote:

> Hi Martin,
>
>> +/* Hardcoded case insensitive comparison. filename_cmp is insensitive
>> + * when running on Windows, but when cross compiling to Windows, we
>> + * also want similar comparisons to be case insensitive. */
>> +static int stricmp (const char *s1, const char *s2)
>> +{
>> +  for (;;)
>> +    {
>> +      int c1 = TOLOWER (*s1++);
>> +      int c2 = TOLOWER (*s2++);
>> +
>> +      if (c1 != c2)
>> +        return (c1 - c2);
>> +
>> +      if (c1 == '\0')
>> +        return 0;
>> +    }
>> +}
>
> The implementation of filename_cmp() in libiberty also treats forward
> slashes and backward slashes as the same, on DOS based filesystems.
> Shouldn't this be carried over to stricmp() as well ?

Oh, right - in general I guess that could be useful too, but all the 
occurrances I switched to use this only ever compared against filename 
suffixes, no full pathnames. So for the usecases changed here, stricmp as 
such is enough - but I guess I can extend it into a filename_dos_cmp 
instead, if that's preferred.

// Martin

Re: [PATCH v2] ld: Make library member file suffix comparisons case insensitive when cross compiling too

[Jan Beulich]

On 23.08.2022 16:01, Nick Clifton via Binutils wrote:
>> +/* Hardcoded case insensitive comparison. filename_cmp is insensitive
>> + * when running on Windows, but when cross compiling to Windows, we
>> + * also want similar comparisons to be case insensitive. */
>> +static int stricmp (const char *s1, const char *s2)
>> +{
>> +  for (;;)
>> +    {
>> +      int c1 = TOLOWER (*s1++);
>> +      int c2 = TOLOWER (*s2++);
>> +
>> +      if (c1 != c2)
>> +        return (c1 - c2);
>> +
>> +      if (c1 == '\0')
>> +        return 0;
>> +    }
>> +}
> 
> The implementation of filename_cmp() in libiberty also treats forward
> slashes and backward slashes as the same, on DOS based filesystems.
> Shouldn't this be carried over to stricmp() as well ?

Well, the function is used on filename extensions (.dll and alike) only,
so that shouldn't be necessary. But I think the function would want to
have a name expressing this very limited purpose, and the comment ahead
of it would also want to call this out.

Jan

[PATCH v3] ld: Make archive member file extension comparisons case insensitive when cross compiling too

[Martin Storsjö]

On Windows, filename_cmp is case insensitive, but when cross compiling
with libraries that may contain members with uppercase file names, we
should keep those comparisons case insensitive when running the build
tools on other OSes too.
---
v3: Renamed the function to fileext_cmp as suggested. (This also fixes
building on Windows, where there already existed a non-static
function named stricmp.)
---
 ld/emultempl/pe.em  | 25 +++++++++++++++++++++----
 ld/emultempl/pep.em | 24 ++++++++++++++++++++----
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/ld/emultempl/pe.em b/ld/emultempl/pe.em
index ad969ccec13..c34ddebd489 100644
--- a/ld/emultempl/pe.em
+++ b/ld/emultempl/pe.em
@@ -171,6 +171,23 @@ static int is_underscoring (void)
   return pe_leading_underscore;
 }
 
+/* A case insensitive comparison, regardless of the host platform,
+   used for comparing file extensions. */
+static int fileext_cmp (const char *s1, const char *s2)
+{
+  for (;;)
+    {
+      int c1 = TOLOWER (*s1++);
+      int c2 = TOLOWER (*s2++);
+
+      if (c1 != c2)
+        return (c1 - c2);
+
+      if (c1 == '\0')
+        return 0;
+    }
+}
+
 static void
 gld${EMULATION_NAME}_before_parse (void)
 {
@@ -1666,7 +1683,7 @@ gld${EMULATION_NAME}_after_open (void)
 		       extension, and use that for the remainder of the
 		       comparisons.  */
 		    pnt = strrchr (bfd_get_filename (is3->the_bfd), '.');
-		    if (pnt != NULL && filename_cmp (pnt, ".dll") == 0)
+		    if (pnt != NULL && fileext_cmp (pnt, ".dll") == 0)
 		      break;
 		  }
 
@@ -1683,7 +1700,7 @@ gld${EMULATION_NAME}_after_open (void)
 			/* Skip static members, ie anything with a .obj
 			   extension.  */
 			pnt = strrchr (bfd_get_filename (is2->the_bfd), '.');
-			if (pnt != NULL && filename_cmp (pnt, ".obj") == 0)
+			if (pnt != NULL && fileext_cmp (pnt, ".obj") == 0)
 			  continue;
 
 			if (filename_cmp (bfd_get_filename (is3->the_bfd),
@@ -1701,7 +1718,7 @@ gld${EMULATION_NAME}_after_open (void)
 	       then leave the filename alone.  */
 	    pnt = strrchr (bfd_get_filename (is->the_bfd), '.');
 
-	    if (is_ms_arch && (filename_cmp (pnt, ".dll") == 0))
+	    if (is_ms_arch && (fileext_cmp (pnt, ".dll") == 0))
 	      {
 		int idata2 = 0, reloc_count=0;
 		asection *sec;
@@ -1857,7 +1874,7 @@ gld${EMULATION_NAME}_unrecognized_file (lang_input_statement_type *entry ATTRIBU
 #ifdef DLL_SUPPORT
   const char *ext = entry->filename + strlen (entry->filename) - 4;
 
-  if (filename_cmp (ext, ".def") == 0 || filename_cmp (ext, ".DEF") == 0)
+  if (fileext_cmp (ext, ".def") == 0)
     {
       pe_def_file = def_file_parse (entry->filename, pe_def_file);
 
diff --git a/ld/emultempl/pep.em b/ld/emultempl/pep.em
index ee36a9a7e56..ba4fc01ed70 100644
--- a/ld/emultempl/pep.em
+++ b/ld/emultempl/pep.em
@@ -181,6 +181,22 @@ static int is_underscoring (void)
   return pep_leading_underscore;
 }
 
+/* A case insensitive comparison, regardless of the host platform,
+   used for comparing file extensions. */
+static int fileext_cmp (const char *s1, const char *s2)
+{
+  for (;;)
+    {
+      int c1 = TOLOWER (*s1++);
+      int c2 = TOLOWER (*s2++);
+
+      if (c1 != c2)
+        return (c1 - c2);
+
+      if (c1 == '\0')
+        return 0;
+    }
+}
 
 static void
 gld${EMULATION_NAME}_before_parse (void)
@@ -1630,7 +1646,7 @@ gld${EMULATION_NAME}_after_open (void)
 		       extension, and use that for the remainder of the
 		       comparisons.  */
 		    pnt = strrchr (bfd_get_filename (is3->the_bfd), '.');
-		    if (pnt != NULL && filename_cmp (pnt, ".dll") == 0)
+		    if (pnt != NULL && fileext_cmp (pnt, ".dll") == 0)
 		      break;
 		  }
 
@@ -1647,7 +1663,7 @@ gld${EMULATION_NAME}_after_open (void)
 			/* Skip static members, ie anything with a .obj
 			   extension.  */
 			pnt = strrchr (bfd_get_filename (is2->the_bfd), '.');
-			if (pnt != NULL && filename_cmp (pnt, ".obj") == 0)
+			if (pnt != NULL && fileext_cmp (pnt, ".obj") == 0)
 			  continue;
 
 			if (filename_cmp (bfd_get_filename (is3->the_bfd),
@@ -1665,7 +1681,7 @@ gld${EMULATION_NAME}_after_open (void)
 	       then leave the filename alone.  */
 	    pnt = strrchr (bfd_get_filename (is->the_bfd), '.');
 
-	    if (is_ms_arch && (filename_cmp (pnt, ".dll") == 0))
+	    if (is_ms_arch && (fileext_cmp (pnt, ".dll") == 0))
 	      {
 		int idata2 = 0, reloc_count=0;
 		asection *sec;
@@ -1727,7 +1743,7 @@ gld${EMULATION_NAME}_unrecognized_file (lang_input_statement_type *entry ATTRIBU
 #ifdef DLL_SUPPORT
   const char *ext = entry->filename + strlen (entry->filename) - 4;
 
-  if (filename_cmp (ext, ".def") == 0 || filename_cmp (ext, ".DEF") == 0)
+  if (fileext_cmp (ext, ".def") == 0)
     {
       pep_def_file = def_file_parse (entry->filename, pep_def_file);
 
-- 
2.25.1

Re: [PATCH v3] ld: Make archive member file extension comparisons case insensitive when cross compiling too

[Jan Beulich]

On 23.08.2022 23:11, Martin Storsjö wrote:
> On Windows, filename_cmp is case insensitive, but when cross compiling
> with libraries that may contain members with uppercase file names, we
> should keep those comparisons case insensitive when running the build
> tools on other OSes too.
> ---
> v3: Renamed the function to fileext_cmp as suggested. (This also fixes
> building on Windows, where there already existed a non-static
> function named stricmp.)

I was actually surprised you did get away with using the earlier name;
I did have the potential name collision in mind, but didn't spell it
out because there was an (imo) more relevant reason for the rename.

> --- a/ld/emultempl/pe.em
> +++ b/ld/emultempl/pe.em
> @@ -171,6 +171,23 @@ static int is_underscoring (void)
>    return pe_leading_underscore;
>  }
>  
> +/* A case insensitive comparison, regardless of the host platform,
> +   used for comparing file extensions. */
> +static int fileext_cmp (const char *s1, const char *s2)
> +{
> +  for (;;)
> +    {
> +      int c1 = TOLOWER (*s1++);
> +      int c2 = TOLOWER (*s2++);
> +
> +      if (c1 != c2)
> +        return (c1 - c2);
> +
> +      if (c1 == '\0')
> +        return 0;
> +    }
> +}

I'm not going to insist, but since you're touching all involved call
sites anyway, two suggestions:

1) Don't convert the 2nd string to lower case, but instead require
that this always be lower case when passed in. The function isn't a
generic string comparison one anymore with the name change, after
all.

2) Omit the leading . in the string literals passed, incrementing the
first argument to compensate. Besides saving a little bit of space
and work, the function name now also carries in its name what is
being done, so the leading .-s don't really carry that much extra
information to the reader anymore.

Jan

> @@ -1666,7 +1683,7 @@ gld${EMULATION_NAME}_after_open (void)
>  		       extension, and use that for the remainder of the
>  		       comparisons.  */
>  		    pnt = strrchr (bfd_get_filename (is3->the_bfd), '.');
> -		    if (pnt != NULL && filename_cmp (pnt, ".dll") == 0)
> +		    if (pnt != NULL && fileext_cmp (pnt, ".dll") == 0)
>  		      break;
>  		  }
>  
> @@ -1683,7 +1700,7 @@ gld${EMULATION_NAME}_after_open (void)
>  			/* Skip static members, ie anything with a .obj
>  			   extension.  */
>  			pnt = strrchr (bfd_get_filename (is2->the_bfd), '.');
> -			if (pnt != NULL && filename_cmp (pnt, ".obj") == 0)
> +			if (pnt != NULL && fileext_cmp (pnt, ".obj") == 0)
>  			  continue;
>  
>  			if (filename_cmp (bfd_get_filename (is3->the_bfd),
> @@ -1701,7 +1718,7 @@ gld${EMULATION_NAME}_after_open (void)
>  	       then leave the filename alone.  */
>  	    pnt = strrchr (bfd_get_filename (is->the_bfd), '.');
>  
> -	    if (is_ms_arch && (filename_cmp (pnt, ".dll") == 0))
> +	    if (is_ms_arch && (fileext_cmp (pnt, ".dll") == 0))
>  	      {
>  		int idata2 = 0, reloc_count=0;
>  		asection *sec;
> @@ -1857,7 +1874,7 @@ gld${EMULATION_NAME}_unrecognized_file (lang_input_statement_type *entry ATTRIBU
>  #ifdef DLL_SUPPORT
>    const char *ext = entry->filename + strlen (entry->filename) - 4;
>  
> -  if (filename_cmp (ext, ".def") == 0 || filename_cmp (ext, ".DEF") == 0)
> +  if (fileext_cmp (ext, ".def") == 0)
>      {
>        pe_def_file = def_file_parse (entry->filename, pe_def_file);
>  

[PATCH v4] ld: Make archive member file extension comparisons case insensitive when cross compiling too

[Martin Storsjö]

On Windows, filename_cmp is case insensitive, but when cross compiling
with libraries that may contain members with uppercase file names, we
should keep those comparisons case insensitive when running the build
tools on other OSes too.
---
v4: Made the function assume that the second parameter is lower case
and without a leading '.'.
---
 ld/emultempl/pe.em  | 30 ++++++++++++++++++++++++++----
 ld/emultempl/pep.em | 29 +++++++++++++++++++++++++----
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/ld/emultempl/pe.em b/ld/emultempl/pe.em
index ad969ccec13..86120fa0f57 100644
--- a/ld/emultempl/pe.em
+++ b/ld/emultempl/pe.em
@@ -171,6 +171,28 @@ static int is_underscoring (void)
   return pe_leading_underscore;
 }
 
+/* A case insensitive comparison, regardless of the host platform, used for
+   comparing file extensions. Parameter s1 points at the extension in a file
+   name (pointing at the starting '.'). Parameter s2 is a lower case string
+   without the leading '.'. */
+static int fileext_cmp (const char *s1, const char *s2)
+{
+  if (*s1 != '.')
+    return 1;
+  s1++;
+  for (;;)
+    {
+      int c1 = TOLOWER (*s1++);
+      int c2 = *s2++; /* Assumed to be lower case from the caller. */
+
+      if (c1 != c2)
+        return (c1 - c2);
+
+      if (c1 == '\0')
+        return 0;
+    }
+}
+
 static void
 gld${EMULATION_NAME}_before_parse (void)
 {
@@ -1666,7 +1688,7 @@ gld${EMULATION_NAME}_after_open (void)
 		       extension, and use that for the remainder of the
 		       comparisons.  */
 		    pnt = strrchr (bfd_get_filename (is3->the_bfd), '.');
-		    if (pnt != NULL && filename_cmp (pnt, ".dll") == 0)
+		    if (pnt != NULL && fileext_cmp (pnt, "dll") == 0)
 		      break;
 		  }
 
@@ -1683,7 +1705,7 @@ gld${EMULATION_NAME}_after_open (void)
 			/* Skip static members, ie anything with a .obj
 			   extension.  */
 			pnt = strrchr (bfd_get_filename (is2->the_bfd), '.');
-			if (pnt != NULL && filename_cmp (pnt, ".obj") == 0)
+			if (pnt != NULL && fileext_cmp (pnt, "obj") == 0)
 			  continue;
 
 			if (filename_cmp (bfd_get_filename (is3->the_bfd),
@@ -1701,7 +1723,7 @@ gld${EMULATION_NAME}_after_open (void)
 	       then leave the filename alone.  */
 	    pnt = strrchr (bfd_get_filename (is->the_bfd), '.');
 
-	    if (is_ms_arch && (filename_cmp (pnt, ".dll") == 0))
+	    if (is_ms_arch && (fileext_cmp (pnt, "dll") == 0))
 	      {
 		int idata2 = 0, reloc_count=0;
 		asection *sec;
@@ -1857,7 +1879,7 @@ gld${EMULATION_NAME}_unrecognized_file (lang_input_statement_type *entry ATTRIBU
 #ifdef DLL_SUPPORT
   const char *ext = entry->filename + strlen (entry->filename) - 4;
 
-  if (filename_cmp (ext, ".def") == 0 || filename_cmp (ext, ".DEF") == 0)
+  if (fileext_cmp (ext, "def") == 0)
     {
       pe_def_file = def_file_parse (entry->filename, pe_def_file);
 
diff --git a/ld/emultempl/pep.em b/ld/emultempl/pep.em
index ee36a9a7e56..96dbd15a870 100644
--- a/ld/emultempl/pep.em
+++ b/ld/emultempl/pep.em
@@ -181,6 +181,27 @@ static int is_underscoring (void)
   return pep_leading_underscore;
 }
 
+/* A case insensitive comparison, regardless of the host platform, used for
+   comparing file extensions. Parameter s1 points at the extension in a file
+   name (pointing at the starting '.'). Parameter s2 is a lower case string
+   without the leading '.'. */
+static int fileext_cmp (const char *s1, const char *s2)
+{
+  if (*s1 != '.')
+    return 1;
+  s1++;
+  for (;;)
+    {
+      int c1 = TOLOWER (*s1++);
+      int c2 = *s2++; /* Assumed to be lower case from the caller. */
+
+      if (c1 != c2)
+        return (c1 - c2);
+
+      if (c1 == '\0')
+        return 0;
+    }
+}
 
 static void
 gld${EMULATION_NAME}_before_parse (void)
@@ -1630,7 +1651,7 @@ gld${EMULATION_NAME}_after_open (void)
 		       extension, and use that for the remainder of the
 		       comparisons.  */
 		    pnt = strrchr (bfd_get_filename (is3->the_bfd), '.');
-		    if (pnt != NULL && filename_cmp (pnt, ".dll") == 0)
+		    if (pnt != NULL && fileext_cmp (pnt, "dll") == 0)
 		      break;
 		  }
 
@@ -1647,7 +1668,7 @@ gld${EMULATION_NAME}_after_open (void)
 			/* Skip static members, ie anything with a .obj
 			   extension.  */
 			pnt = strrchr (bfd_get_filename (is2->the_bfd), '.');
-			if (pnt != NULL && filename_cmp (pnt, ".obj") == 0)
+			if (pnt != NULL && fileext_cmp (pnt, "obj") == 0)
 			  continue;
 
 			if (filename_cmp (bfd_get_filename (is3->the_bfd),
@@ -1665,7 +1686,7 @@ gld${EMULATION_NAME}_after_open (void)
 	       then leave the filename alone.  */
 	    pnt = strrchr (bfd_get_filename (is->the_bfd), '.');
 
-	    if (is_ms_arch && (filename_cmp (pnt, ".dll") == 0))
+	    if (is_ms_arch && (fileext_cmp (pnt, "dll") == 0))
 	      {
 		int idata2 = 0, reloc_count=0;
 		asection *sec;
@@ -1727,7 +1748,7 @@ gld${EMULATION_NAME}_unrecognized_file (lang_input_statement_type *entry ATTRIBU
 #ifdef DLL_SUPPORT
   const char *ext = entry->filename + strlen (entry->filename) - 4;
 
-  if (filename_cmp (ext, ".def") == 0 || filename_cmp (ext, ".DEF") == 0)
+  if (fileext_cmp (ext, "def") == 0)
     {
       pep_def_file = def_file_parse (entry->filename, pep_def_file);
 
-- 
2.25.1

Re: [PATCH v4] ld: Make archive member file extension comparisons case insensitive when cross compiling too

[Nick Clifton]

Hi Martin,

   Sorry to be persnickety, but ...

> +/* A case insensitive comparison, regardless of the host platform, used for
> +   comparing file extensions. Parameter s1 points at the extension in a file
> +   name (pointing at the starting '.'). Parameter s2 is a lower case string
> +   without the leading '.'. */

Comment formatting: two spaces before the closing */

> +static int fileext_cmp (const char *s1, const char *s2)

Function name formatting - name on a separate line from the return type.

And yes, I know that the same problem exists for the is_underscoring()
function directly before this one.  A patch to fix that formatting is
pre-approved. :-)


> +  if (*s1 != '.')
> +    return 1;

Strictly speaking this should be an error return value.  Maybe return
INT_MAX ?  Or generate an error message and then return 1.  This test
will probably never trigger however, so just returning 1 is OK really.
In fact just ignore me on this one...


> +      int c2 = *s2++; /* Assumed to be lower case from the caller. */

Assumptions are bad.  But testing this assumption does lead to needless
performance penalties.  So ignore me on this one too.

But please do fix up the comment formatting.


> diff --git a/ld/emultempl/pep.em b/ld/emultempl/pep.em

Similar comment apply here, of course.


Hmm.  We really ought to move the code duplicated in pe.em and pep.em into a
single file...

Cheers
   Nick

Re: [PATCH v4] ld: Make archive member file extension comparisons case insensitive when cross compiling too

[Martin Storsjö]

On Wed, 24 Aug 2022, Nick Clifton wrote:

> Hi Martin,
>
>  Sorry to be persnickety, but ...

No problem, I don't mind iterating on it to get it properly right, when 
the iteration time is reasonable :-)

>
>> +/* A case insensitive comparison, regardless of the host platform, used 
>> for
>> +   comparing file extensions. Parameter s1 points at the extension in a 
>> file
>> +   name (pointing at the starting '.'). Parameter s2 is a lower case 
>> string
>> +   without the leading '.'. */
>
> Comment formatting: two spaces before the closing */

Oh, I hadn't noticed that aspect - will fix.

>> +static int fileext_cmp (const char *s1, const char *s2)
>
> Function name formatting - name on a separate line from the return type.

Ok, sure.

> And yes, I know that the same problem exists for the is_underscoring()
> function directly before this one.  A patch to fix that formatting is
> pre-approved. :-)
>
>
>> +  if (*s1 != '.')
>> +    return 1;
>
> Strictly speaking this should be an error return value.  Maybe return
> INT_MAX ?  Or generate an error message and then return 1.  This test
> will probably never trigger however, so just returning 1 is OK really.
> In fact just ignore me on this one...

Ok, keeping this as is. Yes, it'd be unexpected if this isn't true, but 
from the point of view of the comparison, it's at least not a match for 
the extension.

>
>> +      int c2 = *s2++; /* Assumed to be lower case from the caller. */
>
> Assumptions are bad.  But testing this assumption does lead to needless
> performance penalties.  So ignore me on this one too.
>
> But please do fix up the comment formatting.

Ok.

>
>> diff --git a/ld/emultempl/pep.em b/ld/emultempl/pep.em
>
> Similar comment apply here, of course.
>
>
> Hmm.  We really ought to move the code duplicated in pe.em and pep.em into a
> single file...

Yep, totally. Unfortunately that aspect is way out of depth for me at the 
moment - I'm not familiar with binutils nearly well enough to take on 
that.

Currently I try to keep things in sync by doing the modifications on one 
and then applying the patch on the other file.

// Martin

[PATCH v5] ld: Make archive member file extension comparisons case insensitive when cross compiling too

[Martin Storsjö]

On Windows, filename_cmp is case insensitive, but when cross compiling
with libraries that may contain members with uppercase file names, we
should keep those comparisons case insensitive when running the build
tools on other OSes too.
---
v5: Moved the function name to a separate line, added double spaces
before the closing of comments.
---
 ld/emultempl/pe.em  | 31 +++++++++++++++++++++++++++----
 ld/emultempl/pep.em | 30 ++++++++++++++++++++++++++----
 2 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/ld/emultempl/pe.em b/ld/emultempl/pe.em
index ad969ccec13..d51bf4d3abb 100644
--- a/ld/emultempl/pe.em
+++ b/ld/emultempl/pe.em
@@ -171,6 +171,29 @@ static int is_underscoring (void)
   return pe_leading_underscore;
 }
 
+/* A case insensitive comparison, regardless of the host platform, used for
+   comparing file extensions. Parameter s1 points at the extension in a file
+   name (pointing at the starting '.'). Parameter s2 is a lower case string
+   without the leading '.'.  */
+static int
+fileext_cmp (const char *s1, const char *s2)
+{
+  if (*s1 != '.')
+    return 1;
+  s1++;
+  for (;;)
+    {
+      int c1 = TOLOWER (*s1++);
+      int c2 = *s2++; /* Assumed to be lower case from the caller.  */
+
+      if (c1 != c2)
+        return (c1 - c2);
+
+      if (c1 == '\0')
+        return 0;
+    }
+}
+
 static void
 gld${EMULATION_NAME}_before_parse (void)
 {
@@ -1666,7 +1689,7 @@ gld${EMULATION_NAME}_after_open (void)
 		       extension, and use that for the remainder of the
 		       comparisons.  */
 		    pnt = strrchr (bfd_get_filename (is3->the_bfd), '.');
-		    if (pnt != NULL && filename_cmp (pnt, ".dll") == 0)
+		    if (pnt != NULL && fileext_cmp (pnt, "dll") == 0)
 		      break;
 		  }
 
@@ -1683,7 +1706,7 @@ gld${EMULATION_NAME}_after_open (void)
 			/* Skip static members, ie anything with a .obj
 			   extension.  */
 			pnt = strrchr (bfd_get_filename (is2->the_bfd), '.');
-			if (pnt != NULL && filename_cmp (pnt, ".obj") == 0)
+			if (pnt != NULL && fileext_cmp (pnt, "obj") == 0)
 			  continue;
 
 			if (filename_cmp (bfd_get_filename (is3->the_bfd),
@@ -1701,7 +1724,7 @@ gld${EMULATION_NAME}_after_open (void)
 	       then leave the filename alone.  */
 	    pnt = strrchr (bfd_get_filename (is->the_bfd), '.');
 
-	    if (is_ms_arch && (filename_cmp (pnt, ".dll") == 0))
+	    if (is_ms_arch && (fileext_cmp (pnt, "dll") == 0))
 	      {
 		int idata2 = 0, reloc_count=0;
 		asection *sec;
@@ -1857,7 +1880,7 @@ gld${EMULATION_NAME}_unrecognized_file (lang_input_statement_type *entry ATTRIBU
 #ifdef DLL_SUPPORT
   const char *ext = entry->filename + strlen (entry->filename) - 4;
 
-  if (filename_cmp (ext, ".def") == 0 || filename_cmp (ext, ".DEF") == 0)
+  if (fileext_cmp (ext, "def") == 0)
     {
       pe_def_file = def_file_parse (entry->filename, pe_def_file);
 
diff --git a/ld/emultempl/pep.em b/ld/emultempl/pep.em
index ee36a9a7e56..f060bf092ed 100644
--- a/ld/emultempl/pep.em
+++ b/ld/emultempl/pep.em
@@ -181,6 +181,28 @@ static int is_underscoring (void)
   return pep_leading_underscore;
 }
 
+/* A case insensitive comparison, regardless of the host platform, used for
+   comparing file extensions. Parameter s1 points at the extension in a file
+   name (pointing at the starting '.'). Parameter s2 is a lower case string
+   without the leading '.'.  */
+static int
+fileext_cmp (const char *s1, const char *s2)
+{
+  if (*s1 != '.')
+    return 1;
+  s1++;
+  for (;;)
+    {
+      int c1 = TOLOWER (*s1++);
+      int c2 = *s2++; /* Assumed to be lower case from the caller.  */
+
+      if (c1 != c2)
+        return (c1 - c2);
+
+      if (c1 == '\0')
+        return 0;
+    }
+}
 
 static void
 gld${EMULATION_NAME}_before_parse (void)
@@ -1630,7 +1652,7 @@ gld${EMULATION_NAME}_after_open (void)
 		       extension, and use that for the remainder of the
 		       comparisons.  */
 		    pnt = strrchr (bfd_get_filename (is3->the_bfd), '.');
-		    if (pnt != NULL && filename_cmp (pnt, ".dll") == 0)
+		    if (pnt != NULL && fileext_cmp (pnt, "dll") == 0)
 		      break;
 		  }
 
@@ -1647,7 +1669,7 @@ gld${EMULATION_NAME}_after_open (void)
 			/* Skip static members, ie anything with a .obj
 			   extension.  */
 			pnt = strrchr (bfd_get_filename (is2->the_bfd), '.');
-			if (pnt != NULL && filename_cmp (pnt, ".obj") == 0)
+			if (pnt != NULL && fileext_cmp (pnt, "obj") == 0)
 			  continue;
 
 			if (filename_cmp (bfd_get_filename (is3->the_bfd),
@@ -1665,7 +1687,7 @@ gld${EMULATION_NAME}_after_open (void)
 	       then leave the filename alone.  */
 	    pnt = strrchr (bfd_get_filename (is->the_bfd), '.');
 
-	    if (is_ms_arch && (filename_cmp (pnt, ".dll") == 0))
+	    if (is_ms_arch && (fileext_cmp (pnt, "dll") == 0))
 	      {
 		int idata2 = 0, reloc_count=0;
 		asection *sec;
@@ -1727,7 +1749,7 @@ gld${EMULATION_NAME}_unrecognized_file (lang_input_statement_type *entry ATTRIBU
 #ifdef DLL_SUPPORT
   const char *ext = entry->filename + strlen (entry->filename) - 4;
 
-  if (filename_cmp (ext, ".def") == 0 || filename_cmp (ext, ".DEF") == 0)
+  if (fileext_cmp (ext, "def") == 0)
     {
       pep_def_file = def_file_parse (entry->filename, pep_def_file);
 
-- 
2.25.1

Re: [PATCH v5] ld: Make archive member file extension comparisons case insensitive when cross compiling too

[Jan Beulich]

On 24.08.2022 12:04, Martin Storsjö wrote:
> @@ -1857,7 +1880,7 @@ gld${EMULATION_NAME}_unrecognized_file (lang_input_statement_type *entry ATTRIBU
>  #ifdef DLL_SUPPORT
>    const char *ext = entry->filename + strlen (entry->filename) - 4;
>  
> -  if (filename_cmp (ext, ".def") == 0 || filename_cmp (ext, ".DEF") == 0)
> +  if (fileext_cmp (ext, "def") == 0)
>      {
>        pe_def_file = def_file_parse (entry->filename, pe_def_file);
>  

The pre-existing code doesn't look safe here (and I did overlook the
lack of strrchr() here when writing my earlier reply). There's a
buffer underflow for file names shorter than 4 characters.

And I'm inclined to say that ".def" on its own isn't a .def-file, but
a file without any extension. (This applies to all other cases you
change as well.)

If I was touching all of this anyway, I'd be inclined to address both
issues as a "side effect" of the patch. But of course it's not a
requirement; it can easily be a separate, later patch. Or you could
also elect to switch to using strrchr() here (thus allowing code to
be dropped from the new function with callers all adding 1 to the
pointer they pass), but leave the "not really an extension" part
alone.

Jan

Re: [PATCH v5] ld: Make archive member file extension comparisons case insensitive when cross compiling too

[Martin Storsjö]

[-- Attachment #1: Type: text/plain, Size: 1842 bytes --]

On Wed, 24 Aug 2022, Jan Beulich wrote:

> On 24.08.2022 12:04, Martin Storsjö wrote:
>> @@ -1857,7 +1880,7 @@ gld${EMULATION_NAME}_unrecognized_file (lang_input_statement_type *entry ATTRIBU
>>  #ifdef DLL_SUPPORT
>>    const char *ext = entry->filename + strlen (entry->filename) - 4;
>>
>> -  if (filename_cmp (ext, ".def") == 0 || filename_cmp (ext, ".DEF") == 0)
>> +  if (fileext_cmp (ext, "def") == 0)
>>      {
>>        pe_def_file = def_file_parse (entry->filename, pe_def_file);
>>
>
> The pre-existing code doesn't look safe here (and I did overlook the
> lack of strrchr() here when writing my earlier reply). There's a
> buffer underflow for file names shorter than 4 characters.

Oh, indeed!

> And I'm inclined to say that ".def" on its own isn't a .def-file, but
> a file without any extension. (This applies to all other cases you
> change as well.)
>
> If I was touching all of this anyway, I'd be inclined to address both
> issues as a "side effect" of the patch. But of course it's not a
> requirement; it can easily be a separate, later patch. Or you could
> also elect to switch to using strrchr() here (thus allowing code to
> be dropped from the new function with callers all adding 1 to the
> pointer they pass), but leave the "not really an extension" part
> alone.

I think simplicity is key here; whatever keeps the code the simplest is 
best, since exactly how we handle hypothetical cases here probably 
shouldn't matter much in practice, as long as it's safe.

At the third call site, I also noticed that we're lacking a null pointer 
check before invoking the comparison, compared to the other ones - I'll 
amend that too. (It's possible that it's in a place where we know for sure 
that it's non-null, but it's not immediately obvious when looking at it 
with the context of the patch at least.)

// Martin

Re: [PATCH v5] ld: Make archive member file extension comparisons case insensitive when cross compiling too

[Martin Storsjö]

On Wed, 24 Aug 2022, Jan Beulich wrote:

> On 24.08.2022 12:04, Martin Storsjö wrote:
>> @@ -1857,7 +1880,7 @@ gld${EMULATION_NAME}_unrecognized_file (lang_input_statement_type *entry ATTRIBU
>>  #ifdef DLL_SUPPORT
>>    const char *ext = entry->filename + strlen (entry->filename) - 4;
>>
>> -  if (filename_cmp (ext, ".def") == 0 || filename_cmp (ext, ".DEF") == 0)
>> +  if (fileext_cmp (ext, "def") == 0)
>>      {
>>        pe_def_file = def_file_parse (entry->filename, pe_def_file);
>>
>
> The pre-existing code doesn't look safe here (and I did overlook the
> lack of strrchr() here when writing my earlier reply). There's a
> buffer underflow for file names shorter than 4 characters.

Oh, indeed!

> And I'm inclined to say that ".def" on its own isn't a .def-file, but
> a file without any extension. (This applies to all other cases you
> change as well.)
>
> If I was touching all of this anyway, I'd be inclined to address both
> issues as a "side effect" of the patch. But of course it's not a
> requirement; it can easily be a separate, later patch. Or you could
> also elect to switch to using strrchr() here (thus allowing code to
> be dropped from the new function with callers all adding 1 to the
> pointer they pass), but leave the "not really an extension" part
> alone.

I think simplicity is key here; whatever keeps the code the simplest is 
best, since exactly how we handle hypothetical cases here probably 
shouldn't matter much in practice, as long as it's safe.

At the third call site, I also noticed that we're lacking a null pointer 
check before invoking the comparison, compared to the other ones - I'll 
amend that too. (It's possible that it's in a place where we know for sure 
that it's non-null, but it's not immediately obvious when looking at it 
with the context of the patch at least.)

// Martin

[PATCH v6] ld: Make archive member file extension comparisons case insensitive when cross compiling too

[Martin Storsjö]

On Windows, filename_cmp is case insensitive, but when cross compiling
with libraries that may contain members with uppercase file names, we
should keep those comparisons case insensitive when running the build
tools on other OSes too.

Also make the check for .def consistent with the other ones, fixing
out of bounds reads if file names are shorter than 4 characters.
---
v6: Switch the check for .def extension to use strrchr, add a missed
null pointer check at the third call site.
---
 ld/emultempl/pe.em  | 32 +++++++++++++++++++++++++++-----
 ld/emultempl/pep.em | 32 +++++++++++++++++++++++++++-----
 2 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/ld/emultempl/pe.em b/ld/emultempl/pe.em
index ad969ccec13..03c261e89c6 100644
--- a/ld/emultempl/pe.em
+++ b/ld/emultempl/pe.em
@@ -534,6 +534,28 @@ gld${EMULATION_NAME}_list_options (FILE *file)
   fprintf (file, _("  --build-id[=STYLE]                 Generate build ID\n"));
 }
 
+/* A case insensitive comparison, regardless of the host platform, used for
+   comparing file extensions. Parameter s1 points at the extension in a file
+   name (pointing at the starting '.'). Parameter s2 is a lower case string
+   without the leading '.'.  */
+static int
+fileext_cmp (const char *s1, const char *s2)
+{
+  if (*s1 != '.')
+    return 1;
+  s1++;
+  for (;;)
+    {
+      int c1 = TOLOWER (*s1++);
+      int c2 = *s2++; /* Assumed to be lower case from the caller.  */
+
+      if (c1 != c2)
+        return (c1 - c2);
+
+      if (c1 == '\0')
+        return 0;
+    }
+}
 
 static void
 set_pe_name (char *name, long val)
@@ -1666,7 +1688,7 @@ gld${EMULATION_NAME}_after_open (void)
 		       extension, and use that for the remainder of the
 		       comparisons.  */
 		    pnt = strrchr (bfd_get_filename (is3->the_bfd), '.');
-		    if (pnt != NULL && filename_cmp (pnt, ".dll") == 0)
+		    if (pnt != NULL && fileext_cmp (pnt, "dll") == 0)
 		      break;
 		  }
 
@@ -1683,7 +1705,7 @@ gld${EMULATION_NAME}_after_open (void)
 			/* Skip static members, ie anything with a .obj
 			   extension.  */
 			pnt = strrchr (bfd_get_filename (is2->the_bfd), '.');
-			if (pnt != NULL && filename_cmp (pnt, ".obj") == 0)
+			if (pnt != NULL && fileext_cmp (pnt, "obj") == 0)
 			  continue;
 
 			if (filename_cmp (bfd_get_filename (is3->the_bfd),
@@ -1701,7 +1723,7 @@ gld${EMULATION_NAME}_after_open (void)
 	       then leave the filename alone.  */
 	    pnt = strrchr (bfd_get_filename (is->the_bfd), '.');
 
-	    if (is_ms_arch && (filename_cmp (pnt, ".dll") == 0))
+	    if (is_ms_arch && pnt != NULL && (fileext_cmp (pnt, "dll") == 0))
 	      {
 		int idata2 = 0, reloc_count=0;
 		asection *sec;
@@ -1855,9 +1877,9 @@ static bool
 gld${EMULATION_NAME}_unrecognized_file (lang_input_statement_type *entry ATTRIBUTE_UNUSED)
 {
 #ifdef DLL_SUPPORT
-  const char *ext = entry->filename + strlen (entry->filename) - 4;
+  const char *ext = strrchr (entry->filename, '.');
 
-  if (filename_cmp (ext, ".def") == 0 || filename_cmp (ext, ".DEF") == 0)
+  if (ext != NULL && fileext_cmp (ext, "def") == 0)
     {
       pe_def_file = def_file_parse (entry->filename, pe_def_file);
 
diff --git a/ld/emultempl/pep.em b/ld/emultempl/pep.em
index ee36a9a7e56..239f830edbb 100644
--- a/ld/emultempl/pep.em
+++ b/ld/emultempl/pep.em
@@ -181,6 +181,28 @@ static int is_underscoring (void)
   return pep_leading_underscore;
 }
 
+/* A case insensitive comparison, regardless of the host platform, used for
+   comparing file extensions. Parameter s1 points at the extension in a file
+   name (pointing at the starting '.'). Parameter s2 is a lower case string
+   without the leading '.'.  */
+static int
+fileext_cmp (const char *s1, const char *s2)
+{
+  if (*s1 != '.')
+    return 1;
+  s1++;
+  for (;;)
+    {
+      int c1 = TOLOWER (*s1++);
+      int c2 = *s2++; /* Assumed to be lower case from the caller.  */
+
+      if (c1 != c2)
+        return (c1 - c2);
+
+      if (c1 == '\0')
+        return 0;
+    }
+}
 
 static void
 gld${EMULATION_NAME}_before_parse (void)
@@ -1630,7 +1652,7 @@ gld${EMULATION_NAME}_after_open (void)
 		       extension, and use that for the remainder of the
 		       comparisons.  */
 		    pnt = strrchr (bfd_get_filename (is3->the_bfd), '.');
-		    if (pnt != NULL && filename_cmp (pnt, ".dll") == 0)
+		    if (pnt != NULL && fileext_cmp (pnt, "dll") == 0)
 		      break;
 		  }
 
@@ -1647,7 +1669,7 @@ gld${EMULATION_NAME}_after_open (void)
 			/* Skip static members, ie anything with a .obj
 			   extension.  */
 			pnt = strrchr (bfd_get_filename (is2->the_bfd), '.');
-			if (pnt != NULL && filename_cmp (pnt, ".obj") == 0)
+			if (pnt != NULL && fileext_cmp (pnt, "obj") == 0)
 			  continue;
 
 			if (filename_cmp (bfd_get_filename (is3->the_bfd),
@@ -1665,7 +1687,7 @@ gld${EMULATION_NAME}_after_open (void)
 	       then leave the filename alone.  */
 	    pnt = strrchr (bfd_get_filename (is->the_bfd), '.');
 
-	    if (is_ms_arch && (filename_cmp (pnt, ".dll") == 0))
+	    if (is_ms_arch && pnt != NULL && (fileext_cmp (pnt, "dll") == 0))
 	      {
 		int idata2 = 0, reloc_count=0;
 		asection *sec;
@@ -1725,9 +1747,9 @@ static bool
 gld${EMULATION_NAME}_unrecognized_file (lang_input_statement_type *entry ATTRIBUTE_UNUSED)
 {
 #ifdef DLL_SUPPORT
-  const char *ext = entry->filename + strlen (entry->filename) - 4;
+  const char *ext = strrchr (entry->filename, '.');
 
-  if (filename_cmp (ext, ".def") == 0 || filename_cmp (ext, ".DEF") == 0)
+  if (ext != NULL && fileext_cmp (ext, "def") == 0)
     {
       pep_def_file = def_file_parse (entry->filename, pep_def_file);
 
-- 
2.25.1

Re: [PATCH v6] ld: Make archive member file extension comparisons case insensitive when cross compiling too

[Jan Beulich]

On 24.08.2022 12:47, Martin Storsjö wrote:
> On Windows, filename_cmp is case insensitive, but when cross compiling
> with libraries that may contain members with uppercase file names, we
> should keep those comparisons case insensitive when running the build
> tools on other OSes too.
> 
> Also make the check for .def consistent with the other ones, fixing
> out of bounds reads if file names are shorter than 4 characters.
> ---
> v6: Switch the check for .def extension to use strrchr, add a missed
> null pointer check at the third call site.

With that, as I've previously tried to express, ...

> --- a/ld/emultempl/pe.em
> +++ b/ld/emultempl/pe.em
> @@ -534,6 +534,28 @@ gld${EMULATION_NAME}_list_options (FILE *file)
>    fprintf (file, _("  --build-id[=STYLE]                 Generate build ID\n"));
>  }
>  
> +/* A case insensitive comparison, regardless of the host platform, used for
> +   comparing file extensions. Parameter s1 points at the extension in a file
> +   name (pointing at the starting '.'). Parameter s2 is a lower case string
> +   without the leading '.'.  */
> +static int
> +fileext_cmp (const char *s1, const char *s2)
> +{
> +  if (*s1 != '.')
> +    return 1;
> +  s1++;

... this is now properly dead code, and instead ...

> +  for (;;)
> +    {
> +      int c1 = TOLOWER (*s1++);
> +      int c2 = *s2++; /* Assumed to be lower case from the caller.  */
> +
> +      if (c1 != c2)
> +        return (c1 - c2);
> +
> +      if (c1 == '\0')
> +        return 0;
> +    }
> +}
>  
>  static void
>  set_pe_name (char *name, long val)
> @@ -1666,7 +1688,7 @@ gld${EMULATION_NAME}_after_open (void)
>  		       extension, and use that for the remainder of the
>  		       comparisons.  */
>  		    pnt = strrchr (bfd_get_filename (is3->the_bfd), '.');
> -		    if (pnt != NULL && filename_cmp (pnt, ".dll") == 0)
> +		    if (pnt != NULL && fileext_cmp (pnt, "dll") == 0)

... you'll want to pass pnt + 1 to the function here (and then similarly
below).

Jan

[PATCH v7] ld: Make archive member file extension comparisons case insensitive when cross compiling too

[Martin Storsjö]

On Windows, filename_cmp is case insensitive, but when cross compiling
with libraries that may contain members with uppercase file names, we
should keep those comparisons case insensitive when running the build
tools on other OSes too.

Also make the check for .def consistent with the other ones, fixing
out of bounds reads if file names are shorter than 4 characters.
---
v7: Skip the '.' entirely in the first parameter to fileext_cmp too,
simplifying the function and making the two parameters more symmetrical.
---
 ld/emultempl/pe.em  | 27 ++++++++++++++++++++++-----
 ld/emultempl/pep.em | 27 ++++++++++++++++++++++-----
 2 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/ld/emultempl/pe.em b/ld/emultempl/pe.em
index ad969ccec13..2706d73d144 100644
--- a/ld/emultempl/pe.em
+++ b/ld/emultempl/pe.em
@@ -534,6 +534,23 @@ gld${EMULATION_NAME}_list_options (FILE *file)
   fprintf (file, _("  --build-id[=STYLE]                 Generate build ID\n"));
 }
 
+/* A case insensitive comparison, regardless of the host platform, used for
+   comparing file extensions.  */
+static int
+fileext_cmp (const char *s1, const char *s2)
+{
+  for (;;)
+    {
+      int c1 = TOLOWER (*s1++);
+      int c2 = *s2++; /* Assumed to be lower case from the caller.  */
+
+      if (c1 != c2)
+        return (c1 - c2);
+
+      if (c1 == '\0')
+        return 0;
+    }
+}
 
 static void
 set_pe_name (char *name, long val)
@@ -1666,7 +1683,7 @@ gld${EMULATION_NAME}_after_open (void)
 		       extension, and use that for the remainder of the
 		       comparisons.  */
 		    pnt = strrchr (bfd_get_filename (is3->the_bfd), '.');
-		    if (pnt != NULL && filename_cmp (pnt, ".dll") == 0)
+		    if (pnt != NULL && fileext_cmp (pnt + 1, "dll") == 0)
 		      break;
 		  }
 
@@ -1683,7 +1700,7 @@ gld${EMULATION_NAME}_after_open (void)
 			/* Skip static members, ie anything with a .obj
 			   extension.  */
 			pnt = strrchr (bfd_get_filename (is2->the_bfd), '.');
-			if (pnt != NULL && filename_cmp (pnt, ".obj") == 0)
+			if (pnt != NULL && fileext_cmp (pnt + 1, "obj") == 0)
 			  continue;
 
 			if (filename_cmp (bfd_get_filename (is3->the_bfd),
@@ -1701,7 +1718,7 @@ gld${EMULATION_NAME}_after_open (void)
 	       then leave the filename alone.  */
 	    pnt = strrchr (bfd_get_filename (is->the_bfd), '.');
 
-	    if (is_ms_arch && (filename_cmp (pnt, ".dll") == 0))
+	    if (is_ms_arch && pnt != NULL && (fileext_cmp (pnt + 1, "dll") == 0))
 	      {
 		int idata2 = 0, reloc_count=0;
 		asection *sec;
@@ -1855,9 +1872,9 @@ static bool
 gld${EMULATION_NAME}_unrecognized_file (lang_input_statement_type *entry ATTRIBUTE_UNUSED)
 {
 #ifdef DLL_SUPPORT
-  const char *ext = entry->filename + strlen (entry->filename) - 4;
+  const char *ext = strrchr (entry->filename, '.');
 
-  if (filename_cmp (ext, ".def") == 0 || filename_cmp (ext, ".DEF") == 0)
+  if (ext != NULL && fileext_cmp (ext + 1, "def") == 0)
     {
       pe_def_file = def_file_parse (entry->filename, pe_def_file);
 
diff --git a/ld/emultempl/pep.em b/ld/emultempl/pep.em
index ee36a9a7e56..accefc0ed60 100644
--- a/ld/emultempl/pep.em
+++ b/ld/emultempl/pep.em
@@ -181,6 +181,23 @@ static int is_underscoring (void)
   return pep_leading_underscore;
 }
 
+/* A case insensitive comparison, regardless of the host platform, used for
+   comparing file extensions.  */
+static int
+fileext_cmp (const char *s1, const char *s2)
+{
+  for (;;)
+    {
+      int c1 = TOLOWER (*s1++);
+      int c2 = *s2++; /* Assumed to be lower case from the caller.  */
+
+      if (c1 != c2)
+        return (c1 - c2);
+
+      if (c1 == '\0')
+        return 0;
+    }
+}
 
 static void
 gld${EMULATION_NAME}_before_parse (void)
@@ -1630,7 +1647,7 @@ gld${EMULATION_NAME}_after_open (void)
 		       extension, and use that for the remainder of the
 		       comparisons.  */
 		    pnt = strrchr (bfd_get_filename (is3->the_bfd), '.');
-		    if (pnt != NULL && filename_cmp (pnt, ".dll") == 0)
+		    if (pnt != NULL && fileext_cmp (pnt + 1, "dll") == 0)
 		      break;
 		  }
 
@@ -1647,7 +1664,7 @@ gld${EMULATION_NAME}_after_open (void)
 			/* Skip static members, ie anything with a .obj
 			   extension.  */
 			pnt = strrchr (bfd_get_filename (is2->the_bfd), '.');
-			if (pnt != NULL && filename_cmp (pnt, ".obj") == 0)
+			if (pnt != NULL && fileext_cmp (pnt + 1, "obj") == 0)
 			  continue;
 
 			if (filename_cmp (bfd_get_filename (is3->the_bfd),
@@ -1665,7 +1682,7 @@ gld${EMULATION_NAME}_after_open (void)
 	       then leave the filename alone.  */
 	    pnt = strrchr (bfd_get_filename (is->the_bfd), '.');
 
-	    if (is_ms_arch && (filename_cmp (pnt, ".dll") == 0))
+	    if (is_ms_arch && pnt != NULL && (fileext_cmp (pnt + 1, "dll") == 0))
 	      {
 		int idata2 = 0, reloc_count=0;
 		asection *sec;
@@ -1725,9 +1742,9 @@ static bool
 gld${EMULATION_NAME}_unrecognized_file (lang_input_statement_type *entry ATTRIBUTE_UNUSED)
 {
 #ifdef DLL_SUPPORT
-  const char *ext = entry->filename + strlen (entry->filename) - 4;
+  const char *ext = strrchr (entry->filename, '.');
 
-  if (filename_cmp (ext, ".def") == 0 || filename_cmp (ext, ".DEF") == 0)
+  if (ext != NULL && fileext_cmp (ext + 1, "def") == 0)
     {
       pep_def_file = def_file_parse (entry->filename, pep_def_file);
 
-- 
2.25.1

Re: [PATCH v7] ld: Make archive member file extension comparisons case insensitive when cross compiling too

[Jan Beulich]

On 24.08.2022 14:25, Martin Storsjö wrote:
> On Windows, filename_cmp is case insensitive, but when cross compiling
> with libraries that may contain members with uppercase file names, we
> should keep those comparisons case insensitive when running the build
> tools on other OSes too.
> 
> Also make the check for .def consistent with the other ones, fixing
> out of bounds reads if file names are shorter than 4 characters.
> ---
> v7: Skip the '.' entirely in the first parameter to fileext_cmp too,
> simplifying the function and making the two parameters more symmetrical.

Thanks, lgtm now. But before committing please give Nick another chance
to comment back (I guess waiting for a day or so should suffice).

And thanks for going through full 7 iterations - that's certainly not
a usual thing for binutils patches.

Jan

Re: [PATCH v7] ld: Make archive member file extension comparisons case insensitive when cross compiling too

[Nick Clifton]

Hi Martin,

> v7: Skip the '.' entirely in the first parameter to fileext_cmp too,
> simplifying the function and making the two parameters more symmetrical.

Approved - please apply.  (And thanks for persevering).

Cheers
   Nick

Re: [PATCH v7] ld: Make archive member file extension comparisons case insensitive when cross compiling too

[Martin Storsjö]

On Wed, 24 Aug 2022, Nick Clifton wrote:

> Hi Martin,
>
>> v7: Skip the '.' entirely in the first parameter to fileext_cmp too,
>> simplifying the function and making the two parameters more symmetrical.
>
> Approved - please apply.  (And thanks for persevering).

Applied - thanks to both of you!

// Martin

Re: [PATCH v4] ld: Make archive member file extension comparisons case insensitive when cross compiling too

[Martin Storsjö]

On Wed, 24 Aug 2022, Nick Clifton wrote:

>> +/* A case insensitive comparison, regardless of the host platform, used 
>> for
>> +   comparing file extensions. Parameter s1 points at the extension in a 
>> file
>> +   name (pointing at the starting '.'). Parameter s2 is a lower case 
>> string
>> +   without the leading '.'. */
>
> Comment formatting: two spaces before the closing */
>
>> +static int fileext_cmp (const char *s1, const char *s2)
>
> Function name formatting - name on a separate line from the return type.
>
> And yes, I know that the same problem exists for the is_underscoring()
> function directly before this one.  A patch to fix that formatting is
> pre-approved. :-)

I pushed a patch that fixes this aspect now as well, for consistency.

// Martin