[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