[PATCH 3/3] gas: Add a macro test with expression argument

H.J. Lu hjl.tools@gmail.com
Mon Aug 12 11:21:34 GMT 2024


On Mon, Aug 12, 2024 at 12:39 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 12.08.2024 01:11, H.J. Lu wrote:
> > --- /dev/null
> > +++ b/gas/testsuite/gas/macros/arg1.s
> > @@ -0,0 +1,7 @@
> > +     .macro test arg1
> > +     .byte \arg1
> > +     .endm
> > +
> > +     .data
> > +     test  0x10 + 0
> > +     test  0x10 + 1
>
> So this is precisely what that entire change was about that you're proposing
> to revert. Within binutils, instances of such broken macro invocations were
> deliberately fixed up front. And the NEWS entry the commit added also is very
> clear about the above no longer working (and never having been guaranteed to
> work).
>
> Just look at it the way it is textually present above: Knowing that macro
> arguments don't require commas as separators, how many arguments do you see?
> And no, using knowledge on the internal workings (brokenness) of the scrubber
> is not allowed to determine the answer. (My answer: Three. And that's what
> gas also should determine.)
>
> If, purely from a practical / pragmatic perspective we'd really need to keep
> the above working for some more time, then I expect we'll need to invent a
> mode within which we can warn about such broken macros, telling people that
> new behavior will be enforced in, say, the next release. Otherwise how do
> you propose we ever address (without heuristics) the issues that the changes
> at hand are actually aiming at addressing?
>
> I did actually think about possible transitional states. Yet I didn't figure
> any that would be halfway sane _and_ useful. For example, while we could add
> a command line option to request old vs new scrubbing modes. To be useful
> (for the purpose of fully transitioning sooner or later), that ought to
> default to "new", though. Yet then people will need to fix their code anyway,
> just (possibly) by adding the new command line option instead of touching
> assembly sources.
>
> And then: I deliberately waited for comments much longer than I would usually
> have done. No-one really cared to comment on the changing behavior. And hence
> I'm a little irritated that now not just a possible workaround is suggested,
> but outright reverting.
>
> Bottom line: Clearly a NAK for this testsuite addition. We must not test
> accepting input in ways other than (more or less) documented. (Documentation
> isn't great for macros, but the possibility of not using commas to separate
> parameters and arguments can at least be derived from reading what's there.)
> As to reverting the two commits while working out a possible transition path:
> I'm open to that, provided some constructive comments actually surface, so
> we / I have a way forward.
>
> Jan

At very least, we must support the old behavior with a command-line
option which can be used with the testcase.

-- 
H.J.


More information about the Binutils mailing list