Bug 14243 - software that includes bfd.h fails to build with "#error config.h must be included before this header"
Summary: software that includes bfd.h fails to build with "#error config.h must be inc...
Status: RESOLVED WONTFIX
Alias: None
Product: binutils
Classification: Unclassified
Component: binutils (show other bugs)
Version: 2.24
: P2 critical
Target Milestone: ---
Assignee: yuexu
URL:
Keywords:
: 15920 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-06-14 19:18 UTC by Maynard Johnson
Modified: 2014-11-19 02:57 UTC (History)
8 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Maynard Johnson 2012-06-14 19:18:10 UTC
A build of the oprofile package against a June 12, 2012 CVS snapshot of binutils fails with the following:

------------------------------------------------------------
make[2]: Entering directory `/home/cseo/at5.0/at5.0-5/src/oprofile/libopagent'
/bin/sh ../libtool --tag=CC   --mode=compile /opt/at5.0-5-rc1/bin/gcc -DHAVE_CONFIG_H -I. -I.. -I/home/cseo/at5.0/at5.0-5/src/oprofile/libopagent    -fPIC -I /home/cseo/at5.0/at5.0-5/src/oprofile/libop -I /home/cseo/at5.0/at5.0-5/src/oprofile/libutil -g -MT libopagent_la-opagent.lo -MD -MP -MF .deps/libopagent_la-opagent.Tpo -c -o libopagent_la-opagent.lo `test -f 'opagent.c' || echo '/home/cseo/at5.0/at5.0-5/src/oprofile/libopagent/'`opagent.c
libtool: compile:  /opt/at5.0-5-rc1/bin/gcc -DHAVE_CONFIG_H -I. -I.. -I/home/cseo/at5.0/at5.0-5/src/oprofile/libopagent -fPIC -I /home/cseo/at5.0/at5.0-5/src/oprofile/libop -I /home/cseo/at5.0/at5.0-5/src/oprofile/libutil -g -MT libopagent_la-opagent.lo -MD -MP -MF .deps/libopagent_la-opagent.Tpo -c opagent.c  -fPIC -DPIC -o .libs/libopagent_la-opagent.o
In file included from opagent.c:63:0:
/opt/at5.0-5-rc1/include/bfd.h:37:2: error: #error config.h must be included before this header
make[2]: Entering directory `/home/cseo/at5.0/at5.0-5/src/oprofile/libopagent'
/bin/sh ../libtool --tag=CC   --mode=compile /opt/at5.0-5-rc1/bin/gcc -DHAVE_CONFIG_H -I. -I.. -I/home/cseo/at5.0/at5.0-5/src/oprofile/libopagent    -fPIC -I /home/cseo/at5.0/at5.0-5/src/oprofile/libop -I /home/cseo/at5.0/at5.0-5/src/oprofile/libutil -g -MT libopagent_la-opagent.lo -MD -MP -MF .deps/libopagent_la-opagent.Tpo -c -o libopagent_la-opagent.lo `test -f 'opagent.c' || echo '/home/cseo/at5.0/at5.0-5/src/oprofile/libopagent/'`opagent.c
libtool: compile:  /opt/at5.0-5-rc1/bin/gcc -DHAVE_CONFIG_H -I. -I.. -I/home/cseo/at5.0/at5.0-5/src/oprofile/libopagent -fPIC -I /home/cseo/at5.0/at5.0-5/src/oprofile/libop -I /home/cseo/at5.0/at5.0-5/src/oprofile/libutil -g -MT libopagent_la-opagent.lo -MD -MP -MF .deps/libopagent_la-opagent.Tpo -c opagent.c  -fPIC -DPIC -o .libs/libopagent_la-opagent.o
In file included from opagent.c:63:0:
/opt/at5.0-5-rc1/include/bfd.h:37:2: error: #error config.h must be included before this header


