Commit: _bfd_is_local_label_name: Include assembler local labels

Commit: _bfd_is_local_label_name: Include assembler local labels

[Nick Clifton]

Hi Guys,

  I am checking in the patch below to update _bfd_is_local_label_name so
  that it includes assembler generated local labels.

Cheers
  Nick

bfd/ChangeLog
2015-04-07  Nick Clifton  <nickc@redhat.com>

	* elf.c (_bfd_elf_is_local_label_name): Treat assembler generated
	local labels as local.

diff --git a/bfd/elf.c b/bfd/elf.c
index cbc0c91..bb5f1c6 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -7743,6 +7743,10 @@ _bfd_elf_is_local_label_name (bfd *abfd ATTRIBUTE_UNUSED,
   if (name[0] == '_' && name[1] == '.' && name[2] == 'L' && name[3] == '_')
     return TRUE;
 
+  /* Treat assembler generated local labels as local.  */
+  if (name[0] == 'L' && name[strlen (name) - 1] < 32)
+    return TRUE;
+
   return FALSE;
 }
 

Re: Commit: _bfd_is_local_label_name: Include assembler local labels

[H.J. Lu]

On Tue, Apr 7, 2015 at 3:22 AM, Nick Clifton <nickc@redhat.com> wrote:
> Hi Guys,
>
>   I am checking in the patch below to update _bfd_is_local_label_name so
>   that it includes assembler generated local labels.
>
> Cheers
>   Nick
>
> bfd/ChangeLog
> 2015-04-07  Nick Clifton  <nickc@redhat.com>
>
>         * elf.c (_bfd_elf_is_local_label_name): Treat assembler generated
>         local labels as local.
>
> diff --git a/bfd/elf.c b/bfd/elf.c
> index cbc0c91..bb5f1c6 100644
> --- a/bfd/elf.c
> +++ b/bfd/elf.c
> @@ -7743,6 +7743,10 @@ _bfd_elf_is_local_label_name (bfd *abfd ATTRIBUTE_UNUSED,
>    if (name[0] == '_' && name[1] == '.' && name[2] == 'L' && name[3] == '_')
>      return TRUE;
>
> +  /* Treat assembler generated local labels as local.  */
> +  if (name[0] == 'L' && name[strlen (name) - 1] < 32)
> +    return TRUE;
> +
>    return FALSE;
>  }
>

Do you have a testcase for this?  I thought assembler generated local label
on ELF targets started with ".L", not "L".

-- 
H.J.

Re: Commit: _bfd_is_local_label_name: Include assembler local labels

[Andreas Schwab]

Nick Clifton <nickc@redhat.com> writes:

> +  /* Treat assembler generated local labels as local.  */
> +  if (name[0] == 'L' && name[strlen (name) - 1] < 32)

Local/dollar labels have a trailing instance number after the ^B/^A.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Commit: _bfd_is_local_label_name: Include assembler local labels

[Nicholas Clifton]

Hi Andreas, Hi H.J.,

 >> +  /* Treat assembler generated local labels as local.  */
 >> +  if (name[0] == 'L' && name[strlen (name) - 1] < 32)


> Do you have a testcase for this?

Not specifically for this feature, but lots of tests in the gas, ld and 
binutils testsuites involve invoking this function.


> I thought assembler generated local label
> on ELF targets started with ".L", not "L".

Only if the target defines LOCAL_LABEL_PREFIX as '.'.  Not all targets 
do this.  For targets that do define LOCAL_LABEL_PREFIX this way the 
test at the start of _bfd_elf_is_local_label_name will have already 
caught them.

> Local/dollar labels have a trailing instance number after the ^B/^A.

Ah - I had not come across this before.

Whilst rechecking the patch, I also came across assembler generated fake 
symbol names, which are also supposed to be treated as locals.  So, 
before I check it in, what do you guys think of this reworking of the 
assembler local symbol patch ?

Cheers
   Nick

diff --git a/bfd/elf.c b/bfd/elf.c
index 5fad4f1..b986a60 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -7751,9 +7751,46 @@ _bfd_elf_is_local_label_name (bfd *abfd 
ATTRIBUTE_UNUSED,
    if (name[0] == '_' && name[1] == '.' && name[2] == 'L' && name[3] == 
'_')
      return TRUE;

-  /* Treat assembler generated local labels as local.  */
-  if (name[0] == 'L' && name[strlen (name) - 1] < 32)
-    return TRUE;
+  /* Treat assembler generated fake symbols, dollar local labels and
+     forward-backward labels (aka local labels) as locals.
+     These labels have the form:
+
+       L0^A.*                                  (fake symbols)
+
+       [.]?L[0123456789]+{^A|^B}[0123456789]*  (local labels)
+
+     Versions which start with .L will have already been matched above,
+     so we only need to match the rest.  */
+  if (name[0] == 'L' && ISDIGIT (name[1]))
+    {
+      bfd_boolean ret = FALSE;
+      const char * p;
+      char c;
+
+      for (p = name + 2; (c = *p); p++)
+	{
+	  if (c == 1 || c == 2)
+	    {
+	      if (c == 1 && p == name + 2)
+		/* A fake symbol.  */
+		return TRUE;
+
+	      /* FIXME: We are being paranoid here and treating symbols like
+		 L0^Bfoo as if there were non-local, on the grounds that the
+		 assembler will never generate them.  But can any symbol
+		 containing an ASCII value in the range 1-31 ever be anything
+		 other than some kind of local ?  */
+	      ret = TRUE;
+	    }
+
+	  if (! ISDIGIT (c))
+	    {
+	      ret = FALSE;
+	      break;
+	    }
+	}
+      return ret;
+    }

    return FALSE;
  }

Re: Commit: _bfd_is_local_label_name: Include assembler local labels

[Nicholas Clifton]

Hi Guys,

On 08/04/15 17:31, Nicholas Clifton wrote:

> Whilst rechecking the patch, I also came across assembler generated fake
> symbol names, which are also supposed to be treated as locals.  So,
> before I check it in, what do you guys think of this reworking of the
> assembler local symbol patch ?

I have now checked in this patch, along with this changelog entry.

Cheers
   Nick

bfd/ChangeLog
2015-04-24  Nick Clifton  <nickc@redhat.com>

	* elf.c (_bfd_elf_is_local_label_name): Extend test for assembler
	local labels to include local labels with a numeric suffix and
	fake symbols.