x86: restrict gas'es recognition of -s to Solaris

Message ID c24e073a-2fde-4a5f-8ecb-61d204abb704@suse.com
State New
Headers
Series x86: restrict gas'es recognition of -s to Solaris |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Jan Beulich Nov. 21, 2024, 12:39 p.m. UTC
  When there for Solaris compatibility only, also recognize it only there.
This way the option becomes available for other possible uses.

While adjusting md_shortopts[], also re-arrange things such that we have
only a single, uniform definition of it.
---
Arguably -k should then also be limited to TE_FreeBSD.
  

Comments

Rainer Orth Nov. 21, 2024, 12:51 p.m. UTC | #1
Hi Jan,

> When there for Solaris compatibility only, also recognize it only there.
> This way the option becomes available for other possible uses.

I wonder if this is the best possible way to handle this:

* -s is only present for Solaris as compatiblity.  At least gcc doesn't
  use it at all.

* Besides, gcc has done away with Stabs and gdb is following suite (or
  has already), so Stabs become increasingly irrelevant.

* Keeping the (effectively unused) option on Solaris, but using it for
  something else would mean that `something else' isn't available on
  Solaris.  Just a guarantee for user confusion, I believe.

In short, I wonder if it wouldn't be better to just reject -s everywhere
instead.

WDYT?
	Rainer
  
Jan Beulich Nov. 21, 2024, 1:44 p.m. UTC | #2
On 21.11.2024 13:51, Rainer Orth wrote:
>> When there for Solaris compatibility only, also recognize it only there.
>> This way the option becomes available for other possible uses.
> 
> I wonder if this is the best possible way to handle this:
> 
> * -s is only present for Solaris as compatiblity.  At least gcc doesn't
>   use it at all.
> 
> * Besides, gcc has done away with Stabs and gdb is following suite (or
>   has already), so Stabs become increasingly irrelevant.
> 
> * Keeping the (effectively unused) option on Solaris, but using it for
>   something else would mean that `something else' isn't available on
>   Solaris.  Just a guarantee for user confusion, I believe.

That "something" could still be available there via long option. Solaris
is, imo, confusing enough already anyway, and hence it shouldn't come as a
surprise to anyone that some option may need spelling differently.

> In short, I wonder if it wouldn't be better to just reject -s everywhere
> instead.

If someone had proposed a patch to do so, I probably wouldn't have objected
(so long as a fair reason was provided). But personally I think this would
be going too far.

Jan
  
Rainer Orth Nov. 22, 2024, 9:08 a.m. UTC | #3
Jan Beulich <jbeulich@suse.com> writes:

> On 21.11.2024 13:51, Rainer Orth wrote:
>>> When there for Solaris compatibility only, also recognize it only there.
>>> This way the option becomes available for other possible uses.
>> 
>> I wonder if this is the best possible way to handle this:
>> 
>> * -s is only present for Solaris as compatiblity.  At least gcc doesn't
>>   use it at all.
>> 
>> * Besides, gcc has done away with Stabs and gdb is following suite (or
>>   has already), so Stabs become increasingly irrelevant.
>> 
>> * Keeping the (effectively unused) option on Solaris, but using it for
>>   something else would mean that `something else' isn't available on
>>   Solaris.  Just a guarantee for user confusion, I believe.
>
> That "something" could still be available there via long option. Solaris
> is, imo, confusing enough already anyway, and hence it shouldn't come as a
> surprise to anyone that some option may need spelling differently.

I'd have expected removing unnecessary differences would be considered
worthwhile, rather than cementing them.

>> In short, I wonder if it wouldn't be better to just reject -s everywhere
>> instead.
>
> If someone had proposed a patch to do so, I probably wouldn't have objected
> (so long as a fair reason was provided). But personally I think this would
> be going too far.

Your call: I'm busy with the upcoming GCC 15 release myself.

	Rainer
  

Patch

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -16706,11 +16706,14 @@  bool i386_record_operator (operatorT op,
 }
 #endif
 
+const char md_shortopts[] =
 #ifdef OBJ_ELF
-const char md_shortopts[] = "kVQ:sqnO::";
-#else
-const char md_shortopts[] = "qnO::";
+  "kVQ:"
+# ifdef TE_SOLARIS
+  "s"
+# endif
 #endif
+  "qnO::";
 
 #define OPTION_32 (OPTION_MD_BASE + 0)
 #define OPTION_64 (OPTION_MD_BASE + 1)
@@ -16834,10 +16837,12 @@  md_parse_option (int c, const char *arg)
     case 'k':
       break;
 
+# ifdef TE_SOLARIS
     case 's':
       /* -s: On i386 Solaris, this tells the native assembler to use
 	 .stab instead of .stab.excl.  We always use .stab anyhow.  */
       break;
+# endif
 
     case OPTION_MSHARED:
       shared = 1;