[PATCH v2] pe/coff - add support for base64 encoded long section names

[Jan Beulich]

On 22.05.2023 21:48, Tristan Gingold via Binutils wrote:
> so new version of the patch, including generation of base64 encoded 
> section name indexes.
> 
> I was able to assemble the test from PR 30444 (using -mbig-obj to 
> overcome the 2^16 sections number limit).
> 
> I haven't added a test due to its size.
> 
> No failures on x86_64-gnu-linux for binutils configured for 
> x86_64-pc-mingw64.

A couple of nits and a question, looks good to me otherwise.

> --- a/bfd/coffcode.h
> +++ b/bfd/coffcode.h
> @@ -3625,18 +3625,54 @@ coff_write_object_contents (bfd * abfd)
>   	  len = strlen (current->name);
>   	  if (len > SCNNMLEN)
>   	    {
> -	      /* The s_name field is defined to be NUL-padded but need not be
> -		 NUL-terminated.  We use a temporary buffer so that we can still
> -		 sprintf all eight chars without splatting a terminating NUL
> -		 over the first byte of the following member (s_paddr).  */
> -	      /* PR 21096: The +20 is to stop a bogus warning from gcc7 about
> -		 a possible buffer overflow.  */
> -	      char s_name_buf[SCNNMLEN + 1 + 20];
> 
>   	      /* An inherent limitation of the /nnnnnnn notation used to indicate
>   		 the offset of the long name in the string table is that we
>   		 cannot address entries beyone the ten million byte boundary.  */
> -	      if (string_size >= 10000000)
> +	      if (string_size < 10000000)
> +		{
> +		  /* The s_name field is defined to be NUL-padded but need not
> +		     be NUL-terminated.  We use a temporary buffer so that we
> +		     can still sprintf all eight chars without splatting a
> +		     terminating NUL over the first byte of the following
> +		     member (s_paddr).  */
> +		  /* PR 21096: The +20 is to stop a bogus warning from gcc7
> +		     about a possible buffer overflow.  */
> +		  char s_name_buf[SCNNMLEN + 1 + 20];
> +
> +		  /* We do not need to use snprintf here as we have already
> +		     verified that string_size is not too big, plus we have
> +		     an overlarge buffer, just in case.  */
> +		  sprintf (s_name_buf, "/%lu", (unsigned long) string_size);
> +		  /* Then strncpy takes care of any padding for us.  */
> +		  strncpy (section.s_name, s_name_buf, SCNNMLEN);
> +		}
> +	      else
> +#ifdef COFF_WITH_PE
> +		{
> +		  /* PE use a bae64 encoding for long section names whose

base64

> +		     index is very large.  */
> +		  static const char base64[] =
> +		    "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
> +		    "abcdefghijklmnopqrstuvwxyz"
> +		    "0123456789+/";
> +		  unsigned long off = string_size;
> +		  unsigned i;
> +
> +		  section.s_name[0] = '/';
> +		  section.s_name[1] = '/';
> +		  for (i = SCNNMLEN - 1; i >=2; i--)

Blank before 2 please.

> +/* Decode a base64 coded string at STR of length LEN, and write the result
> +   to RES.  Return true on success.
> +   Return false in case of invalid character or overflow.  */
> +
> +static bool
> +decode_base64 (const char *str, unsigned len, uint32_t *res)
> +{
> +  unsigned i;
> +  uint32_t val;
> +
> +  val = 0;
> +  for (i = 0; i < len; i++)
> +    {
> +      char c = str[i];
> +      unsigned d;
> +
> +      if (c >= 'A' && c <= 'Z')
> +	d = c - 'A';
> +      else if (c >= 'a' && c <= 'z')
> +	d = c - 'a' + 26;
> +      else if (c >= '0' && c <= '9')
> +	d = c - '0' + 52;
> +      else if (c == '+')
> +	d = 62;
> +      else if (c == '/')
> +	d = 63;
> +      else
> +	return false;
> +
> +      /* Check for overflow. */
> +      if ((val >> 26) != 0)
> +	return false;
> +
> +      val = (val << 6) + d;
> +    }
> +
> +  *res = val;
> +  return true;
> +}

So you decided to not also permit the nul-tail-padded form along with
the "canonical" 'A'-head-padded one here? (Generating just the
"canonical" form in coff_write_object_contents() is of course not under
question.)

Jan