public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Palmer Dabbelt <palmer@dabbelt.com>
To: christoph.muellner@vrull.eu
Cc: jbeulich@suse.com, binutils@sourceware.org,
	Andrew Waterman <andrew@sifive.com>,
	Jim Wilson <jim.wilson.gcc@gmail.com>,
	nelson@rivosinc.com
Subject: Re: [PATCH] RISC-V: fix build after "Add support for arbitrary immediate encoding formats"
Date: Fri, 30 Sep 2022 15:17:35 -0700 (PDT)	[thread overview]
Message-ID: <mhng-1c72f11a-8424-45bd-b4c5-2588ef505222@palmer-ri-x1c9> (raw)
In-Reply-To: <CAEg0e7i+2U1oYnxuCf=n9-+dtXOk+dC-8m=9nboue-2C=NmGig@mail.gmail.com>

On Fri, 30 Sep 2022 03:26:29 PDT (-0700), christoph.muellner@vrull.eu wrote:
> Hi Jan,
>
> Thanks for pointing that out!
>
> BR
> Christoph
>
> On Fri, Sep 30, 2022 at 11:41 AM Jan Beulich <jbeulich@suse.com> wrote:
>
>> Pre- and post-increment/decrement are side effects, the behavior of
>> which is undefined when combined with passing an address of the accessed
>> variable in the same function invocation. There's no need for the
>> increments here - simply adding 1 achieves the intended effect without
>> triggering compiler diagnostics (which are fatal with -Werror).
>> ---
>> Committing as obvious.

Thanks, sorry we missed this one.

That said: I'd argue that the sscanf() version of these routines were 
much less prone to these sorts of string parsing issues, I re-wrote 
these because I'd run into a handful of issues when trying to 
forward-port the original T-Head patches.  I don't remember exactly what 
the issues were, though (sorry if I missed something during the patch 
review, I've been pretty out of it over the last few weeks).

>>
>> --- a/gas/config/tc-riscv.c
>> +++ b/gas/config/tc-riscv.c
>> @@ -1287,10 +1287,10 @@ validate_riscv_insn (const struct riscv_
>>                 case 'u': /* 'XuN@S' ... N-bit unsigned immediate at bit
>> S.  */
>>                   goto use_imm;
>>                 use_imm:
>> -                 n = strtol (++oparg, (char **)&oparg, 10);
>> +                 n = strtol (oparg + 1, (char **)&oparg, 10);
>>                   if (*oparg != '@')
>>                     goto unknown_validate_operand;
>> -                 s = strtol (++oparg, (char **)&oparg, 10);
>> +                 s = strtol (oparg + 1, (char **)&oparg, 10);
>>                   oparg--;
>>
>>                   USE_IMM (n, s);
>> @@ -3327,10 +3327,10 @@ riscv_ip (char *str, struct riscv_cl_ins
>>                       sign = false;
>>                       goto parse_imm;
>>                     parse_imm:
>> -                     n = strtol (++oparg, (char **)&oparg, 10);
>> +                     n = strtol (oparg + 1, (char **)&oparg, 10);
>>                       if (*oparg != '@')
>>                         goto unknown_riscv_ip_operand;
>> -                     s = strtol (++oparg, (char **)&oparg, 10);
>> +                     s = strtol (oparg + 1, (char **)&oparg, 10);
>>                       oparg--;
>>
>>                       my_getExpression (imm_expr, asarg);
>> --- a/opcodes/riscv-dis.c
>> +++ b/opcodes/riscv-dis.c
>> @@ -586,10 +586,10 @@ print_insn_args (const char *oparg, insn
>>                   sign = false;
>>                   goto print_imm;
>>                 print_imm:
>> -                 n = strtol (++oparg, (char **)&oparg, 10);
>> +                 n = strtol (oparg + 1, (char **)&oparg, 10);
>>                   if (*oparg != '@')
>>                     goto undefined_modifier;
>> -                 s = strtol (++oparg, (char **)&oparg, 10);
>> +                 s = strtol (oparg + 1, (char **)&oparg, 10);
>>                   oparg--;
>>
>>                   if (!sign)
>>

  reply	other threads:[~2022-09-30 22:17 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-13 12:59 [PATCH 0/3] RISC-V: alias insn adjustments Jan Beulich
2022-09-13 13:02 ` [PATCH 1/3] RISC-V: re-arrange opcode table for consistent alias handling Jan Beulich
2022-09-15  2:30   ` Nelson Chu
2022-09-15  7:42     ` Jan Beulich
2022-09-16  9:53       ` Nelson Chu
2023-07-11 21:02         ` Fangrui Song
     [not found]         ` <DS7PR12MB576580071090C394AECFC618CB31A@DS7PR12MB5765.namprd12.prod.outlook.com>
2023-07-12  8:15           ` Jan Beulich
2023-07-14 21:25             ` Fangrui Song
     [not found]             ` <MN0PR12MB57613B59178FAADE5FCD3837CB34A@MN0PR12MB5761.namprd12.prod.outlook.com>
2023-07-14 22:08               ` Stefan O'Rear
2023-07-17  6:50                 ` Jan Beulich
2023-07-21 22:16                   ` Song Fangrui
2023-07-22 15:14                     ` Jeff Law
2023-07-22 16:55                       ` Andrew Waterman
     [not found]                   ` <DS7PR12MB57658EF28577B41BF35C5E7CCB3FA@DS7PR12MB5765.namprd12.prod.outlook.com>
2023-07-24  7:23                     ` Jan Beulich
2023-08-30  3:14                       ` Fangrui Song
2022-09-13 13:03 ` [PATCH 2/3] RISC-V: drop stray INSN_ALIAS flags Jan Beulich
2022-09-15  2:43   ` Nelson Chu
2022-09-13 13:04 ` [PATCH 3/3] RISC-V: add alias for SLLI.UW Jan Beulich
2022-09-13 14:54 ` [PATCH 0/3] RISC-V: alias insn adjustments Tsukasa OI
2022-09-13 16:11   ` Jan Beulich
2022-09-13 16:58     ` Tsukasa OI
2022-09-14  6:26       ` Jan Beulich
2022-09-30  9:41 ` [PATCH] RISC-V: fix build after "Add support for arbitrary immediate encoding formats" Jan Beulich
2022-09-30 10:26   ` Christoph Müllner
2022-09-30 22:17     ` Palmer Dabbelt [this message]
2022-09-30  9:42 ` [PATCH] RISC-V: fallout from "re-arrange opcode table for consistent alias handling" Jan Beulich
2022-09-30  9:42 ` [PATCH] RISC-V: don't cast expressions' X_add_number to long in diagnostics Jan Beulich
2022-09-30 10:13   ` Christoph Müllner
2022-09-30 14:43   ` Nelson Chu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=mhng-1c72f11a-8424-45bd-b4c5-2588ef505222@palmer-ri-x1c9 \
    --to=palmer@dabbelt.com \
    --cc=andrew@sifive.com \
    --cc=binutils@sourceware.org \
    --cc=christoph.muellner@vrull.eu \
    --cc=jbeulich@suse.com \
    --cc=jim.wilson.gcc@gmail.com \
    --cc=nelson@rivosinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).