[patch PE-COFF]: Some minor nits

[patch PE-COFF]: Some minor nits

[Kai Tietz]

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

Hello,

Some minor nits I found in coffcode.h. One is that debugging sections have 
to have readonly flag set (abstracted by the section flags of '.debug' in 
pe-coff spec).
Secondly a clean-up about NOREAD sections. I introduced a helper flag for 
section's flags to indicated, that a section has no memory flag set,
I tested this patch for i686-pc-cygwin and for x86_64-pc-ming32 and saw no 
regressions on testrun.

ChangeLog
2009-03-13  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.

Is this patch ok for apply?
 
Cheers,
Kai



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

[-- Attachment #2: noread.diff --]
[-- Type: application/octet-stream, Size: 3555 bytes --]

Index: src/bfd/bfd-in2.h
===================================================================
--- src.orig/bfd/bfd-in2.h	2009-03-11 11:15:39.000000000 +0100
+++ src/bfd/bfd-in2.h	2009-03-13 10:47:19.447863400 +0100
@@ -1304,6 +1304,10 @@
      TMS320C54X only.  */
 #define SEC_TIC54X_CLINK 0x20000000
 
+  /* Indicate that section has the no read flag set. This happens
+     when memory read flag isn't set. */
+#define SEC_COFF_NOREAD 0x40000000
+
   /*  End of section flags.  */
 
   /* Some internal packed boolean fields.  */
Index: src/bfd/coffcode.h
===================================================================
--- src.orig/bfd/coffcode.h	2009-03-11 11:15:46.000000000 +0100
+++ src/bfd/coffcode.h	2009-03-13 11:00:12.682238400 +0100
@@ -637,7 +637,7 @@
   /* FIXME: There is no gas syntax to specify the debug section flag.  */
   if (CONST_STRNEQ (sec_name, DOT_DEBUG)
       || CONST_STRNEQ (sec_name, GNU_LINKONCE_WI))
-    sec_flags = SEC_DEBUGGING;
+    sec_flags = SEC_DEBUGGING | SEC_READONLY;
 
   /* skip LOAD */
   /* READONLY later */
@@ -666,19 +666,14 @@
   /* skip LINK_DUPLICATES */
   /* skip LINKER_CREATED */
 
-  if (sec_flags & (SEC_ALLOC | SEC_LOAD))
-    {
-      /* For now, the read/write bits are mapped onto SEC_READONLY, even
-	 though the semantics don't quite match.  The bits from the input
-	 are retained in pei_section_data(abfd, section)->pe_flags.  */
-      styp_flags |= IMAGE_SCN_MEM_READ;       /* Always readable.  */
-      if ((sec_flags & SEC_READONLY) == 0)
-	styp_flags |= IMAGE_SCN_MEM_WRITE;    /* Invert READONLY for write.  */
-      if (sec_flags & SEC_CODE)
-	styp_flags |= IMAGE_SCN_MEM_EXECUTE;  /* CODE->EXECUTE.  */
-      if (sec_flags & SEC_COFF_SHARED)
-	styp_flags |= IMAGE_SCN_MEM_SHARED;   /* Shared remains meaningful.  */
-    }
+  if ((sec_flags & SEC_COFF_NOREAD) == 0)
+    styp_flags |= IMAGE_SCN_MEM_READ;     /* Invert NOREAD for read.  */
+  if ((sec_flags & SEC_READONLY) == 0)
+    styp_flags |= IMAGE_SCN_MEM_WRITE;    /* Invert READONLY for write.  */
+  if (sec_flags & SEC_CODE)
+    styp_flags |= IMAGE_SCN_MEM_EXECUTE;  /* CODE->EXECUTE.  */
+  if (sec_flags & SEC_COFF_SHARED)
+    styp_flags |= IMAGE_SCN_MEM_SHARED;   /* Shared remains meaningful.  */
 
   return styp_flags;
 }
@@ -1117,6 +1112,10 @@
   /* Assume read only unless IMAGE_SCN_MEM_WRITE is specified.  */
   sec_flags = SEC_READONLY;
 
