[PATCH 3/4] ELF: SHF_STRINGS isn't really tied to SHF_MERGE

[Jan Beulich]

On 17.01.2025 11:30, Richard Earnshaw (lists) wrote:
> On 17/01/2025 07:19, Jan Beulich wrote:
>> On 16.01.2025 19:27, Richard Earnshaw wrote:
>>> 	  else if ((attr & (SHF_MERGE | SHF_STRINGS)) != 0)
>>> 	    {
>>> 	      as_warn (_("entity size for SHF_MERGE / SHF_STRINGS not specified"));
>>> 	      attr &= ~(SHF_MERGE | SHF_STRINGS);
>>> 	    }
>>>
>>>
>>> how about, if instead we had
>>>
>>> 	  else if ((attr & SHF_MERGE) != 0)
>>> 	    {
>>> 	      as_warn (_("entity size for SHF_MERGE not specified"));
>>> 	      attr &= ~(SHF_MERGE | SHF_STRINGS);
>>> 	    }
>>> 	  else if ((attr & SHF_STRINGS) != 0)
>>> 	    {
>>>                 as_tsktsk (_"assuming entity size of 1 for SHF_STRINGS"));
>>> 	      entsize = 1;
>>> 	    }
>>>
>>> as_tsktsk is slightly weaker as a warning in that it doesn't cause
>>> assembler failure with --fatal-warnings.
>>
>> After thinking this over some more I was going to suggest something
>> similar, just not with as_tsktsk(): How about we introduce a separate
>> control for pedantic warnings (e.g. --ped-warn and --no-ped-warn), to
>> use here and maybe also for the @progbits defaulting (and anything
>> alike)?
> 
> That sounds like a pretty fundamental change to be making this late in 
> the development cycle.  I think we need something that can be fixed 
> before we branch and that sounds like quite a lot of additional change.

I wouldn't call it "fundamental", but a release being around the corner
is of course a relevant factor. Care to ...

>> However, while having a way to simply silence such warnings without
>> silencing all warning may be viewed as desirable in some cases, the
>> problem with downgrading such warnings further is that I'll then
>> expect people to complain that the assembler silently did something
>> the programmer didn't ask for. (I'd like to mention in this context
>> that I find it odd that while as_warn() can be silenced, as_tsktsk()
>> can't be. Hence why I didn't consider using it here.)
> 
> Yes, there are anomalies in the assembler behaviour, but tsktsk is 
> intended for cases where the assembler is pretty sure that it's 
> assumptions are safe or correct, whereas as_warn is intended for cases 
> where there is a guess, but it's very much a guess.

... submit a patch then? It seems clear now that we won't fully agree
on the underlying principles. Yet as indicated earlier, with suitable
justification I'm not intending to stand in the way of a patch like
you suggest it. I just rather would not like to write it like this
myself.

>> What to fall back to is an orthogonal question: I can see your solution
>> as being as good or bad as mine: Your suggestion would change assembler
>> behavior, too - so far we didn't set sh_entsize to non-zero in such
>> cases. The only thing I'll insist on is that we produce a spec-
>> conforming object file.
> 
> Yes, in that we previously produced an object file that conformed to hte 
> programmers source code, but not to the spec.  With your change it 
> no-longer produces something conforming to the source code (you've 
> cleared SHF_STRINGS) although it is conforming to the ELF spec.  That, 
> IMO is simply wrong.

I don't think you would have liked better if I had made the assembler
raise an error. IMO adding or removing information are equally wrong
(or right).

Jan