Re: [PATCH v4 10/12] RISC-V/gas: warn about non-power-of-2 stack-align attribute

Nelson Chu <[email protected]> Mon, 27 Jul 2026 12:17:11 +0800
Newsgroups gmane.comp.gnu.binutils
Message-ID <CAHT-pJx8oAHN8h1OOexCKw5y3Qd+Xu4=EDnO9foSG70PncrNZg@mail.gmail.com>
On Fri, Jul 24, 2026 at 6:06 PM Jan Beulich <[email protected]> wrote:
>
> Only power-of-2 values are sensible for alignment.
>
> Reviewed-by: Jiawei <[email protected]>
> ---
> Should we reject changing an already set non-zero value, like
> riscv_merge_attributes() does?

The key lies in whether we care whether the previous elf attribute
settings still hold true.

* For elf architecture attributes,
Before assembling instructions: we don't care.
After assembling instructions: we care, but assuming the file-level
setting should represent the entire code, unless the function-level
settings intervene, we added the limitation to make sure it won't
change after assembling instructions.

* For priv-spec attributes
Before assembling instructions and csrs: we don't care.
After assembling instructions and csrs: we care.

* Otherwise
They shouldn't affect assembling any instructions, so we don't care.

Personally, for these non-arch and non-priv spec attributes, holding
true or not holding true for their previous settings are both right
and fine.  I don't have a strong opinion on this.

Thanks
Nelson

>
> Should we permit 0 as an explicit indication of "attribute not set"?

This probably needs to check what psABI defined what the 0 value
meant.  If they don't define, then I think we can treat 0 as "the
attribute not set".

> ---
> v2: New.
>
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -6137,6 +6137,13 @@ s_riscv_attribute (int ignored ATTRIBUTE
>                    "any instructions"));
>        break;
>
> +    case Tag_RISCV_stack_align:
> +      if (!attr[Tag_RISCV_stack_align].i
> +         || (attr[Tag_RISCV_stack_align].i &
> +             (attr[Tag_RISCV_stack_align].i - 1)))
> +       as_warn (_("`stack_align' attribute with non-power-of-2 value"));
> +      break;
> +
>      case Tag_RISCV_unaligned_access:
>        if (attr[Tag_RISCV_unaligned_access].i > 1)
>         as_warn (_("`unaligned_access' attribute with non-boolean value"));
> --- a/gas/testsuite/gas/riscv/attribute-warn.d
> +++ b/gas/testsuite/gas/riscv/attribute-warn.d
> @@ -4,4 +4,5 @@
>
>  Attribute Section: riscv
>  File Attributes
> +  Tag_RISCV_stack_align: 65536-bytes
>    Tag_RISCV_arch: [a-zA-Z0-9_\"].*
> --- a/gas/testsuite/gas/riscv/attribute-warn.l
> +++ b/gas/testsuite/gas/riscv/attribute-warn.l
> @@ -1,2 +1,4 @@
>  .*: Assembler messages:
>  .*:2: Warning: `unaligned_access' attribute with non-boolean value
> +.*:5: Warning: `stack_align' attribute with non-power-of-2 value
> +.*:6: Warning: `stack_align' attribute with non-power-of-2 value
> --- a/gas/testsuite/gas/riscv/attribute-warn.s
> +++ b/gas/testsuite/gas/riscv/attribute-warn.s
> @@ -1,3 +1,7 @@
>         .attribute unaligned_access, 1
>         .attribute unaligned_access, 0x1234
>         .attribute unaligned_access, 0
> +
> +       .attribute stack_align, 0
> +       .attribute stack_align, 0x4444
> +       .attribute stack_align, 0x10000
>