+  /* If section disallows read, then set the NOREAD flag. */
+  if ((styp_flags & IMAGE_SCN_MEM_READ) == 0)
+    sec_flags |= SEC_COFF_NOREAD;
+
   /* Process each flag bit in styp_flags in turn.  */
   while (styp_flags)
     {
@@ -1149,7 +1148,7 @@
 	  break;
 #endif
 	case IMAGE_SCN_MEM_READ:
-	  /* Ignored, assume it always to be true.  */
+	  sec_flags |= SEC_COFF_NOREAD;
 	  break;
 	case IMAGE_SCN_TYPE_NO_PAD:
 	  /* Skip.  */
@@ -1245,6 +1244,8 @@
     sec_flags |= SEC_LINK_ONCE | SEC_LINK_DUPLICATES_DISCARD;
 #endif
 
+  section->flags = sec_flags;
+
   if (flags_ptr)
     * flags_ptr = sec_flags;
 
Index: src/bfd/section.c
===================================================================
--- src.orig/bfd/section.c	2009-03-11 11:14:47.000000000 +0100
+++ src/bfd/section.c	2009-03-13 10:47:37.791613400 +0100
@@ -343,6 +343,10 @@
 .     TMS320C54X only.  *}
 .#define SEC_TIC54X_CLINK 0x20000000
 .
