[PATCH] mips: Avoid a form of the .type directive that is not supported by LLVM's Integrated Assembler

[PATCH] mips: Avoid a form of the .type directive that is not supported by LLVM's Integrated Assembler

[Daniel Sanders]

The target independent parts of the LLVM Lexer considers 'fault@function'
to be a single token representing the 'fault' symbol with a 'function'
modifier. However, this is not the case in the .type directive where
'function' refers to STT_FUNC from the ELF standard.

This is the only example of this form of '.type' that we are aware of in
MIPS source so we'd prefer to make this small source change rather than
complicate the target independent parts of LLVM's assembly lexer with
directive and/or target specific exceptions to the lexing rules.

Signed-off-by: Scott Egerton <Scott.Egerton@imgtec.com>
Signed-off-by: Daniel Sanders <daniel.sanders@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: Markos Chandras <markos.chandras@imgtec.com>
Cc: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
Cc: linux-mips@linux-mips.org

---
 arch/mips/kernel/r4k_fpu.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/kernel/r4k_fpu.S b/arch/mips/kernel/r4k_fpu.S
index f09546e..17732f8 100644
--- a/arch/mips/kernel/r4k_fpu.S
+++ b/arch/mips/kernel/r4k_fpu.S
@@ -358,7 +358,7 @@ LEAF(_restore_msa_all_upper)
 
 	.set	reorder
 
-	.type	fault@function
+	.type	fault, @function
 	.ent	fault
 fault:	li	v0, -EFAULT				# failure
 	jr	ra
-- 
2.1.4

[PATCH] mips: Avoid a form of the .type directive that is not supported by LLVM's Integrated Assembler

[Daniel Sanders]

The target independent parts of the LLVM Lexer considers 'fault@function'
to be a single token representing the 'fault' symbol with a 'function'
modifier. However, this is not the case in the .type directive where
'function' refers to STT_FUNC from the ELF standard.

This is the only example of this form of '.type' that we are aware of in
MIPS source so we'd prefer to make this small source change rather than
complicate the target independent parts of LLVM's assembly lexer with
directive and/or target specific exceptions to the lexing rules.

Signed-off-by: Scott Egerton <Scott.Egerton@imgtec.com>
Signed-off-by: Daniel Sanders <daniel.sanders@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: Markos Chandras <markos.chandras@imgtec.com>
Cc: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
Cc: linux-mips@linux-mips.org

---
 arch/mips/kernel/r4k_fpu.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/kernel/r4k_fpu.S b/arch/mips/kernel/r4k_fpu.S
index f09546e..17732f8 100644
--- a/arch/mips/kernel/r4k_fpu.S
+++ b/arch/mips/kernel/r4k_fpu.S
@@ -358,7 +358,7 @@ LEAF(_restore_msa_all_upper)
 
 	.set	reorder
 
-	.type	fault@function
+	.type	fault, @function
 	.ent	fault
 fault:	li	v0, -EFAULT				# failure
 	jr	ra
-- 
2.1.4

Re: [PATCH] mips: Avoid a form of the .type directive that is not supported by LLVM's Integrated Assembler

[Maciej W. Rozycki]

Hi Daniel,

 Please try to fit your patch summary (subject) in 75 characters to avoid 
line wrapping in GIT.

> The target independent parts of the LLVM Lexer considers 'fault@function'
> to be a single token representing the 'fault' symbol with a 'function'
> modifier. However, this is not the case in the .type directive where
> 'function' refers to STT_FUNC from the ELF standard.

 If LLVM strives to be GNU toolchain compatible, then this looks like a 
bug in their scanner as generic ELF support in GAS (gas/config/obj-elf.c) 
has this, in `obj_elf_type':

  if (*input_line_pointer == ',')
    ++input_line_pointer;

so the comma is entirely optional.  I realise this is undocumented, but 
there you go.  It must have been there since forever.

> This is the only example of this form of '.type' that we are aware of in
> MIPS source so we'd prefer to make this small source change rather than
> complicate the target independent parts of LLVM's assembly lexer with
> directive and/or target specific exceptions to the lexing rules.

 So this has nothing to do with the MIPS target really.

 As to the change itself I agree it seems rather pointless to have this 
single oddity, which I suspect has been a finger slip which has only 
survived because GAS is happy to accept this form.  So:

Reviewed-by: Maciej W. Rozycki <macro@imgtec.com>

although please make the same change to arch/mips/kernel/r2300_fpu.S (the 
same patch should apply there cleanly) for consistency and resend with the 
last paragraph rewritten so as not to confuse people this has anything to 
do with our target.

 For the record this was introduced with commit 0ae8dceaebe3 ("Merge with 
2.3.10.").

  Maciej

Re: [PATCH] mips: Avoid a form of the .type directive that is not supported by LLVM's Integrated Assembler

[Maciej W. Rozycki]

Hi Daniel,

 Please try to fit your patch summary (subject) in 75 characters to avoid 
line wrapping in GIT.

> The target independent parts of the LLVM Lexer considers 'fault@function'
> to be a single token representing the 'fault' symbol with a 'function'
> modifier. However, this is not the case in the .type directive where
> 'function' refers to STT_FUNC from the ELF standard.

 If LLVM strives to be GNU toolchain compatible, then this looks like a 
bug in their scanner as generic ELF support in GAS (gas/config/obj-elf.c) 
has this, in `obj_elf_type':

  if (*input_line_pointer == ',')
    ++input_line_pointer;

