[RFA] Undefined behaviour in include/coff/ti.h macros [was Re: BFD built failures with GCC 4.5]

[Dave Korn <dave dot korn dot cygwin at googlemail dot com>]

[ TIC4X/TIC54X maintainers CC'd. ]

On 23/04/2010 16:44, Dave Korn wrote:
> On 22/04/2010 18:58, Tom Tromey wrote:
>> I tried to build GDB (with --enable-targets=all) with GCC 4.5.
>>
>> I got the append errors.
>> I looked into them a bit, but I don't know anything about COFF or the
>> logic in ti.h (e.g. GET_SCNHDR_FLAGS).
> 
>   Thanks for spotting the problem; I'm on it now.

  Ouch, this one really turned out to open a whole bulk catering-size can of
worms.

  The underlying problem is that there are two different versions of the
section header struct in TI COFF format:

/* COFF0, COFF1 */
struct external_scnhdr_v01 {
	char		s_name[8];	/* section name			*/
	char		s_paddr[4];	/* physical address, aliased s_nlib */
	char		s_vaddr[4];	/* virtual address		*/
	char		s_size[4];	/* section size (in WORDS)      */
	char		s_scnptr[4];	/* file ptr to raw data for section */
	char		s_relptr[4];	/* file ptr to relocation	*/
	char		s_lnnoptr[4];	/* file ptr to line numbers	*/
	char		s_nreloc[2];	/* number of relocation entries	*/
	char		s_nlnno[2];	/* number of line number entries*/
	char		s_flags[2];	/* flags			*/
        char            s_reserved[1];  /* reserved                     */
        char            s_page[1];      /* section page number (LOAD)   */
};

/* COFF2 */
struct external_scnhdr {
	char		s_name[8];	/* section name			*/
	char		s_paddr[4];	/* physical address, aliased s_nlib */
	char		s_vaddr[4];	/* virtual address		*/
	char		s_size[4];	/* section size (in WORDS)      */
	char		s_scnptr[4];	/* file ptr to raw data for section */
	char		s_relptr[4];	/* file ptr to relocation	*/
	char		s_lnnoptr[4];	/* file ptr to line numbers	*/
	char		s_nreloc[4];	/* number of relocation entries	*/
	char		s_nlnno[4];	/* number of line number entries*/
	char		s_flags[4];	/* flags			*/
        char            s_reserved[2];  /* reserved                     */
        char            s_page[2];      /* section page number (LOAD)   */
};

  Notice how these are basically the same, except that the sizes of the last
few members, beginning at s_nreloc, are doubled in the COFF2 version.  The
tic4x/tic54x targets want to handle both these formats transparently, but
there's a problem: there's only one bfd_swap_section_header_in/out routine,
and it calls the various PUT/GET_SCNHDR_xxx macros with just a pointer to the
default struct external_scnhdr.

  In the current code, the solution is to apply a byte offset in the non-COFF2
case, which is hardcoded to the delta between the offsetof values of the
corresponding members in the two different versions of the structs, e.g.:

#define PUT_SCNHDR_FLAGS(ABFD, VAL, LOC) \
  (COFF2_P (ABFD) ? H_PUT_32 (ABFD, VAL, LOC) : H_PUT_16 (ABFD, VAL, (LOC) - 4))
#define GET_SCNHDR_PAGE(ABFD, LOC) \
  (COFF2_P (ABFD) ? H_GET_16 (ABFD, LOC) : (unsigned) H_GET_8 (ABFD, (LOC) - 7))

... and so on.  This is described by this comment:

/* COFF2 changes the offsets and sizes of these fields
   Assume we're dealing with the COFF2 scnhdr structure, and adjust
   accordingly.  Note: The GNU C versions of some of these macros
   are necessary in order to avoid compile time warnings triggered
   gcc's array bounds checking.  The PUT_SCNHDR_PAGE macro also has
   the advantage on not evaluating LOC twice.  */

and the "GNU C versions" to which it is referring replace the above
constructions with a statement-expression that attempts to avoid the array
bounds-checking error by dereferencing the pointer as a char*, e.g.:

#define PUT_SCNHDR_FLAGS(ABFD, VAL, LOC) \
  do \
    { \
      char * ptr = (LOC); \
      if (COFF2_P (ABFD)) \
	H_PUT_32 (ABFD, VAL, ptr); \
      else \
	H_PUT_16 (ABFD, VAL, ptr - 4); \
    } \
  while (0)
#define GET_SCNHDR_PAGE(ABFD, LOC) \
  ({ \
    unsigned page; \
    char * ptr = (LOC); \
    if (COFF2_P (ABFD)) \
      page = H_GET_16 (ABFD, ptr); \
    else \
      page = (unsigned) H_GET_8 (ABFD, ptr - 7); \
    page; \
  })

  Well, GCC just got cleverer, and now it can see right past that trick, and
it properly identifies that ptr is pointing to one of the char array members
of the struct, and complains about attempting to access the (e.g. again) -4th
or -7th element of an array of char[2] or char[1] (respectively).  That's a
real warning, because what we're doing there is pointer arithmetic, and
attempting to index a pointer right out of one array and into another: but
that's not allowed, as per the C standard; quoting here n1256, 6.5.6.8:

> If both the pointer operand and the result point to elements of the same
> array object, or one past the last element of the array object, the
> evaluation shall not produce an overflow; otherwise, the behavior is
> undefined. If the result points one past the last element of the array
> object, it shall not be used as the operand of a unary * operator that is
> evaluated.

  So, I guess that's undefined behaviour there.  Now, the optimal solution
would be to have an actual pointer to the actual type of the actual section
header, and dereference that to get directly at the actual member at whatever
its actual offsetof may be.  But that doesn't sit well with the structure of
coff_swap_scnhdr_in/out and the GET/SET_SCNHDR_XXXX macros that it uses,
because they take an lvalue of the dereferenced member of the section header
pointer, such as for example:

   scnhdr_int->s_flags = GET_SCNHDR_FLAGS (abfd, scnhdr_ext->s_flags);

and so there's no way from within the macro to change the type of the pointer
before it gets dereferenced.


  Ouch again.  So, the first draft at a solution I've come up with puts
wrappers around the various GET/SET_SCNHDR_ functions, that take the pointer
and the member as separate arguments:

   scnhdr_int->s_flags = GET_SCNHDR_FLAGS_WRAPPER (abfd, scnhdr_ext, s_flags);

where the default definition is

#ifndef GET_SCNHDR_FLAGS_WRAPPER
#define GET_SCNHDR_FLAGS_WRAPPER(ABFD, STRUCT, MEM) \
		GET_SCNHDR_FLAGS(ABFD, STRUCT->MEM)
#endif

which acts as the original code in all cases that don't define an override.
For TI COFF, I then add overrides that cast the pointer to the other kind of
section header before dereferencing it in the non-COFF2 case.  That, in turn,
would generate type-punning warnings without first unioning the two section
header structs, so I've ended up with a bunch of macros that look like the below:

union external_scnhdr_both {
	struct external_scnhdr_v01	v1;
	struct external_scnhdr		v2;
};

#define SCNHDR_BOTH union external_scnhdr_both

#define GET_SCNHDR_FLAGS_WRAPPER(ABFD, STRUCT, MEM) \
  ({ \
    int flags; \
    char *data = (char *)(STRUCT); \
    if (COFF2_P (ABFD)) \
      flags = H_GET_32 (ABFD, STRUCT->MEM); \
    else \
      flags = H_GET_16 (ABFD, ((SCNHDR_BOTH *)(data))->v1.MEM); \
    flags; \
  })


  Well, I think that's valid code; it compiles at all levels of
-Wstrict-aliasing without array bounds or type-punning warnings, but it's also
a bit ugly and fragile if any code ends up calling the GET/SET_SCNHDR_ macros
directly rather than the _WRAPPER version.

  I'm not sure what might make a better solution though.  Could we make the
different TI COFF formats use different bfd target vectors, without breaking
backward compatibility, or can anyone see a cleaner way to solve the problem
of accessing two different types of section header with only one set of code?

  Attached is a first draft of the fix (for TIC54X only, and untested as yet;
purely a PoC so far.)

    cheers,
      DaveK


? ld/testsuite/ld-pe/basefile1.s
? ld/testsuite/ld-pe/pe-basefile.exp
Index: bfd/coffswap.h
===================================================================
RCS file: /cvs/src/src/bfd/coffswap.h,v
retrieving revision 1.19
diff -p -u -r1.19 coffswap.h
--- bfd/coffswap.h	5 Sep 2009 07:56:22 -0000	1.19
+++ bfd/coffswap.h	24 Apr 2010 04:41:18 -0000
@@ -211,6 +211,43 @@
 #define PUT_RELOC_VADDR H_PUT_32
 #endif
 
+/* In order to support TI COFF, which defines two variants of the 
+   section header structure and then attempts to access members of
+   one via dereferencing a pointer to the member of the other version
+   and adjusting it by a constant byte offset, we provide thin wrappers
+   around the subset of the GET/PUT_SCNHDR_XXXX macros that take
+   separate arguments for the object pointer and member to access.
+   These default to just a straight dereference, but the TI COFF
+   version uses them to recast the object pointer to the correct
+   type before dereferencing.  */
+
+#ifndef GET_SCNHDR_NLNNO_WRAPPER
+#define GET_SCNHDR_NLNNO_WRAPPER(ABFD, STRUCT, MEM) \
+		GET_SCNHDR_NLNNO(ABFD, STRUCT->MEM)
+#endif
+#ifndef GET_SCNHDR_FLAGS_WRAPPER
+#define GET_SCNHDR_FLAGS_WRAPPER(ABFD, STRUCT, MEM) \
+		GET_SCNHDR_FLAGS(ABFD, STRUCT->MEM)
+#endif
+#ifndef GET_SCNHDR_PAGE_WRAPPER
+#define GET_SCNHDR_PAGE_WRAPPER(ABFD, STRUCT, MEM) \
+		GET_SCNHDR_PAGE(ABFD, STRUCT->MEM)
+#endif
+
+#ifndef PUT_SCNHDR_NLNNO_WRAPPER
+#define PUT_SCNHDR_NLNNO_WRAPPER(ABFD, VAL, STRUCT, MEM) \
+		PUT_SCNHDR_NLNNO(ABFD, VAL, STRUCT->MEM)
+#endif
+#ifndef PUT_SCNHDR_FLAGS_WRAPPER
+#define PUT_SCNHDR_FLAGS_WRAPPER(ABFD, VAL, STRUCT, MEM) \
+		PUT_SCNHDR_FLAGS(ABFD, VAL, STRUCT->MEM)
+#endif
+#ifndef PUT_SCNHDR_PAGE_WRAPPER
+#define PUT_SCNHDR_PAGE_WRAPPER(ABFD, VAL, STRUCT, MEM) \
+		PUT_SCNHDR_PAGE(ABFD, VAL, STRUCT->MEM)
+#endif
+
+
 #ifndef NO_COFF_RELOCS
 
 static void
@@ -766,9 +803,9 @@ coff_swap_scnhdr_in (bfd * abfd, void * 
   scnhdr_int->s_scnptr = GET_SCNHDR_SCNPTR (abfd, scnhdr_ext->s_scnptr);
   scnhdr_int->s_relptr = GET_SCNHDR_RELPTR (abfd, scnhdr_ext->s_relptr);
   scnhdr_int->s_lnnoptr = GET_SCNHDR_LNNOPTR (abfd, scnhdr_ext->s_lnnoptr);
-  scnhdr_int->s_flags = GET_SCNHDR_FLAGS (abfd, scnhdr_ext->s_flags);
+  scnhdr_int->s_flags = GET_SCNHDR_FLAGS_WRAPPER (abfd, scnhdr_ext, s_flags);
   scnhdr_int->s_nreloc = GET_SCNHDR_NRELOC (abfd, scnhdr_ext->s_nreloc);
-  scnhdr_int->s_nlnno = GET_SCNHDR_NLNNO (abfd, scnhdr_ext->s_nlnno);
+  scnhdr_int->s_nlnno = GET_SCNHDR_NLNNO_WRAPPER (abfd, scnhdr_ext, s_nlnno);
 #ifdef I960
   scnhdr_int->s_align = GET_SCNHDR_ALIGN (abfd, scnhdr_ext->s_align);
 #endif
@@ -795,13 +832,13 @@ coff_swap_scnhdr_out (bfd * abfd, void *
   PUT_SCNHDR_SCNPTR (abfd, scnhdr_int->s_scnptr, scnhdr_ext->s_scnptr);
   PUT_SCNHDR_RELPTR (abfd, scnhdr_int->s_relptr, scnhdr_ext->s_relptr);
   PUT_SCNHDR_LNNOPTR (abfd, scnhdr_int->s_lnnoptr, scnhdr_ext->s_lnnoptr);
-  PUT_SCNHDR_FLAGS (abfd, scnhdr_int->s_flags, scnhdr_ext->s_flags);
+  PUT_SCNHDR_FLAGS_WRAPPER (abfd, scnhdr_int->s_flags, scnhdr_ext, s_flags);
 #if defined(M88)
   H_PUT_32 (abfd, scnhdr_int->s_nlnno, scnhdr_ext->s_nlnno);
   H_PUT_32 (abfd, scnhdr_int->s_nreloc, scnhdr_ext->s_nreloc);
 #else
   if (scnhdr_int->s_nlnno <= MAX_SCNHDR_NLNNO)
-    PUT_SCNHDR_NLNNO (abfd, scnhdr_int->s_nlnno, scnhdr_ext->s_nlnno);
+    PUT_SCNHDR_NLNNO_WRAPPER (abfd, scnhdr_int->s_nlnno, scnhdr_ext, s_nlnno);
   else
     {
       char buf[sizeof (scnhdr_int->s_name) + 1];
@@ -812,7 +849,7 @@ coff_swap_scnhdr_out (bfd * abfd, void *
 	(_("%s: warning: %s: line number overflow: 0x%lx > 0xffff"),
 	 bfd_get_filename (abfd),
 	 buf, scnhdr_int->s_nlnno);
-      PUT_SCNHDR_NLNNO (abfd, 0xffff, scnhdr_ext->s_nlnno);
+      PUT_SCNHDR_NLNNO_WRAPPER (abfd, 0xffff, scnhdr_ext, s_nlnno);
     }
 
   if (scnhdr_int->s_nreloc <= MAX_SCNHDR_NRELOC)
Index: include/coff/ti.h
===================================================================
RCS file: /cvs/src/src/include/coff/ti.h,v
retrieving revision 1.22
diff -p -u -r1.22 ti.h
--- include/coff/ti.h	5 Sep 2009 07:56:25 -0000	1.22
+++ include/coff/ti.h	24 Apr 2010 04:41:20 -0000
@@ -193,6 +193,11 @@ struct external_scnhdr {
         char            s_page[2];      /* section page number (LOAD)   */
 };
 
+union external_scnhdr_both {
+	struct external_scnhdr_v01	v1;
+	struct external_scnhdr		v2;
+};
+
 /*
  * Special section flags
  */
@@ -210,104 +215,105 @@ struct external_scnhdr {
 #define STYP_ALIGN      (0x0F00) /* TI COFF stores section alignment here */
 
 #define	SCNHDR_V01 struct external_scnhdr_v01
-#define SCNHDR struct external_scnhdr
+#define SCNHDR struct external_scnhdr
+#define SCNHDR_BOTH union external_scnhdr_both
 #define	SCNHSZ_V01 40                  /* for v0 and v1 */
 #define SCNHSZ 48
 
-/* COFF2 changes the offsets and sizes of these fields 
-   Assume we're dealing with the COFF2 scnhdr structure, and adjust
-   accordingly.  Note: The GNU C versions of some of these macros
-   are necessary in order to avoid compile time warnings triggered
-   gcc's array bounds checking.  The PUT_SCNHDR_PAGE macro also has
-   the advantage on not evaluating LOC twice.  */
+/* COFF2 changes the offsets and sizes of these fields, so we need
+   to provide definitions of the GET/SET_SCNHDR_xxx_WRAPPER macros
+   here, as we must have separate declarations for the struct and
+   for the member being accessed.  The old strategy of accessing
+   everything through a pointer to external_scnhdr and offsetting
+   it by the delta in byte to the corresponding member in external_scnhdr_v01
+   is no longer acceptable since GCC-4.5.0, as we were indexing out
+   of the bounds of one array to reach into another; this was invoking
+   undefined behaviour per n1256#6.5.6.8, and cannot be relied on.  */
 
 #define GET_SCNHDR_NRELOC(ABFD, LOC) \
   (COFF2_P (ABFD) ? H_GET_32 (ABFD, LOC) : H_GET_16 (ABFD, LOC))
 #define PUT_SCNHDR_NRELOC(ABFD, VAL, LOC) \
   (COFF2_P (ABFD) ? H_PUT_32 (ABFD, VAL, LOC) : H_PUT_16 (ABFD, VAL, LOC))
-#ifdef __GNUC__
 #define GET_SCNHDR_NLNNO(ABFD, LOC) \
+  (COFF2_P (ABFD) ? H_GET_32 (ABFD, LOC) : H_GET_16 (ABFD, LOC))
+#define PUT_SCNHDR_NLNNO(ABFD, VAL, LOC) \
+  (COFF2_P (ABFD) ? H_PUT_32 (ABFD, VAL, LOC) : H_PUT_16 (ABFD, VAL, LOC))
+#define GET_SCNHDR_FLAGS(ABFD, LOC) \
+  (COFF2_P (ABFD) ? H_GET_32 (ABFD, LOC) : H_GET_16 (ABFD, LOC))
+#define PUT_SCNHDR_FLAGS(ABFD, VAL, LOC) \
+  (COFF2_P (ABFD) ? H_PUT_32 (ABFD, VAL, LOC) : H_PUT_16 (ABFD, VAL, LOC))
+#define GET_SCNHDR_PAGE(ABFD, LOC) \
+  (COFF2_P (ABFD) ? H_GET_16 (ABFD, LOC) : (unsigned) H_GET_8 (ABFD, LOC))
+/* On output, make sure that the "reserved" field is zero.  */
+#define PUT_SCNHDR_PAGE(ABFD, VAL, LOC) \
+  (COFF2_P (ABFD) \
+   ? H_PUT_16 (ABFD, VAL, LOC) \
+   : H_PUT_8 (ABFD, VAL, LOC), H_PUT_8 (ABFD, 0, (LOC) - 1))
+
+#define GET_SCNHDR_NLNNO_WRAPPER(ABFD, STRUCT, MEM) \
   ({ \
-    int nlnno;		\
-    char * ptr = (LOC); \
+    int nlnno;		\
+    char *data = STRUCT->s_name; \
     if (COFF2_P (ABFD)) \
-      nlnno = H_GET_32 (ABFD, ptr); \
+      nlnno = H_GET_32 (ABFD, STRUCT->MEM); \
     else \
-      nlnno = H_GET_16 (ABFD, ptr - 2); \
+      nlnno = H_GET_16 (ABFD, ((SCNHDR_BOTH *)(data))->v1.MEM); \
     nlnno; \
   })
-#define PUT_SCNHDR_NLNNO(ABFD, VAL, LOC) \
+#define PUT_SCNHDR_NLNNO_WRAPPER(ABFD, VAL, STRUCT, MEM) \
   do \
     { \
-      char * ptr = (LOC); \
+      char *data = STRUCT->s_name; \
       if (COFF2_P (ABFD)) \
-	H_PUT_32 (ABFD, VAL, ptr); \
+	H_PUT_32 (ABFD, VAL, STRUCT->MEM); \
       else \
-	H_PUT_16 (ABFD, VAL, ptr - 2); \
+	H_PUT_16 (ABFD, VAL, ((SCNHDR_BOTH *)(data))->v1.MEM); \
     } \
   while (0)
-#define GET_SCNHDR_FLAGS(ABFD, LOC) \
+#define GET_SCNHDR_FLAGS_WRAPPER(ABFD, STRUCT, MEM) \
   ({ \
     int flags; \
-    char * ptr = (LOC); \
+    char *data = STRUCT->s_name; \
     if (COFF2_P (ABFD)) \
-      flags = H_GET_32 (ABFD, ptr); \
+      flags = H_GET_32 (ABFD, STRUCT->MEM); \
     else \
-      flags = H_GET_16 (ABFD, ptr - 4); \
+      flags = H_GET_16 (ABFD, ((SCNHDR_BOTH *)(data))->v1.MEM); \
     flags; \
   })
-#define PUT_SCNHDR_FLAGS(ABFD, VAL, LOC) \
+#define PUT_SCNHDR_FLAGS_WRAPPER(ABFD, VAL, STRUCT, MEM) \
   do \
     { \
-      char * ptr = (LOC); \
+      char *data = STRUCT->s_name; \
       if (COFF2_P (ABFD)) \
-	H_PUT_32 (ABFD, VAL, ptr); \
+	H_PUT_32 (ABFD, VAL, STRUCT->MEM); \
       else \
-	H_PUT_16 (ABFD, VAL, ptr - 4); \
+	H_PUT_16 (ABFD, VAL, ((SCNHDR_BOTH *)(data))->v1.MEM); \
     } \
   while (0)
-#define GET_SCNHDR_PAGE(ABFD, LOC) \
+#define GET_SCNHDR_PAGE_WRAPPER(ABFD, STRUCT, MEM) \
   ({ \
     unsigned page; \
-    char * ptr = (LOC); \
+    char *data = STRUCT->s_name; \
     if (COFF2_P (ABFD)) \
-      page = H_GET_16 (ABFD, ptr); \
+      page = H_GET_16 (ABFD, STRUCT->MEM); \
     else \
-      page = (unsigned) H_GET_8 (ABFD, ptr - 7); \
+      page = (unsigned) H_GET_8 (ABFD, ((SCNHDR_BOTH *)(data))->v1.MEM); \
     page; \
   })
 /* On output, make sure that the "reserved" field is zero.  */
-#define PUT_SCNHDR_PAGE(ABFD, VAL, LOC) \
+#define PUT_SCNHDR_PAGE_WRAPPER(ABFD, VAL, STRUCT, MEM) \
   do \
     { \
-      char * ptr = (LOC); \
+      char *data = STRUCT->s_name; \
       if (COFF2_P (ABFD)) \
-	H_PUT_16 (ABFD, VAL, ptr); \
+	H_PUT_16 (ABFD, VAL, STRUCT->MEM); \
       else \
 	{ \
-	  H_PUT_8 (ABFD, VAL, ptr - 7); \
-	  H_PUT_8 (ABFD, 0, ptr - 8); \
+	  H_PUT_8 (ABFD, VAL, ((SCNHDR_BOTH *)(data))->v1.MEM); \
+	  H_PUT_8 (ABFD, 0, ((SCNHDR_BOTH *)(data))->v1.s_reserved); \
 	} \
     } \
   while (0)
-#else
-#define GET_SCNHDR_NLNNO(ABFD, LOC) \
-  (COFF2_P (ABFD) ? H_GET_32 (ABFD, LOC) : H_GET_16 (ABFD, (LOC) - 2))
-#define PUT_SCNHDR_NLNNO(ABFD, VAL, LOC) \
-  (COFF2_P (ABFD) ? H_PUT_32 (ABFD, VAL, LOC) : H_PUT_16 (ABFD, VAL, (LOC) - 2))
-#define GET_SCNHDR_FLAGS(ABFD, LOC) \
-  (COFF2_P (ABFD) ? H_GET_32 (ABFD, LOC) : H_GET_16 (ABFD, (LOC) - 4))
-#define PUT_SCNHDR_FLAGS(ABFD, VAL, LOC) \
-  (COFF2_P (ABFD) ? H_PUT_32 (ABFD, VAL, LOC) : H_PUT_16 (ABFD, VAL, (LOC) - 4))
-#define GET_SCNHDR_PAGE(ABFD, LOC) \
-  (COFF2_P (ABFD) ? H_GET_16 (ABFD, LOC) : (unsigned) H_GET_8 (ABFD, (LOC) - 7))
-/* On output, make sure that the "reserved" field is zero.  */
-#define PUT_SCNHDR_PAGE(ABFD, VAL, LOC) \
-  (COFF2_P (ABFD) \
-   ? H_PUT_16 (ABFD, VAL, LOC) \
-   : H_PUT_8 (ABFD, VAL, (LOC) - 7), H_PUT_8 (ABFD, 0, (LOC) - 8))
-#endif
-
 
 /* TI COFF stores section size as number of bytes (address units, not octets),
    so adjust to be number of octets, which is what BFD expects */ 
@@ -320,7 +326,7 @@ struct external_scnhdr {
   do									\
     {									\
       ((struct internal_scnhdr *)(INT))->s_page =			\
-	GET_SCNHDR_PAGE (ABFD, ((SCNHDR *)(EXT))->s_page);		\
+	GET_SCNHDR_PAGE_WRAPPER (ABFD, ((SCNHDR *)(EXT)), s_page);	\
     }									\
    while (0)
 
@@ -341,14 +347,14 @@ struct external_scnhdr {
 #define COFF_ADJUST_SCNHDR_OUT_POST(ABFD, INT, EXT) \
   do									   \
     {									   \
-      PUT_SCNHDR_NLNNO (ABFD, ((struct internal_scnhdr *)(INT))->s_nlnno,  \
-			((SCNHDR *)(EXT))->s_nlnno);			   \
+      PUT_SCNHDR_NLNNO_WRAPPER (ABFD, ((struct internal_scnhdr *)(INT))->s_nlnno,  \
+			((SCNHDR *)(EXT)), s_nlnno);			   \
       PUT_SCNHDR_NRELOC (ABFD, ((struct internal_scnhdr *)(INT))->s_nreloc,\
 			 ((SCNHDR *)(EXT))->s_nreloc);			   \
-      PUT_SCNHDR_FLAGS (ABFD, ((struct internal_scnhdr *)(INT))->s_flags,  \
-			((SCNHDR *)(EXT))->s_flags);			   \
-      PUT_SCNHDR_PAGE (ABFD, ((struct internal_scnhdr *)(INT))->s_page,    \
-		       ((SCNHDR *)(EXT))->s_page);			   \
+      PUT_SCNHDR_FLAGS_WRAPPER (ABFD, ((struct internal_scnhdr *)(INT))->s_flags,  \
+			((SCNHDR *)(EXT)), s_flags);			   \
+      PUT_SCNHDR_PAGE_WRAPPER (ABFD, ((struct internal_scnhdr *)(INT))->s_page,    \
+		       ((SCNHDR *)(EXT)), s_page);			   \
     }									   \
    while (0)