+.  {* Indicate that section has the no read flag set. This happens
+.     when memory read flag isn't set. *}
+.#define SEC_COFF_NOREAD 0x40000000
+.
 .  {*  End of section flags.  *}
 .
 .  {* Some internal packed boolean fields.  *}

Re: [patch PE-COFF]: Some minor nits

[Dave Korn]

Kai Tietz wrote:
> Hello,

  Hi Kai,

> Some minor nits I found in coffcode.h. One is that debugging sections have 
> to have readonly flag set (abstracted by the section flags of '.debug' in 
> pe-coff spec).

  Can you point us at a reference for this?  And have you run the GDB
testsuite to see if it causes any changes?  (As I discovered recently,
sometimes the other tools have come to depend on these minor non-compliances
of ours!)

> Secondly a clean-up about NOREAD sections. 

  http://www.national.com/rap/files/datasheet.pdf :)

> I introduced a helper flag for 
> section's flags to indicated, that a section has no memory flag set,
> I tested this patch for i686-pc-cygwin and for x86_64-pc-ming32 and saw no 
> regressions on testrun.

  Does anything actually currently use NOREAD?

    cheers,
      DaveK

Re: [patch PE-COFF]: Some minor nits

[Kai Tietz]

Dave Korn <dave.korn.cygwin@googlemail.com> wrote on 17.03.2009 00:15:12:

> Kai Tietz wrote:
> > Hello,
> 
>   Hi Kai,
> 
> > Some minor nits I found in coffcode.h. One is that debugging sections 
have 
> > to have readonly flag set (abstracted by the section flags of '.debug' 
in 
> > pe-coff spec).
> 
>   Can you point us at a reference for this?  And have you run the GDB
> testsuite to see if it causes any changes?  (As I discovered recently,
> sometimes the other tools have come to depend on these minor 
non-compliances
> of ours!)

Well, see pe-coff-v8 specification in section "46 Special Sections". Here 
are the used section flags given.
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.
> > Secondly a clean-up about NOREAD sections. 
> 
>   http://www.national.com/rap/files/datasheet.pdf :)
> 
> > I introduced a helper flag for 
> > section's flags to indicated, that a section has no memory flag set,
> > I tested this patch for i686-pc-cygwin and for x86_64-pc-ming32 and 
saw no 
> > regressions on testrun.
> 
>   Does anything actually currently use NOREAD?

Heh, I don't thought about write-only ram. It is more near than that. See 
e.g ".cormeta" (for .NET), ".idlsym", and ".drective".

>     cheers,
>       DaveK
> 

Cheers,
Kai

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

Re: [patch PE-COFF]: Some minor nits

[Dave Korn]

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

Re: [patch PE-COFF]: Some minor nits

[Kai Tietz]

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

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

[-- Attachment #2: noread.diff --]
[-- Type: application/octet-stream, Size: 3556 bytes --]

Index: src/bfd/bfd-in2.h
===================================================================
--- src.orig/bfd/bfd-in2.h	2009-03-11 11:15:39.000000000 +0100
+++ src/bfd/bfd-in2.h	2009-03-13 10:47:19.447863400 +0100
@@ -1304,6 +1304,10 @@
      TMS320C54X only.  */
 #define SEC_TIC54X_CLINK 0x20000000
 
+  /* Indicate that section has the no read flag set. This happens
+     when memory read flag isn't set. */
+#define SEC_COFF_NOREAD 0x40000000
+
   /*  End of section flags.  */
 
   /* Some internal packed boolean fields.  */
Index: src/bfd/coffcode.h
===================================================================
--- src.orig/bfd/coffcode.h	2009-03-11 11:15:46.000000000 +0100
+++ src/bfd/coffcode.h	2009-03-13 11:00:12.682238400 +0100
@@ -637,7 +637,7 @@
   /* FIXME: There is no gas syntax to specify the debug section flag.  */
   if (CONST_STRNEQ (sec_name, DOT_DEBUG)
       || CONST_STRNEQ (sec_name, GNU_LINKONCE_WI))
-    sec_flags = SEC_DEBUGGING;
+    sec_flags = SEC_DEBUGGING | SEC_READONLY;
 
   /* skip LOAD */
   /* READONLY later */
@@ -666,19 +666,14 @@
   /* skip LINK_DUPLICATES */
   /* skip LINKER_CREATED */
 
-  if (sec_flags & (SEC_ALLOC | SEC_LOAD))
-    {
-      /* For now, the read/write bits are mapped onto SEC_READONLY, even
-	 though the semantics don't quite match.  The bits from the input
-	 are retained in pei_section_data(abfd, section)->pe_flags.  */
-      styp_flags |= IMAGE_SCN_MEM_READ;       /* Always readable.  */
-      if ((sec_flags & SEC_READONLY) == 0)
-	styp_flags |= IMAGE_SCN_MEM_WRITE;    /* Invert READONLY for write.  */
-      if (sec_flags & SEC_CODE)
-	styp_flags |= IMAGE_SCN_MEM_EXECUTE;  /* CODE->EXECUTE.  */
-      if (sec_flags & SEC_COFF_SHARED)
-	styp_flags |= IMAGE_SCN_MEM_SHARED;   /* Shared remains meaningful.  */
-    }
+  if ((sec_flags & SEC_COFF_NOREAD) == 0)
+    styp_flags |= IMAGE_SCN_MEM_READ;     /* Invert NOREAD for read.  */
+  if ((sec_flags & SEC_READONLY) == 0)
+    styp_flags |= IMAGE_SCN_MEM_WRITE;    /* Invert READONLY for write.  */
+  if (sec_flags & SEC_CODE)
+    styp_flags |= IMAGE_SCN_MEM_EXECUTE;  /* CODE->EXECUTE.  */
+  if (sec_flags & SEC_COFF_SHARED)
+    styp_flags |= IMAGE_SCN_MEM_SHARED;   /* Shared remains meaningful.  */
 
   return styp_flags;
 }
@@ -1117,6 +1112,10 @@
   /* Assume read only unless IMAGE_SCN_MEM_WRITE is specified.  */
   sec_flags = SEC_READONLY;
 
+  /* If section disallows read, then set the NOREAD flag. */
+  if ((styp_flags & IMAGE_SCN_MEM_READ) == 0)
+    sec_flags |= SEC_COFF_NOREAD;
+
   /* Process each flag bit in styp_flags in turn.  */
   while (styp_flags)
     {
@@ -1149,7 +1148,7 @@
 	  break;
 #endif
 	case IMAGE_SCN_MEM_READ:
-	  /* Ignored, assume it always to be true.  */
+	  sec_flags &= ~SEC_COFF_NOREAD;
 	  break;
 	case IMAGE_SCN_TYPE_NO_PAD:
 	  /* Skip.  */
@@ -1245,6 +1244,8 @@
     sec_flags |= SEC_LINK_ONCE | SEC_LINK_DUPLICATES_DISCARD;
 #endif
 
+  section->flags = sec_flags;
+
   if (flags_ptr)
     * flags_ptr = sec_flags;
 
Index: src/bfd/section.c
===================================================================
--- src.orig/bfd/section.c	2009-03-11 11:14:47.000000000 +0100
+++ src/bfd/section.c	2009-03-13 10:47:37.791613400 +0100
@@ -343,6 +343,10 @@
 .     TMS320C54X only.  *}
 .#define SEC_TIC54X_CLINK 0x20000000
 .
+.  {* Indicate that section has the no read flag set. This happens
+.     when memory read flag isn't set. *}
+.#define SEC_COFF_NOREAD 0x40000000
+.
 .  {*  End of section flags.  *}
 .
 .  {* Some internal packed boolean fields.  *}

Re: [patch PE-COFF]: Some minor nits

[Dave Korn]

Kai Tietz wrote:

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

  Oh, sorry to be such a nuisance, I've just spotted another potential hiccup,
only minor though:

@@ -1245,6 +1244,8 @@
     sec_flags |= SEC_LINK_ONCE | SEC_LINK_DUPLICATES_DISCARD;
 #endif

+  section->flags = sec_flags;
+
   if (flags_ptr)
     * flags_ptr = sec_flags;


  This changes the behaviour of the function styp_to_sec_flags.  As far as I
could see the only client is this code in make_a_section_from_file in coffgen.c:


  return_section->lineno_count = hdr->s_nlnno;
  return_section->userdata = NULL;
  return_section->next = NULL;
  return_section->target_index = target_index;

  if (! bfd_coff_styp_to_sec_flags_hook (abfd, hdr, name, return_section,
					 & flags))
    result = FALSE;

  return_section->flags = flags;

... so it will make no difference in this case, but I still think it isn't
right for what should be a simple conversion function returning an integer to
modify one of its input parameters.  Nothing about the function name suggests
it will modify the section object, nor the documentation comment at the top.
I think the assignment can be safely dropped.

    cheers,
      DaveK

Re: [patch PE-COFF]: Some minor nits

[Kai Tietz]

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

2009/3/18 Dave Korn <dave.korn.cygwin@googlemail.com>:
> Kai Tietz wrote:
>
>> Thanks Dave for reviewing it. Here the adjusted patch as discussed.
>
>  Oh, sorry to be such a nuisance, I've just spotted another potential hiccup,
> only minor though:
>
> @@ -1245,6 +1244,8 @@
>     sec_flags |= SEC_LINK_ONCE | SEC_LINK_DUPLICATES_DISCARD;
>  #endif
>
> +  section->flags = sec_flags;
> +
>   if (flags_ptr)
>     * flags_ptr = sec_flags;
>
>
>  This changes the behaviour of the function styp_to_sec_flags.  As far as I
> could see the only client is this code in make_a_section_from_file in coffgen.c:
>
>
>  return_section->lineno_count = hdr->s_nlnno;
>  return_section->userdata = NULL;
>  return_section->next = NULL;
>  return_section->target_index = target_index;
>
>  if (! bfd_coff_styp_to_sec_flags_hook (abfd, hdr, name, return_section,
>                                         & flags))
>    result = FALSE;
>
>  return_section->flags = flags;
>
> ... so it will make no difference in this case, but I still think it isn't
> right for what should be a simple conversion function returning an integer to
> modify one of its input parameters.  Nothing about the function name suggests
> it will modify the section object, nor the documentation comment at the top.
> I think the assignment can be safely dropped.
>
>    cheers,
>      DaveK
>

Well, you're right. The issue here was for me, that locally I removed
in the function make_a_section_from_file the assignment of
section->flags. I removed this hunk from the patch.

Cheers,
Kai

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

[-- Attachment #2: noread.diff --]
[-- Type: application/octet-stream, Size: 3378 bytes --]

Index: src/bfd/bfd-in2.h
===================================================================
--- src.orig/bfd/bfd-in2.h	2009-03-11 11:15:39.000000000 +0100
+++ src/bfd/bfd-in2.h	2009-03-13 10:47:19.447863400 +0100
@@ -1304,6 +1304,10 @@
      TMS320C54X only.  */
 #define SEC_TIC54X_CLINK 0x20000000
 
+  /* Indicate that section has the no read flag set. This happens
+     when memory read flag isn't set. */
+#define SEC_COFF_NOREAD 0x40000000
+
   /*  End of section flags.  */
 
   /* Some internal packed boolean fields.  */
Index: src/bfd/coffcode.h
===================================================================
--- src.orig/bfd/coffcode.h	2009-03-11 11:15:46.000000000 +0100
+++ src/bfd/coffcode.h	2009-03-13 11:00:12.682238400 +0100
@@ -637,7 +637,7 @@
   /* FIXME: There is no gas syntax to specify the debug section flag.  */
   if (CONST_STRNEQ (sec_name, DOT_DEBUG)
       || CONST_STRNEQ (sec_name, GNU_LINKONCE_WI))
-    sec_flags = SEC_DEBUGGING;
+    sec_flags = SEC_DEBUGGING | SEC_READONLY;
 
   /* skip LOAD */
   /* READONLY later */
@@ -666,19 +666,14 @@
   /* skip LINK_DUPLICATES */
   /* skip LINKER_CREATED */
 
-  if (sec_flags & (SEC_ALLOC | SEC_LOAD))
-    {
-      /* For now, the read/write bits are mapped onto SEC_READONLY, even
-	 though the semantics don't quite match.  The bits from the input
-	 are retained in pei_section_data(abfd, section)->pe_flags.  */
-      styp_flags |= IMAGE_SCN_MEM_READ;       /* Always readable.  */
-      if ((sec_flags & SEC_READONLY) == 0)
-	styp_flags |= IMAGE_SCN_MEM_WRITE;    /* Invert READONLY for write.  */
-      if (sec_flags & SEC_CODE)
-	styp_flags |= IMAGE_SCN_MEM_EXECUTE;  /* CODE->EXECUTE.  */
-      if (sec_flags & SEC_COFF_SHARED)
-	styp_flags |= IMAGE_SCN_MEM_SHARED;   /* Shared remains meaningful.  */
-    }
+  if ((sec_flags & SEC_COFF_NOREAD) == 0)
+    styp_flags |= IMAGE_SCN_MEM_READ;     /* Invert NOREAD for read.  */
+  if ((sec_flags & SEC_READONLY) == 0)
+    styp_flags |= IMAGE_SCN_MEM_WRITE;    /* Invert READONLY for write.  */
+  if (sec_flags & SEC_CODE)
+    styp_flags |= IMAGE_SCN_MEM_EXECUTE;  /* CODE->EXECUTE.  */
+  if (sec_flags & SEC_COFF_SHARED)
+    styp_flags |= IMAGE_SCN_MEM_SHARED;   /* Shared remains meaningful.  */
 
   return styp_flags;
 }
@@ -1117,6 +1112,10 @@
   /* Assume read only unless IMAGE_SCN_MEM_WRITE is specified.  */
   sec_flags = SEC_READONLY;
 
+  /* If section disallows read, then set the NOREAD flag. */
+  if ((styp_flags & IMAGE_SCN_MEM_READ) == 0)
+    sec_flags |= SEC_COFF_NOREAD;
+
   /* Process each flag bit in styp_flags in turn.  */
   while (styp_flags)
     {
@@ -1149,7 +1148,7 @@
 	  break;
 #endif
 	case IMAGE_SCN_MEM_READ:
-	  /* Ignored, assume it always to be true.  */
+	  sec_flags &= ~SEC_COFF_NOREAD;
 	  break;
 	case IMAGE_SCN_TYPE_NO_PAD:
 	  /* Skip.  */
Index: src/bfd/section.c
===================================================================
--- src.orig/bfd/section.c	2009-03-11 11:14:47.000000000 +0100
+++ src/bfd/section.c	2009-03-13 10:47:37.791613400 +0100
@@ -343,6 +343,10 @@
 .     TMS320C54X only.  *}
 .#define SEC_TIC54X_CLINK 0x20000000
 .
+.  {* Indicate that section has the no read flag set. This happens
+.     when memory read flag isn't set. *}
+.#define SEC_COFF_NOREAD 0x40000000
+.
 .  {*  End of section flags.  *}
 .
 .  {* Some internal packed boolean fields.  *}

Re: [patch PE-COFF]: Some minor nits

[Dave Korn]

Kai Tietz wrote:
>  I removed this hunk from the patch.

  That all looks right to me now :)

    cheers,
      DaveK

Re: [patch PE-COFF]: Some minor nits

[Nick Clifton]

Hi Kai,

> Well, you're right. The issue here was for me, that locally I removed
> in the function make_a_section_from_file the assignment of
> section->flags. I removed this hunk from the patch.

Patch approved - please apply.

Cheers
   Nick

Re: [patch PE-COFF]: Some minor nits

[Kai Tietz]

2009/3/19 Nick Clifton <nickc@redhat.com>:
> Hi Kai,
>
>> Well, you're right. The issue here was for me, that locally I removed
>> in the function make_a_section_from_file the assignment of
>> section->flags. I removed this hunk from the patch.
>
> Patch approved - please apply.
>
> Cheers
>  Nick
>
>
>

Ok, applied.

Thanks,
Kai

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