Re: [PATCH RFC] RISC-V: alter the special character used in FAKE_LABEL_NAME

[Jan Beulich <jbeulich@suse.com>]

Nick,

On 13.03.2023 15:25, Nick Clifton wrote:
>> What I don't think you addressed is my 2nd observation wrt quoted symbol
>> names. There all characters can appear in symbol names, and hence
>> deriving "local" (or not) based on symbol name looks wrong to me.
> 
> Ok, that is fair.  Let's see.  So the issue is, what should S_IS_LOCAL do
> if a user provides a global, quoted symbol name that includes characters
> that match DOLLAR_LABEL_CHAR, LOCAL_LABEL_CHAR or FAKE_LABEL_CHAR, right ?
> 
> I would imagine that this is an extremely unusual case, which is why it has
> not come up before.
> 
> First off, I wonder why S_IS_LOCAL does not check to see if BSF_GLOBAL is
> set in flags and return false if that is the case.  If it did, I think that
> that would solve the problem.  I cannot think of any situation where you
> could have a global symbol that needs to be treated as local for the purposes
> of S_IS_LOCAL.

Well - fundamentally I expect there to be a reason for this name checking.
It may of course simply be a remnant of non-BFD-gas times. It could also
be that this works around certain supposedly-locals being marked global by
mistake. It's also not clear whether e.g. the comment in write_object_file()
regarding the relation of S_IS_LOCAL() and S_IS_EXTERNAL() is merely writing
down the observation of S_IS_LOCAL() does, or whether it means to point out
that this has to be that way for a reason (which then sadly isn't mentioned).

I wonder therefore whether S_IS_LOCAL() and S_IS_EXTERNAL() can't be made
true opposites of one another (with perhaps one of the two simply expanding
to a call to the other, inverting the result).

> Alternatively, maybe it would be simpler to just document that quoted
> symbol names that contain special characters might be treated as if they
> were local symbols, and leave it up to the user to decide whether they
> want to risk such behaviour.  Not very user friendly, but it would match
> the current behaviour.  We could also add a warning to read_symbol_name()
> and get_symbol_name() to alert users to the problem.

That's not good imo, as it might end in surprises / perceived regressions:
We couldn't easily change the special characters we use, or add another one
if needed. So I guess ...

> What do you think ?  Any preferences ?

... my preference is clear - see about making S_IS_LOCAL() and
S_IS_EXTERNAL() as close opposites of one another as possible, if no other
reason for their differences is known. I can give that a try, and see what
running the testsuite says. Yet even if that didn't raise any problems,
there'd be uncertainty left due to insufficient testsuite coverage on many
targets.

Jan