[patch PE-COFF]: Some minor nits

[Kai Tietz]

2009/3/18 Dave Korn <dave.korn.cygwin@googlemail.com>:
> Kai Tietz wrote:
>> Dave Korn <dave.korn.cygwin@googlemail.com> wrote on 17.03.2009 00:15:12:
>
>>>   Can you point us at a reference for this?  And have you run the GDB
>>> testsuite to see if it causes any changes?
>
>> Well, see pe-coff-v8 specification in section "46 Special Sections". Here
>> are the used section flags given.
>
>  Right, thank you.  That looks like the correct mapping of bits, but I think
> I spotted something inconsistent:
>
> +  if ((sec_flags & SEC_COFF_NOREAD) == 0)
> +    styp_flags |= IMAGE_SCN_MEM_READ;     /* Invert NOREAD for read.  */
>
>  Set IMAGE_SCN_MEM_READ if *NOT* COFF_NOREAD.
>
> +  /* If section disallows read, then set the NOREAD flag. */
> +  if ((styp_flags & IMAGE_SCN_MEM_READ) == 0)
> +    sec_flags |= SEC_COFF_NOREAD;
>
>  Set COFF_NOREAD if *NOT* IMAGE_SCN_MEM_READ.
>
>        case IMAGE_SCN_MEM_READ:
> -         /* Ignored, assume it always to be true.  */
> +         sec_flags |= SEC_COFF_NOREAD;
>
>  Set COFF_NOREAD if IMAGE_SCN_MEM_READ.  This looks like the wrong way round,
> the bit should be set by default and cleared if IMAGE_SCN_MEM_READ is found?
>
>  I also see that in both these two hunks it would be possible for a section
> that has neither IMAGE_SCN_MEM_READ nor IMAGE_SCN_MEM_WRITE to end up setting
> SEC_READONLY and SEC_COFF_NOREAD simultaneously, because SEC_READONLY gets set
> by default.  That seems inconsistent to me, could it be a problem?  I think it
> will lead to the section in memory being writeable but not readable, rather
> than neither.
>
>> I tested it with gdb for w64 and found no regression here. Other platforms
>> I didn't tested with gdb, just by binutil's testsuite.
>
>  I'm sure just one platform is enough for the GDB testsuite, I was just
> hoping to make sure you'd done a smoke test that it didn't all break.
>
>    cheers,
>      DaveK
>
>

Thanks Dave for reviewing it. Here the adjusted patch as discussed.

ChangeLog
2009-03-18  Kai Tietz  <kai.tietz@onevision.com>

       * bfd-in2.h: Regenerated.
       * coffcode.h (sec_to_styp_flags): For pe-coff add SEC_READONLY
       for debugging sections and map memory read/write dependent on
       SEC_COFF_NOREAD.
       (styp_to_sec_flags): Set SEC_COFF_NOREAD for sections
       without memory read flags set.
       * section.c: Add SEC_COFF_NOREAD to section flags.


Tested on mingw-w64 and cygwin without any new regressions.

Ok for apply?

Cheers,
Kai

-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination
-------------- next part --------------
A non-text attachment was scrubbed...
Name: noread.diff
Type: application/octet-stream
Size: 3556 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20090318/b10345f5/attachment.obj>