Bug 11017

Summary:	PAGE_SIZE redefined in dlltool.c
Product:	binutils	Reporter:	Jerker Bäck <jerker.back>
Component:	binutils	Assignee:	unassigned
Status:	RESOLVED FIXED
Severity:	normal	CC:	bug-binutils
Priority:	P2
Version:	2.21
Target Milestone:	---
Host:	x86_64-unknown-interix6.1	Target:	x86_64-unknown-interix6.1
Build:	x86_64-unknown-interix6.1	Last reconfirmed:
Attachments:	Solution based on COFF_PAGE_SIZE

Description Jerker Bäck 2009-11-24 21:24:00 UTC

dlltool.c(244) defines PAGE_SIZE

The PAGE_SIZE symbol is/should be defined in limits.h on a POSIX system (see
http://www.opengroup.org). NT POSIX subsystem (SUA) defines it to 0x10000
(10*0x1000). In my headers PAGE_SIZE is defined as an enum and dlltool.c
generates a compiler error due to PAGE_SIZE is defined before including system
headers.

I need advise how to fix this. 
Assuming setting PAGE_SIZE to 0x1000 is preferable, we can let dlltool.c set an
explicit value after including the system headers, like this:

Index: dlltool.c
===================================================================
RCS file: /cvs/src/src/binutils/dlltool.c,v
retrieving revision 1.97
diff -d -u -p -r1.97 dlltool.c
--- dlltool.c	28 Oct 2009 17:21:36 -0000	1.97
+++ dlltool.c	24 Nov 2009 17:27:50 -0000
@@ -241,9 +241,6 @@
 
 #define show_allnames 0
 
-#define PAGE_SIZE ((bfd_vma) 4096)
-#define PAGE_MASK ((bfd_vma) (-4096))
-
 #include "sysdep.h"
 #include "bfd.h"
 #include "libiberty.h"
@@ -259,6 +256,9 @@
 #include <stdarg.h>
 #include <assert.h>
 
+#define PAGE_SIZE ((bfd_vma) 4096)
+#define PAGE_MASK ((bfd_vma) (-4096))
+

Comment 1 Hans-Peter Nilsson 2009-11-24 21:39:56 UTC

Random comment coming out of the woodwork:
I can't help thinking that you should change this and all uses to
TARGET_PAGE_SIZE and TARGET_PAGE_MASK, and the problem would probably be solved.
If you *really* need the host page size...no you don't.

Comment 2 Hans-Peter Nilsson 2009-11-24 21:41:16 UTC

*** Bug 11016 has been marked as a duplicate of this bug. ***

Comment 3 Andreas Schwab 2009-11-24 21:42:15 UTC

*** Bug 11014 has been marked as a duplicate of this bug. ***

Comment 4 Jerker Bäck 2009-11-25 12:44:40 UTC

(In reply to comment #1)
> Random comment coming out of the woodwork:
> I can't help thinking that you should change this and all uses to
> TARGET_PAGE_SIZE and TARGET_PAGE_MASK, and the problem would probably be 
solved.
> If you *really* need the host page size...no you don't.

That is an idea. TARGET_PAGE_SIZE=0x1000 is set by all targets using dlltool 
(AFAICS), but it is defined by the ld/emulparams scripts and used only in ld 
scripts. BTW HOST is TARGET, although dlltool is not currently used by 
Interix. It favours its own PE shared library solution.

Comment 5 Jerker Bäck 2009-12-01 13:46:06 UTC

Created attachment 4434 [details]
Solution based on COFF_PAGE_SIZE

Antother solution could be to use COFF_PAGE_SIZE. The COFF_PAGE_SIZE define
must then be moved from implementation to header.

Comment 6 Sourceware Commits 2009-12-02 14:04:33 UTC

Subject: Bug 11017

CVSROOT:	/cvs/src
Module name:	src
Changes by:	nickc@sourceware.org	2009-12-02 14:04:17

Modified files:
	include/coff   : ChangeLog i386.h x86_64.h 
	bfd            : ChangeLog coff-i386.c 
	binutils       : ChangeLog dlltool.c 

Log message:
	PR binutils/11017
	* dlltool.c (PAGE_SIZE): Delete.
	(PAGE_MASK): Provide default definition based on COFF_PAGE_SIZE.
	Check for DLLTOOL_DEFAULT_MX86_64 and DLLTOOL_DEFAULT_I386.
	
	* coff-i386.h (COFF_PAGE_SIZE): Definition moved to coff/i386.h
	
	* i386lh (COFF_PAGE_SIZE): Define.
	* x86_64.h (COFF_PAGE_SIZE): Define.

Patches:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/include/coff/ChangeLog.diff?cvsroot=src&r1=1.89&r2=1.90
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/include/coff/i386.h.diff?cvsroot=src&r1=1.3&r2=1.4
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/include/coff/x86_64.h.diff?cvsroot=src&r1=1.1&r2=1.2
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&r1=1.4849&r2=1.4850
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/bfd/coff-i386.c.diff?cvsroot=src&r1=1.30&r2=1.31
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/binutils/ChangeLog.diff?cvsroot=src&r1=1.1570&r2=1.1571
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/binutils/dlltool.c.diff?cvsroot=src&r1=1.97&r2=1.98

Comment 7 Nick Clifton 2009-12-02 14:05:30 UTC

Hi Jerker,

  Thanks for the patch - I have now checked it in.

Cheers
  Nick