so the comma is entirely optional.  I realise this is undocumented, but 
there you go.  It must have been there since forever.

> This is the only example of this form of '.type' that we are aware of in
> MIPS source so we'd prefer to make this small source change rather than
> complicate the target independent parts of LLVM's assembly lexer with
> directive and/or target specific exceptions to the lexing rules.

 So this has nothing to do with the MIPS target really.

 As to the change itself I agree it seems rather pointless to have this 
single oddity, which I suspect has been a finger slip which has only 
survived because GAS is happy to accept this form.  So:

Reviewed-by: Maciej W. Rozycki <macro@imgtec.com>

although please make the same change to arch/mips/kernel/r2300_fpu.S (the 
same patch should apply there cleanly) for consistency and resend with the 
last paragraph rewritten so as not to confuse people this has anything to 
do with our target.

 For the record this was introduced with commit 0ae8dceaebe3 ("Merge with 
2.3.10.").

  Maciej

RE: [PATCH] mips: Avoid a form of the .type directive that is not supported by LLVM's Integrated Assembler

[Daniel Sanders]

Hi Maceij,

> Hi Daniel,
> Please try to fit your patch summary (subject) in 75 characters to avoid
> line wrapping in GIT.

Ok, I'll fix this in my next version.

> > The target independent parts of the LLVM Lexer considers 'fault@function'
> > to be a single token representing the 'fault' symbol with a 'function'
> > modifier. However, this is not the case in the .type directive where
> > 'function' refers to STT_FUNC from the ELF standard.
> 
>  If LLVM strives to be GNU toolchain compatible, then this looks like a
> bug in their scanner as generic ELF support in GAS (gas/config/obj-elf.c)
> has this, in `obj_elf_type':
> 
>   if (*input_line_pointer == ',')
>     ++input_line_pointer;
> 
> so the comma is entirely optional.  I realise this is undocumented, but
> there you go.  It must have been there since forever.

It's not just about the comma, the problem arises when there's nothing separating the symbol from the '@function'.
Adding a single whitespace is also sufficient to avoid the problem but we chose to add the comma as well.

> > This is the only example of this form of '.type' that we are aware of in
> > MIPS source so we'd prefer to make this small source change rather than
> > complicate the target independent parts of LLVM's assembly lexer with
> > directive and/or target specific exceptions to the lexing rules.
> 
>  So this has nothing to do with the MIPS target really.

I suppose it depends how you view it. You're correct that the underlying issue is probably relevant to more targets than just MIPS.
From my perspective this is a MIPS thing since my goal is to get the MIPS Integrated Assembler to a good enough state to be
able to make it our default assembler and on this occasion we're making a small change to MIPS-specific kernel sources to make
that easier.

I'll rewrite the last paragraph anyway though.

>  As to the change itself I agree it seems rather pointless to have this
> single oddity, which I suspect has been a finger slip which has only
> survived because GAS is happy to accept this form.  So:
> 
> Reviewed-by: Maciej W. Rozycki <macro@imgtec.com>
> 
> although please make the same change to arch/mips/kernel/r2300_fpu.S (the
> same patch should apply there cleanly) for consistency and resend with the
> last paragraph rewritten so as not to confuse people this has anything to
> do with our target.
> 
>  For the record this was introduced with commit 0ae8dceaebe3 ("Merge with
> 2.3.10.").

Ok

>   Maciej

RE: [PATCH] mips: Avoid a form of the .type directive that is not supported by LLVM's Integrated Assembler

[Daniel Sanders]

Hi Maceij,

> Hi Daniel,
> Please try to fit your patch summary (subject) in 75 characters to avoid
> line wrapping in GIT.

Ok, I'll fix this in my next version.

> > The target independent parts of the LLVM Lexer considers 'fault@function'
> > to be a single token representing the 'fault' symbol with a 'function'
> > modifier. However, this is not the case in the .type directive where
> > 'function' refers to STT_FUNC from the ELF standard.
> 
>  If LLVM strives to be GNU toolchain compatible, then this looks like a
> bug in their scanner as generic ELF support in GAS (gas/config/obj-elf.c)
> has this, in `obj_elf_type':
> 
>   if (*input_line_pointer == ',')
>     ++input_line_pointer;
> 
> so the comma is entirely optional.  I realise this is undocumented, but
> there you go.  It must have been there since forever.

It's not just about the comma, the problem arises when there's nothing separating the symbol from the '@function'.
Adding a single whitespace is also sufficient to avoid the problem but we chose to add the comma as well.

> > This is the only example of this form of '.type' that we are aware of in
> > MIPS source so we'd prefer to make this small source change rather than
> > complicate the target independent parts of LLVM's assembly lexer with
> > directive and/or target specific exceptions to the lexing rules.
> 
>  So this has nothing to do with the MIPS target really.

I suppose it depends how you view it. You're correct that the underlying issue is probably relevant to more targets than just MIPS.

Re: [PATCH] mips: Avoid a form of the .type directive that is not supported by LLVM's Integrated Assembler

[Ralf Baechle]

On Wed, Feb 17, 2016 at 08:04:31PM +0000, Maciej W. Rozycki wrote:

>  If LLVM strives to be GNU toolchain compatible, then this looks like a 
> bug in their scanner as generic ELF support in GAS (gas/config/obj-elf.c) 
> has this, in `obj_elf_type':
> 
>   if (*input_line_pointer == ',')
>     ++input_line_pointer;
> 
> so the comma is entirely optional.  I realise this is undocumented, but 
> there you go.  It must have been there since forever.

It contradicts documentation.  The gas manual says:

* Type::                        `.type <INT | NAME , TYPE DESCRIPTION>'

And the SGI assembler manual I dug up as ".type name, value".  So maybe
gas is too generous here?

Either way, I think the patch is right and I've just applied v2.

  Ralf

Re: [PATCH] mips: Avoid a form of the .type directive that is not supported by LLVM's Integrated Assembler

[Maciej W. Rozycki]

On Wed, 24 Feb 2016, Ralf Baechle wrote:

> >  If LLVM strives to be GNU toolchain compatible, then this looks like a 
> > bug in their scanner as generic ELF support in GAS (gas/config/obj-elf.c) 
> > has this, in `obj_elf_type':
> > 
> >   if (*input_line_pointer == ',')
> >     ++input_line_pointer;
> > 
> > so the comma is entirely optional.  I realise this is undocumented, but 
> > there you go.  It must have been there since forever.
> 
> It contradicts documentation.  The gas manual says:
> 
> * Type::                        `.type <INT | NAME , TYPE DESCRIPTION>'
> 
> And the SGI assembler manual I dug up as ".type name, value".  So maybe
> gas is too generous here?

 I find it interesting that you mention SGI here as the commit which might 
be responsible for the current interpretation is this:

Mon Sep 12 17:51:39 1994  Ian Lance Taylor  (ian@sanguine.cygnus.com)

	* config/obj-elf.c (obj_elf_type): Rewrite to accept syntax
	reportedly to be used on Irix 6.

Given its age I doubt further information can be found, it might be just 
sloppy coding.

> Either way, I think the patch is right and I've just applied v2.

 Sure, thanks!

  Maciej

Re: [PATCH] mips: Avoid a form of the .type directive that is not supported by LLVM's Integrated Assembler

[Maciej W. Rozycki]

On Wed, 24 Feb 2016, Ralf Baechle wrote:

> >  If LLVM strives to be GNU toolchain compatible, then this looks like a 
> > bug in their scanner as generic ELF support in GAS (gas/config/obj-elf.c) 
> > has this, in `obj_elf_type':
> > 
> >   if (*input_line_pointer == ',')
> >     ++input_line_pointer;
> > 
> > so the comma is entirely optional.  I realise this is undocumented, but 
> > there you go.  It must have been there since forever.
> 
> It contradicts documentation.  The gas manual says:
> 
> * Type::                        `.type <INT | NAME , TYPE DESCRIPTION>'
> 
> And the SGI assembler manual I dug up as ".type name, value".  So maybe
> gas is too generous here?

 I find it interesting that you mention SGI here as the commit which might 
be responsible for the current interpretation is this:

Mon Sep 12 17:51:39 1994  Ian Lance Taylor  (ian@sanguine.cygnus.com)

	* config/obj-elf.c (obj_elf_type): Rewrite to accept syntax
	reportedly to be used on Irix 6.

Given its age I doubt further information can be found, it might be just 
sloppy coding.

> Either way, I think the patch is right and I've just applied v2.

 Sure, thanks!

  Maciej

RE: [PATCH] mips: Avoid a form of the .type directive that is not supported by LLVM's Integrated Assembler

[Daniel Sanders]

Thanks
________________________________________
From: Ralf Baechle [ralf@linux-mips.org]
Sent: 24 February 2016 01:50
To: Maciej Rozycki
Cc: Daniel Sanders; Scott Egerton; Paul Burton; Markos Chandras; Leonid Yegoshin; linux-mips@linux-mips.org
Subject: Re: [PATCH] mips: Avoid a form of the .type directive that is not supported by LLVM's Integrated Assembler

On Wed, Feb 17, 2016 at 08:04:31PM +0000, Maciej W. Rozycki wrote:

>  If LLVM strives to be GNU toolchain compatible, then this looks like a
> bug in their scanner as generic ELF support in GAS (gas/config/obj-elf.c)
> has this, in `obj_elf_type':
>
>   if (*input_line_pointer == ',')
>     ++input_line_pointer;
>
> so the comma is entirely optional.  I realise this is undocumented, but
> there you go.  It must have been there since forever.

It contradicts documentation.  The gas manual says:

* Type::                        `.type <INT | NAME , TYPE DESCRIPTION>'

And the SGI assembler manual I dug up as ".type name, value".  So maybe
gas is too generous here?

Either way, I think the patch is right and I've just applied v2.

  Ralf