[patch PE-COFF]: Some minor nits
Dave Korn
dave.korn.cygwin@googlemail.com
Wed Mar 18 15:47:00 GMT 2009
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
More information about the Binutils
mailing list