[PATCH] Add missing parens

[PATCH] Add missing parens

[Pascal Obry]

[-- Attachment #1: Type: text/plain, Size: 318 bytes --]


This fixes the unwinding on Win64.

ChangeLog:

2009-10-15  Pascal Obry  <obry@adacore.com>
             Eric Botcazou  <ebotcazou@adacore.com>

	* config/i386/cygming.h (DWARF_FRAME_REGNUM): Add enclosing parens.

Patch as attachement.

-- 
   Pascal Obry
   --
   gpg --keyserver keys.gnupg.net --recv-key F949BD3B

[-- Attachment #2: add-missing-parens.dif --]
[-- Type: video/x-dv, Size: 945 bytes --]

Re: [PATCH] Add missing parens

[Pascal Obry]

Le 18/10/2009 21:06, Pascal Obry a écrit :
> This fixes the unwinding on Win64.
>
> ChangeLog:
>
> 2009-10-15 Pascal Obry <obry@adacore.com>
> Eric Botcazou <ebotcazou@adacore.com>
>
> * config/i386/cygming.h (DWARF_FRAME_REGNUM): Add enclosing parens.
>
> Patch as attachement.

Ping! OK to commit?

-- 
   Pascal Obry
   --
   gpg --keyserver keys.gnupg.net --recv-key F949BD3B

Re: [PATCH] Add missing parens

[Dave Korn]

Pascal Obry wrote:
> Le 18/10/2009 21:06, Pascal Obry a écrit :
>> This fixes the unwinding on Win64.
>>
>> ChangeLog:
>>
>> 2009-10-15 Pascal Obry <obry@adacore.com>
>> Eric Botcazou <ebotcazou@adacore.com>
>>
>> * config/i386/cygming.h (DWARF_FRAME_REGNUM): Add enclosing parens.
>>
>> Patch as attachement.
> 
> Ping! OK to commit?

  I can't approve it so I'll just observe that this patch is blatantly
obviously correct and the right thing to do even if there wasn't a bug.

    cheers,
      DaveK

Re: [PATCH] Add missing parens

[Kai Tietz]

2009/10/20 Dave Korn <dave.korn.cygwin@googlemail.com>:
> Pascal Obry wrote:
>> Le 18/10/2009 21:06, Pascal Obry a écrit :
>>> This fixes the unwinding on Win64.
>>>
>>> ChangeLog:
>>>
>>> 2009-10-15 Pascal Obry <obry@adacore.com>
>>> Eric Botcazou <ebotcazou@adacore.com>
>>>
>>> * config/i386/cygming.h (DWARF_FRAME_REGNUM): Add enclosing parens.
>>>
>>> Patch as attachement.
>>
>> Ping! OK to commit?
>
>  I can't approve it so I'll just observe that this patch is blatantly
> obviously correct and the right thing to do even if there wasn't a bug.
>
>    cheers,
>      DaveK
>
>

Sorry for answering that late. Ok for apply.

Thanks,
Kai

-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

Re: [PATCH] Add missing parens

[Pascal Obry]

Dave,

>    I can't approve it so I'll just observe that this patch is blatantly
> obviously correct and the right thing to do even if there wasn't a bug.

There was a bug! The dwarf2 unwinding on x86-64 Windows was broken by this.

-- 
   Pascal Obry
   --
   gpg --keyserver keys.gnupg.net --recv-key F949BD3B

Re: [PATCH] Add missing parens

[Kai Tietz]

2009/10/20 Pascal Obry <obry@adacore.com>:
> Dave,
>
>>   I can't approve it so I'll just observe that this patch is blatantly
>> obviously correct and the right thing to do even if there wasn't a bug.
>
> There was a bug! The dwarf2 unwinding on x86-64 Windows was broken by this.
>
> --
>  Pascal Obry
>  --
>  gpg --keyserver keys.gnupg.net --recv-key F949BD3B
>
>
>

Ok, applied to trunk with some minor changes to line-length at revision 153033.

Thanks,
Kai

-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

Re: [PATCH] Add missing parens

[Pascal Obry]

Kai,

> Ok, applied to trunk with some minor changes to line-length at revision 153033.

Thanks.

-- 
   Pascal Obry
   --
   gpg --keyserver keys.gnupg.net --recv-key F949BD3B

Re: [PATCH] Add missing parens

[Eric Botcazou]

> Ok, applied to trunk with some minor changes to line-length at revision
> 153033.

Not clear why; morever they are not correct, the indentation is 2 spaces.

-- 
Eric Botcazou

Re: [PATCH] Add missing parens

[Kai Tietz]

2009/10/20 Eric Botcazou <ebotcazou@adacore.com>:
>> Ok, applied to trunk with some minor changes to line-length at revision
>> 153033.
>
> Not clear why; morever they are not correct, the indentation is 2 spaces.

The indentation I kept, if you want you can correct it. The issue is,
that using macro DWARF_FRAME_REGNUM in case of example in dwarf2out.c
' if (DWARF_FRAME_REGNUM (i) == DWARF_FRAME_RETURN_COLUMN)' could
leads to wrong code. As there is a semantic difference between
'if ((TARGET_64BIT ? dbx64_register_map[i] : svr4_dbx_register_map[i])
== DWARF_FRAME_RETURN_COLUMN)
and
if (TARGET_64BIT ? dbx64_register_map[i] : svr4_dbx_register_map[i] ==
DWARF_FRAME_RETURN_COLUMN)


Cheers,
Kai

-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

Re: [PATCH] Add missing parens

[Dave Korn]

Pascal Obry wrote:
> Dave,
> 
>>    I can't approve it so I'll just observe that this patch is blatantly
>> obviously correct and the right thing to do even if there wasn't a bug.
> 
> There was a bug! The dwarf2 unwinding on x86-64 Windows was broken by this.
> 

  Slightly unclear language on my part I guess; by saying that it was the
right thing to do "even if" there wasn't a bug, I was not saying that there
was not a bug, but was in fact implying that there was one!

    cheers,
      DaveK

Re: [PATCH] Add missing parens

[Eric Botcazou]

> The indentation I kept, if you want you can correct it.

No, you didn't, you replaced a correct 2-space indentation with a wrong TAB.

> The issue is, 
> that using macro DWARF_FRAME_REGNUM in case of example in dwarf2out.c
> ' if (DWARF_FRAME_REGNUM (i) == DWARF_FRAME_RETURN_COLUMN)' could
> leads to wrong code. As there is a semantic difference between
> 'if ((TARGET_64BIT ? dbx64_register_map[i] : svr4_dbx_register_map[i])
> == DWARF_FRAME_RETURN_COLUMN)
> and
> if (TARGET_64BIT ? dbx64_register_map[i] : svr4_dbx_register_map[i] ==
> DWARF_FRAME_RETURN_COLUMN)

That's precisely the bug the patch fixed so I'm not sure what you mean...

-- 
Eric Botcazou

Re: [PATCH] Add missing parens

[Kai Tietz]

2009/10/21 Eric Botcazou <ebotcazou@adacore.com>:
>> The indentation I kept, if you want you can correct it.
>
> No, you didn't, you replaced a correct 2-space indentation with a wrong TAB.

I corrected this already. Thanks for pointing it out.

>> The issue is,
>> that using macro DWARF_FRAME_REGNUM in case of example in dwarf2out.c
>> ' if (DWARF_FRAME_REGNUM (i) == DWARF_FRAME_RETURN_COLUMN)' could
>> leads to wrong code. As there is a semantic difference between
>> 'if ((TARGET_64BIT ? dbx64_register_map[i] : svr4_dbx_register_map[i])
>> == DWARF_FRAME_RETURN_COLUMN)
>> and
>> if (TARGET_64BIT ? dbx64_register_map[i] : svr4_dbx_register_map[i] ==
>> DWARF_FRAME_RETURN_COLUMN)
>
> That's precisely the bug the patch fixed so I'm not sure what you mean...
>
> --
> Eric Botcazou
>

Merged to 4.4 branch at revision 153585

Cheers,
Kai

-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination