Question about another IA64 HP-UX build problem

Question about another IA64 HP-UX build problem

[Steve Ellcey]

OK, I thought I had found all the IA64 HP-UX build problems but I was
just building gas and not binutils.  When building binutils I ran into a
problem with src/binutils/strings.c, this code has:

#ifdef HAVE_GETC_UNLOCKED
          c = getc_unlocked (stream);
#else
          c = getc (stream);
#endif

Now, on HP-UX the macro HAVE_GETC_UNLOCKED is defined but I only see the
declaration when compiling with -D_REENTRENT.  This is off by default so
I get an 'implicit declaration' error.

My question is, why is this the only use of getc_unlocked in binutils?

I see other uses of getc in nlmheader.y, readelf.c, resrc.c, sysdump.c,
and windres.c, but none of them have this ifdef structure.  Is there
a reason for this?  It looks like GCC has a way to handle this and I
can probably fix it with the same mechanism but I first wanted to find
out if there is a reason for having getc_unlocked in the first place.
Is it a performance issue?

Steve Ellcey
sje@cup.hp.com

Re: Question about another IA64 HP-UX build problem

[James E Wilson]

On Thu, 2005-05-05 at 11:16, Steve Ellcey wrote:
> My question is, why is this the only use of getc_unlocked in binutils?

The code has been there since 2001-12-04.  It was first posted here
    http://sourceware.org/ml/binutils/2001-10/msg00499.html
Have you ever tried building this stuff on ia64-hpux before?  It is
present since at least binutils-2.13; I didn't try looking back any
farther than that.

It is part of Jakub's string section merging code.  Also, it is part of
the large file support.  I can see how one might care about performance
here if you have an object file larger than 2GB and you are trying to do
string merging on it.

getc_unlocked isn't used anywhere else in binutils simply because no one
else has found a need for it, or perhaps, the idea of using it hasn't
occurred to anyone else yet.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com

Re: Question about another IA64 HP-UX build problem

[Steve Ellcey]

> On Thu, 2005-05-05 at 11:16, Steve Ellcey wrote:
> > My question is, why is this the only use of getc_unlocked in binutils?
> 
> The code has been there since 2001-12-04.  It was first posted here
>     http://sourceware.org/ml/binutils/2001-10/msg00499.html
> Have you ever tried building this stuff on ia64-hpux before?  It is
> present since at least binutils-2.13; I didn't try looking back any
> farther than that.

Yes, I have built this on ia64-hpux before.  It always gave a warning
because it didn't see the declaration of getc_unlocked in stdio.h.  This
is because _REENTRENT is not defined and the HP-UX header file only
defines getc_unlocked when _REENTRENT is defined.  But the warning
didn't cause any problem when using the routine.  It is just that the
warning now causes an error due to -Werror.

I'll see if I can figure out how GCC is handling this and copy that.

Steve Ellcey
sje@cup.hp.com

Re: Question about another IA64 HP-UX build problem

[Steve Ellcey]

> getc_unlocked isn't used anywhere else in binutils simply because no one
> else has found a need for it, or perhaps, the idea of using it hasn't
> occurred to anyone else yet.
> -- 
> Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com

Here is the patch I came up with to address the getc_unlocked problem.
The problem is that HAVE_GETC_UNLOCKED is true (because HP-UX has it)
but the declaration isn't seen because we didn't compile with
-D_REENTRENT, thus we get a no-declaration error.  Tested on IA64 HP-UX.


binutils/ChangeLog:

2005-05-05  Steve Ellcey  <sje@cup.hp.com>

	configure.in: Check for getc_unlocked prototype.
	configure: Regenerate
	config.in: Regenerate
	strings.c: Only call getc_unlocked if we have seen a prototype.


*** src.orig/binutils/configure.in	Thu May  5 16:21:14 2005
--- src/binutils/configure.in	Thu May  5 16:20:28 2005
*************** BFD_NEED_DECLARATION(strstr)
*** 217,222 ****
--- 217,223 ----
  BFD_NEED_DECLARATION(sbrk)
  BFD_NEED_DECLARATION(getenv)
  BFD_NEED_DECLARATION(environ)
+ BFD_NEED_DECLARATION(getc_unlocked)
  
  BFD_BINARY_FOPEN
  
