public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] gengtype: do not skip char after escape sequnce
@ 2022-05-04 19:14 Martin Liška
  2022-05-04 19:38 ` Iain Sandoe
  2022-06-01 15:14 ` Jeff Law
  0 siblings, 2 replies; 6+ messages in thread
From: Martin Liška @ 2022-05-04 19:14 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andreas Schwab

Right now, when a \$x escape sequence occures, the
next character after $x is skipped, which is bogus.

The code has very low coverage right now.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

	* gengtype-state.cc (read_a_state_token): Do not skip extra
	character after escaped sequence.
---
 gcc/gengtype-state.cc | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/gcc/gengtype-state.cc b/gcc/gengtype-state.cc
index dfd9ea52785..2dfe8edf1a5 100644
--- a/gcc/gengtype-state.cc
+++ b/gcc/gengtype-state.cc
@@ -473,43 +473,33 @@ read_a_state_token (void)
 		{
 		case 'a':
 		  obstack_1grow (&bstring_obstack, '\a');
-		  getc (state_file);
 		  break;
 		case 'b':
 		  obstack_1grow (&bstring_obstack, '\b');
-		  getc (state_file);
 		  break;
 		case 't':
 		  obstack_1grow (&bstring_obstack, '\t');
-		  getc (state_file);
 		  break;
 		case 'n':
 		  obstack_1grow (&bstring_obstack, '\n');
-		  getc (state_file);
 		  break;
 		case 'v':
 		  obstack_1grow (&bstring_obstack, '\v');
-		  getc (state_file);
 		  break;
 		case 'f':
 		  obstack_1grow (&bstring_obstack, '\f');
-		  getc (state_file);
 		  break;
 		case 'r':
 		  obstack_1grow (&bstring_obstack, '\r');
-		  getc (state_file);
 		  break;
 		case '"':
 		  obstack_1grow (&bstring_obstack, '\"');
-		  getc (state_file);
 		  break;
 		case '\\':
 		  obstack_1grow (&bstring_obstack, '\\');
-		  getc (state_file);
 		  break;
 		case ' ':
 		  obstack_1grow (&bstring_obstack, ' ');
-		  getc (state_file);
 		  break;
 		case 'x':
 		  {
-- 
2.36.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gengtype: do not skip char after escape sequnce
  2022-05-04 19:14 [PATCH] gengtype: do not skip char after escape sequnce Martin Liška
@ 2022-05-04 19:38 ` Iain Sandoe
  2022-05-05  7:55   ` Martin Liška
  2022-06-01 15:14 ` Jeff Law
  1 sibling, 1 reply; 6+ messages in thread
From: Iain Sandoe @ 2022-05-04 19:38 UTC (permalink / raw)
  To: Martin Liska; +Cc: GCC Patches, Andreas Schwab



> On 4 May 2022, at 20:14, Martin Liška <mliska@suse.cz> wrote:
> 
> Right now, when a \$x escape sequence occures, the
> next character after $x is skipped, which is bogus.
> 
> The code has very low coverage right now.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed?

… just curious ...

Is there no way to test this?
or to identify a target where the behaviour would be changed with/without the patch?
(and confirm the expected result).

thanks
Iain


> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 	* gengtype-state.cc (read_a_state_token): Do not skip extra
> 	character after escaped sequence.
> ---
> gcc/gengtype-state.cc | 10 ----------
> 1 file changed, 10 deletions(-)
> 
> diff --git a/gcc/gengtype-state.cc b/gcc/gengtype-state.cc
> index dfd9ea52785..2dfe8edf1a5 100644
> --- a/gcc/gengtype-state.cc
> +++ b/gcc/gengtype-state.cc
> @@ -473,43 +473,33 @@ read_a_state_token (void)
> 		{
> 		case 'a':
> 		  obstack_1grow (&bstring_obstack, '\a');
> -		  getc (state_file);
> 		  break;
> 		case 'b':
> 		  obstack_1grow (&bstring_obstack, '\b');
> -		  getc (state_file);
> 		  break;
> 		case 't':
> 		  obstack_1grow (&bstring_obstack, '\t');
> -		  getc (state_file);
> 		  break;
> 		case 'n':
> 		  obstack_1grow (&bstring_obstack, '\n');
> -		  getc (state_file);
> 		  break;
> 		case 'v':
> 		  obstack_1grow (&bstring_obstack, '\v');
> -		  getc (state_file);
> 		  break;
> 		case 'f':
> 		  obstack_1grow (&bstring_obstack, '\f');
> -		  getc (state_file);
> 		  break;
> 		case 'r':
> 		  obstack_1grow (&bstring_obstack, '\r');
> -		  getc (state_file);
> 		  break;
> 		case '"':
> 		  obstack_1grow (&bstring_obstack, '\"');
> -		  getc (state_file);
> 		  break;
> 		case '\\':
> 		  obstack_1grow (&bstring_obstack, '\\');
> -		  getc (state_file);
> 		  break;
> 		case ' ':
> 		  obstack_1grow (&bstring_obstack, ' ');
> -		  getc (state_file);
> 		  break;
> 		case 'x':
> 		  {
> -- 
> 2.36.0
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gengtype: do not skip char after escape sequnce
  2022-05-04 19:38 ` Iain Sandoe
@ 2022-05-05  7:55   ` Martin Liška
  2022-05-24  8:45     ` Martin Liška
  0 siblings, 1 reply; 6+ messages in thread
From: Martin Liška @ 2022-05-05  7:55 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: GCC Patches, Andreas Schwab

On 5/4/22 21:38, Iain Sandoe wrote:
> 
> 
>> On 4 May 2022, at 20:14, Martin Liška <mliska@suse.cz> wrote:
>>
>> Right now, when a \$x escape sequence occures, the
>> next character after $x is skipped, which is bogus.
>>
>> The code has very low coverage right now.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
> 
> … just curious ...
> 
> Is there no way to test this?

There is and as mentioned, I verified the current behavior is wrong for one
case where '\n' is being handled.

> or to identify a target where the behaviour would be changed with/without the patch?
> (and confirm the expected result).

I've done that.

Martin

> 
> thanks
> Iain
> 
> 
>> Thanks,
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 	* gengtype-state.cc (read_a_state_token): Do not skip extra
>> 	character after escaped sequence.
>> ---
>> gcc/gengtype-state.cc | 10 ----------
>> 1 file changed, 10 deletions(-)
>>
>> diff --git a/gcc/gengtype-state.cc b/gcc/gengtype-state.cc
>> index dfd9ea52785..2dfe8edf1a5 100644
>> --- a/gcc/gengtype-state.cc
>> +++ b/gcc/gengtype-state.cc
>> @@ -473,43 +473,33 @@ read_a_state_token (void)
>> 		{
>> 		case 'a':
>> 		  obstack_1grow (&bstring_obstack, '\a');
>> -		  getc (state_file);
>> 		  break;
>> 		case 'b':
>> 		  obstack_1grow (&bstring_obstack, '\b');
>> -		  getc (state_file);
>> 		  break;
>> 		case 't':
>> 		  obstack_1grow (&bstring_obstack, '\t');
>> -		  getc (state_file);
>> 		  break;
>> 		case 'n':
>> 		  obstack_1grow (&bstring_obstack, '\n');
>> -		  getc (state_file);
>> 		  break;
>> 		case 'v':
>> 		  obstack_1grow (&bstring_obstack, '\v');
>> -		  getc (state_file);
>> 		  break;
>> 		case 'f':
>> 		  obstack_1grow (&bstring_obstack, '\f');
>> -		  getc (state_file);
>> 		  break;
>> 		case 'r':
>> 		  obstack_1grow (&bstring_obstack, '\r');
>> -		  getc (state_file);
>> 		  break;
>> 		case '"':
>> 		  obstack_1grow (&bstring_obstack, '\"');
>> -		  getc (state_file);
>> 		  break;
>> 		case '\\':
>> 		  obstack_1grow (&bstring_obstack, '\\');
>> -		  getc (state_file);
>> 		  break;
>> 		case ' ':
>> 		  obstack_1grow (&bstring_obstack, ' ');
>> -		  getc (state_file);
>> 		  break;
>> 		case 'x':
>> 		  {
>> -- 
>> 2.36.0
>>
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gengtype: do not skip char after escape sequnce
  2022-05-05  7:55   ` Martin Liška
@ 2022-05-24  8:45     ` Martin Liška
  0 siblings, 0 replies; 6+ messages in thread
From: Martin Liška @ 2022-05-24  8:45 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: GCC Patches, Andreas Schwab, Richard Biener

PING^1

On 5/5/22 09:55, Martin Liška wrote:
> On 5/4/22 21:38, Iain Sandoe wrote:
>>
>>
>>> On 4 May 2022, at 20:14, Martin Liška <mliska@suse.cz> wrote:
>>>
>>> Right now, when a \$x escape sequence occures, the
>>> next character after $x is skipped, which is bogus.
>>>
>>> The code has very low coverage right now.
>>>
>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>
>>> Ready to be installed?
>>
>> … just curious ...
>>
>> Is there no way to test this?
> 
> There is and as mentioned, I verified the current behavior is wrong for one
> case where '\n' is being handled.
> 
>> or to identify a target where the behaviour would be changed with/without the patch?
>> (and confirm the expected result).
> 
> I've done that.
> 
> Martin
> 
>>
>> thanks
>> Iain
>>
>>
>>> Thanks,
>>> Martin
>>>
>>> gcc/ChangeLog:
>>>
>>> 	* gengtype-state.cc (read_a_state_token): Do not skip extra
>>> 	character after escaped sequence.
>>> ---
>>> gcc/gengtype-state.cc | 10 ----------
>>> 1 file changed, 10 deletions(-)
>>>
>>> diff --git a/gcc/gengtype-state.cc b/gcc/gengtype-state.cc
>>> index dfd9ea52785..2dfe8edf1a5 100644
>>> --- a/gcc/gengtype-state.cc
>>> +++ b/gcc/gengtype-state.cc
>>> @@ -473,43 +473,33 @@ read_a_state_token (void)
>>> 		{
>>> 		case 'a':
>>> 		  obstack_1grow (&bstring_obstack, '\a');
>>> -		  getc (state_file);
>>> 		  break;
>>> 		case 'b':
>>> 		  obstack_1grow (&bstring_obstack, '\b');
>>> -		  getc (state_file);
>>> 		  break;
>>> 		case 't':
>>> 		  obstack_1grow (&bstring_obstack, '\t');
>>> -		  getc (state_file);
>>> 		  break;
>>> 		case 'n':
>>> 		  obstack_1grow (&bstring_obstack, '\n');
>>> -		  getc (state_file);
>>> 		  break;
>>> 		case 'v':
>>> 		  obstack_1grow (&bstring_obstack, '\v');
>>> -		  getc (state_file);
>>> 		  break;
>>> 		case 'f':
>>> 		  obstack_1grow (&bstring_obstack, '\f');
>>> -		  getc (state_file);
>>> 		  break;
>>> 		case 'r':
>>> 		  obstack_1grow (&bstring_obstack, '\r');
>>> -		  getc (state_file);
>>> 		  break;
>>> 		case '"':
>>> 		  obstack_1grow (&bstring_obstack, '\"');
>>> -		  getc (state_file);
>>> 		  break;
>>> 		case '\\':
>>> 		  obstack_1grow (&bstring_obstack, '\\');
>>> -		  getc (state_file);
>>> 		  break;
>>> 		case ' ':
>>> 		  obstack_1grow (&bstring_obstack, ' ');
>>> -		  getc (state_file);
>>> 		  break;
>>> 		case 'x':
>>> 		  {
>>> -- 
>>> 2.36.0
>>>
>>
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gengtype: do not skip char after escape sequnce
  2022-05-04 19:14 [PATCH] gengtype: do not skip char after escape sequnce Martin Liška
  2022-05-04 19:38 ` Iain Sandoe
@ 2022-06-01 15:14 ` Jeff Law
  2022-06-16  6:22   ` Martin Liška
  1 sibling, 1 reply; 6+ messages in thread
From: Jeff Law @ 2022-06-01 15:14 UTC (permalink / raw)
  To: gcc-patches



On 5/4/2022 1:14 PM, Martin Liška wrote:
> Right now, when a \$x escape sequence occures, the
> next character after $x is skipped, which is bogus.
>
> The code has very low coverage right now.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> 	* gengtype-state.cc (read_a_state_token): Do not skip extra
> 	character after escaped sequence.
Any way you can add a test for this?  I'm OK with the patch as-is, but 
with a test is obviously better.

jeff


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gengtype: do not skip char after escape sequnce
  2022-06-01 15:14 ` Jeff Law
@ 2022-06-16  6:22   ` Martin Liška
  0 siblings, 0 replies; 6+ messages in thread
From: Martin Liška @ 2022-06-16  6:22 UTC (permalink / raw)
  To: Jeff Law, gcc-patches

On 6/1/22 17:14, Jeff Law via Gcc-patches wrote:
> 
> 
> On 5/4/2022 1:14 PM, Martin Liška wrote:
>> Right now, when a \$x escape sequence occures, the
>> next character after $x is skipped, which is bogus.
>>
>> The code has very low coverage right now.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
>> Thanks,
>> Martin
>>
>> gcc/ChangeLog:
>>
>>     * gengtype-state.cc (read_a_state_token): Do not skip extra
>>     character after escaped sequence.
> Any way you can add a test for this?

Don't know how to construct one.

Anyway, I'm going to push it :)

Martin


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-06-16  6:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-04 19:14 [PATCH] gengtype: do not skip char after escape sequnce Martin Liška
2022-05-04 19:38 ` Iain Sandoe
2022-05-05  7:55   ` Martin Liška
2022-05-24  8:45     ` Martin Liška
2022-06-01 15:14 ` Jeff Law
2022-06-16  6:22   ` Martin Liška

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).