This failure is apparently related to the binutils change that went in via bug # 14072.  This change apparently enforces that any file that includes "bfd.h" also includes "config.h".  As I understand it, this change was intended for binutils source files only since the "config.h" file being required is binutils' config.h.  Yet the change is improperly requiring external software to also include a "config.h" before including bfd.h.  For the case in question, oprofile's opagent.c does not include the oprofile's config.h and does not require it.  While we *could* hack opagent.c (and several other oprofile files that include bfd.h) to unnecessarily include oprofile's config.h, this seems to be the wrong fix to this problem.
Comment 1 Alan Modra 2012-06-15 00:33:54 UTC
This is a correctness issue.  bfd.h and the headers that bfd.h #include test at least one HAVE_* macro.  So you need to include the file that defines those HAVE_* macros before bfd.h.  You may argue that the use of HAVE_STRINGSIZE will never affect any host that builds oprofile.  While that may be true, it's still a correctness issue.  Future versions of bfd.h may test other HAVE_* macros.

This isn't about requiring any of the various binutils config.h files, but rather *an* autoconf generated config.h.  It's completely irrelevant that opagent.c itself does not need config.h.  opagent.c *and its headers* needs config.h.
Comment 2 Alan Modra 2012-06-15 02:14:04 UTC
A further comment.  I see HAVE_STRINGIZE is obsolescent according to the autoconf docs.  So my argument for config.h being included before the current version of bfd.h isn't really that strong.  However, it is a really good idea to always include config.h (or sysdep.h or alloca-conf.h that include config.h) before any other files, including system headers.  See bug #13558.

Beside all that, I can't see the binutils project reverting this change.  Yes, it causes some initial pain (and I grumbled about the change myself), but overall the change is a good one.  If you (oprofile project) want to insulate yourself from a change like this one, then you really should be distributing your own copy of bfd along with oprofile sources.  That would allow you to resolve these issues at your leisure.
Comment 3 Mike Frysinger 2012-06-15 03:08:30 UTC
we do compile with HAVE_CONFIG_H which many files respect, but last time i tried to increase usage of this define, i was shot down ...
Comment 4 Maynard Johnson 2012-06-15 15:15:56 UTC
(In reply to comment #2)
> A further comment.  I see HAVE_STRINGIZE is obsolescent according to the
> autoconf docs.  So my argument for config.h being included before the current
> version of bfd.h isn't really that strong.  However, it is a really good idea
> to always include config.h (or sysdep.h or alloca-conf.h that include config.h)
> before any other files, including system headers.  See bug #13558.
> 
> Beside all that, I can't see the binutils project reverting this change.  Yes,
> it causes some initial pain (and I grumbled about the change myself), but
> overall the change is a good one.  If you (oprofile project) want to insulate
> yourself from a change like this one, then you really should be distributing
> your own copy of bfd along with oprofile sources.  That would allow you to
> resolve these issues at your leisure.

OK, I'll post patches to the oprofile list to comply with this requirement.  You can close this bug report. Thanks.
Comment 5 Markus Trippelsdorf 2012-09-18 18:06:57 UTC
The same issue also breaks tools/perf of the Linux kernel tree:

perf % make
Makefile:668: No bfd.h/libbfd found, install binutils-dev[el]/zlib-static to gain symbol demangling
...
Comment 6 Mike Frysinger 2012-09-18 20:00:36 UTC
so send a patch to LKML to fix the issue
Comment 7 Markus Trippelsdorf 2012-09-18 20:38:55 UTC
(In reply to comment #6)
> so send a patch to LKML to fix the issue

If you do a quick google search with "/usr/include/bfd.h:37:2: error: #error config.h must", you'll see that perf is not the only project that got hit by this issue.

So instead of wasting everybody's time, the "#if !defined PACKAGE && !defined PACKAGE_VERSION" check should be removed from bfd.h before it gets installed.
Comment 8 Mike Frysinger 2012-09-18 22:00:39 UTC
modifying files at install time isn't a great idea.  using binutils libs outside of the binutils project is uncommon, so the overhead of changing other projects who aren't already using recent autotools isn't that big of a deal.
Comment 9 Dr. David Alan Gilbert 2012-12-26 02:04:49 UTC
For applications that don't use autotools what's the suggested fix?

It seems bad enough to change it to require all apps that use it to be changed, but it would be OK if binutils at least placed a file in it's isntall directory that could be the one included.

Dave
Comment 10 Mike Frysinger 2012-12-26 17:28:54 UTC
(In reply to comment #9)

people can #define PACKAGE "foo" just as easily as autotools
Comment 11 Mike Frysinger 2013-09-03 02:10:03 UTC
*** Bug 15920 has been marked as a duplicate of this bug. ***