*** src.orig/binutils/strings.c	Thu May  5 16:21:08 2005
--- src/binutils/strings.c	Thu May  5 16:20:33 2005
*************** get_char (FILE *stream, file_off *addres
*** 447,453 ****
  	{
  	  if (stream == NULL)
  	    return EOF;
! #ifdef HAVE_GETC_UNLOCKED
  	  c = getc_unlocked (stream);
  #else
  	  c = getc (stream);
--- 447,453 ----
  	{
  	  if (stream == NULL)
  	    return EOF;
! #if defined(HAVE_GETC_UNLOCKED) && !defined(NEED_DECLARATION_GETC_UNLOCKED)
  	  c = getc_unlocked (stream);
  #else
  	  c = getc (stream);

Re: Question about another IA64 HP-UX build problem

[Daniel Jacobowitz]

On Thu, May 05, 2005 at 04:25:38PM -0700, Steve Ellcey wrote:
> > getc_unlocked isn't used anywhere else in binutils simply because no one
> > else has found a need for it, or perhaps, the idea of using it hasn't
> > occurred to anyone else yet.
> > -- 
> > Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com
> 
> Here is the patch I came up with to address the getc_unlocked problem.
> The problem is that HAVE_GETC_UNLOCKED is true (because HP-UX has it)
> but the declaration isn't seen because we didn't compile with
> -D_REENTRENT, thus we get a no-declaration error.  Tested on IA64 HP-UX.

If you're going to use BFD_NEED_DECLARATION, please use it to provide
the declaration instead :-)

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: Question about another IA64 HP-UX build problem

[Steve Ellcey]

> > Here is the patch I came up with to address the getc_unlocked problem.
> > The problem is that HAVE_GETC_UNLOCKED is true (because HP-UX has it)
> > but the declaration isn't seen because we didn't compile with
> > -D_REENTRENT, thus we get a no-declaration error.  Tested on IA64 HP-UX.
> 
> If you're going to use BFD_NEED_DECLARATION, please use it to provide
> the declaration instead :-)
> 
> -- 
> Daniel Jacobowitz
> CodeSourcery, LLC

I considered that, but I don't think I really want to do that.  I don't
believe I should use getc_unlocked unless I have the official OS
declaration for it.  If we were using a newer autoconf I would have
used 'AC_CHECK_DECLS(getc_unlocked)' and then my ifdef would have been:

#if defined(HAVE_GETC_UNLOCKED) && defined(HAVE_DECL_GETC_UNLOCKED)

which would look better but does the same thing.

The idea is to use getc_unlocked only if the OS provided a declaration
for it and not to use it if you just found it in libc but didn't see a
declaration for it.

On HP-UX the <stdio.h> header file will automatically convert getc into
getc_unlocked if you did not compile with -D_REENTRENT which tells the
compiler you are doing multithreaded stuff.  Without the -D_REENTRENT
getc is unlocked anyway and the getc_unlocked declaration is not
visible.

Steve Ellcey
sje@cup.hp.com

Re: Question about another IA64 HP-UX build problem

[Hans-Peter Nilsson]

On Thu, 5 May 2005, Steve Ellcey wrote:
> Yes, I have built this on ia64-hpux before.  It always gave a warning
> because it didn't see the declaration of getc_unlocked in stdio.h.  This
> is because _REENTRENT is not defined and the HP-UX header file only
> defines getc_unlocked when _REENTRENT is defined.

Not that I usually point out obvious but consistent typos but if
you don't mean s/_REENTRENT/_REENTRANT/ something is out of the
ordinary and might need fixing.  (And you might want to check
that the typo doesn't creep into your code. ;-)

brgds, H-P

Re: Question about another IA64 HP-UX build problem

[Steve Ellcey]

> On Thu, 5 May 2005, Steve Ellcey wrote:
> > Yes, I have built this on ia64-hpux before.  It always gave a warning
> > because it didn't see the declaration of getc_unlocked in stdio.h.  This
> > is because _REENTRENT is not defined and the HP-UX header file only
> > defines getc_unlocked when _REENTRENT is defined.
> 
> Not that I usually point out obvious but consistent typos but if
> you don't mean s/_REENTRENT/_REENTRANT/ something is out of the
> ordinary and might need fixing.  (And you might want to check
> that the typo doesn't creep into your code. ;-)
> 
> brgds, H-P

I did mean _REENTRANT.  That is what the header files look for, I don't
define anything when compiling, nor do I want to, so there was no typo
in my code.

Steve Ellcey
sje@cup.hp.com

Re: Question about another IA64 HP-UX build problem

[John David Anglin]

> On Thu, 5 May 2005, Steve Ellcey wrote:
> > Yes, I have built this on ia64-hpux before.  It always gave a warning
> > because it didn't see the declaration of getc_unlocked in stdio.h.  This
> > is because _REENTRENT is not defined and the HP-UX header file only
> > defines getc_unlocked when _REENTRENT is defined.

The same error occurs on hppa2.0w-hp-hpux11.11.  Adding -pthread to CFLAGS
will provide the define but that will also cause linking against libpthread.

The linux manpage advises not using the nonstandard unlocked variants:

CONFORMING TO
       The     four     functions     getc_unlocked(),     getchar_unlocked(),
       putc_unlocked(),  putchar_unlocked()  are  in POSIX.1.  The nonstandard
       *_unlocked() variants occur on a few Unix systems, and are available in
       recent glibc.  They should probably not be used.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)