Re: [PATCH] config/debuginfod.m4: Use PKG_CHECK_MODULES

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] config/debuginfod.m4: Use PKG_CHECK_MODULES

Tom Tromey-2
>>>>> "Aaron" == Aaron Merey via Binutils <[hidden email]> writes:

Aaron>         * Makefile.in: Replace LIBDEBUGINFOD with DEBUGINFOD_LIBS.
Aaron>         * aclocal.m4: Rebuild.

Instead of inlining pkg.m4 into aclocal.m4, how about making
config/pkg.m4 and then using m4_include from gdb/acinclude.m4?
That's what gdb does for other shared m4 code.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] config/debuginfod.m4: Use PKG_CHECK_MODULES

Tom Tromey-2
Simon> Since it's debuginfo.m4 that is using PKG_CHECK_MODULES, can you put the include
Simon> of pkg.m4 in debuginfo.m4, instead of in {binutils,gdb}/configure.ac?

Simon> Otherwise, from GDB's point of view I think it looks good, unless
Simon> Tom has some things to add.

I'm happy with it.  Thanks for persevering.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] config/debuginfod.m4: Use PKG_CHECK_MODULES

Sourceware - gdb-patches mailing list
On Tue, Jul 21, 2020 at 11:20 AM Tom Tromey <[hidden email]> wrote:
>
> Simon> Since it's debuginfo.m4 that is using PKG_CHECK_MODULES, can you put the include
> Simon> of pkg.m4 in debuginfo.m4, instead of in {binutils,gdb}/configure.ac?
>
> Simon> Otherwise, from GDB's point of view I think it looks good, unless
> Simon> Tom has some things to add.
>
> I'm happy with it.  Thanks for persevering.

Great. I can push to binutils-gdb but not gcc. Should I just push to
binutils-gdb for now or wait until the patch can be applied to both
repos at once?

Aaron

Reply | Threaded
Open this post in threaded view
|

Re: V2 [PATCH] PKG_CHECK_MODULES: Check if $pkg_cv_[]$1[]_LIBS works

Sourceware - gdb-patches mailing list
On Mon, Jul 27, 2020 at 12:32 PM H.J. Lu <[hidden email]> wrote:

>
> On Mon, Jul 27, 2020 at 12:14 PM H.J. Lu <[hidden email]> wrote:
> >
> > On Mon, Jul 27, 2020 at 9:11 AM Aaron Merey <[hidden email]> wrote:
> > >
> > > On Mon, Jul 27, 2020 at 11:32 AM H.J. Lu <[hidden email]> wrote:
> > > >
> > > > On Sat, Jul 25, 2020 at 9:01 AM H.J. Lu <[hidden email]> wrote:
> > > > > This caused:
> > > > >
> > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=26301
> > > > >
> > > >
> > > > It is quite normal to have debuginfod headers without libdebuginfod on
> > > > multilib OSes.  Restore AC_CHECK_LIB to check if libdebuginfod exists.
> > > > And always define HAVE_LIBDEBUGINFOD to 0 or 1 for
> > > >
> > > > binutils/dwarf.c:#if HAVE_LIBDEBUGINFOD
> > > > binutils/dwarf.c:#if HAVE_LIBDEBUGINFOD
> > > > binutils/dwarf.c:#if HAVE_LIBDEBUGINFOD
> > > > binutils/dwarf.h:#if HAVE_LIBDEBUGINFOD
> > > > binutils/objdump.c:#if HAVE_LIBDEBUGINFOD
> > > > binutils/objdump.c:#endif /* HAVE_LIBDEBUGINFOD */
> > > > binutils/readelf.c:#if HAVE_LIBDEBUGINFOD
> > > > binutils/readelf.c:#endif /* HAVE_LIBDEBUGINFOD */
> > > > gdb/top.c:#if HAVE_LIBDEBUGINFOD
> > > >
> > > > OK for master?
> > >
> > > Thanks for spotting this. Normally PKG_CHECH_MODULES would correctly
> > > detect whether the .so and header are installed and build accordingly,
> > > but when cross compiling the AC_CHECK_LIB may be needed.
> >
> > I am not cross compiling.  I am simply using "gcc -m32".   The problem
> > is PKG_CHECK_MODULES which doesn't check if $pkg_cv_[]$1[]_LIBS
> > actually works.   Here is the updated patch to fix PKG_CHECK_MODULES.
> > Any comments or objections?
> >
> >
>
> HAVE_LIBDEBUGINFOD is a separate issue.  Here is the updated patch
> which only adds AC_TRY_LINK to PKG_CHECK_MODULES to check if
> $pkg_cv_[]$1[]_LIBS works.
>

I am checking it in.

--
H.J.
Reply | Threaded
Open this post in threaded view
|

[PATCH] PKG_CHECK_MODULES: Properly check if $pkg_cv_[]$1[]_LIBS works

Sourceware - gdb-patches mailing list
In reply to this post by Sourceware - gdb-patches mailing list
On Tue, Jul 28, 2020 at 5:54 AM Mark Wielaard <[hidden email]> wrote:

>
> Hi,
>
> On Mon, 2020-07-27 at 12:32 -0700, H.J. Lu via Binutils wrote:
> > diff --git a/config/pkg.m4 b/config/pkg.m4
> > index 13a8890178..45587e97c8 100644
> > --- a/config/pkg.m4
> > +++ b/config/pkg.m4
> > @@ -147,6 +147,12 @@ AC_MSG_CHECKING([for $2])
> >  _PKG_CONFIG([$1][_CFLAGS], [cflags], [$2])
> >  _PKG_CONFIG([$1][_LIBS], [libs], [$2])
> >
> > +dnl Check whether $pkg_cv_[]$1[]_LIBS works.
> > +pkg_save_LDFLAGS="$LDFLAGS"
> > +LDFLAGS="$LDFLAGS $pkg_cv_[]$1[]_LIBS"
> > +AC_TRY_LINK([],[return 0;], [pkg_failed=no], [pkg_failed=yes])
> > +LDFLAGS=$pkg_save_LDFLAGS
> > +
> >  m4_define([_PKG_TEXT], [Alternatively, you may set the environment
> > variables $1[]_CFLAGS
> >  and $1[]_LIBS to avoid the need to call pkg-config.
> >  See the pkg-config man page for more details.])
>
> This hunk seems wrong. You are testing whether the $pkg_cv_[]$1[]_LIBS
> flags work, but they might be empty if the library wasn't found
> (pkg_failed=yes). Then the AC_TRY_LINK will obviously succeed, and then
> you set pkg_failed=no. But that indicates that the library was found.
>
This fixes it.

Thanks.

--
H.J.

0001-PKG_CHECK_MODULES-Properly-check-if-pkg_cv_-1-_LIBS-.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: V2 [PATCH] PKG_CHECK_MODULES: Check if $pkg_cv_[]$1[]_LIBS works

Sourceware - gdb-patches mailing list
In reply to this post by Sourceware - gdb-patches mailing list
On Tue, Jul 28, 2020 at 7:01 AM Simon Marchi <[hidden email]> wrote:

>
> On 2020-07-28 9:56 a.m., H.J. Lu wrote:
> > On Tue, Jul 28, 2020 at 6:51 AM Andreas Schwab <[hidden email]> wrote:
> >>
> >> On Jul 28 2020, H.J. Lu via Binutils wrote:
> >>
> >>> On x86, the native GCC can support -m32 and -m64.  "gcc -m32" or "gcc -m64"
> >>>  are not cross compiling.
> >>
> >> You cannot link -m64 and -m32 together.
> >>
> >>> I didn't set PKG_CONFIG_LIBDIR and I don't want to set it.
> >>
> >> Then use the correct pkg-config for your target.  If you think
> >> pkg-config is broken, then fix _that_.
> >>
> >
> > I did:
> >
> > RUNTESTFLAGS="--target_board 'unix{-m32}'" CC="gcc -m32 -fno-lto
> > -fcf-protection"
> >  CXX="g++ -fno-lto -m32 -fcf-protection" /exp
> > ort/gnu/import/git/gitlab/x86-binutils/configure \
> > --enable-targets=x86_64-linux \
> > i686-linux \
> > --enable-plugins --disable-gdb --disable-gdbserver --disable-libdecnumbe
> > r --disable-readline --disable-sim --with-sysroot=/ --with-system-zlib \
> > --prefix=/usr/local \
> > --with-local-prefix=/usr/local
> > configure: WARNING: you should use --build, --host, --target
> > checking build system type... i686-pc-linux-gnu
> > checking host system type... i686-pc-linux-gnu
> > checking target system type... i686-pc-linux-gnu
>
> So... is your build system a 32-bit one?  Why does the above say i686-pc-linux-gnu
> and not x86_64-something?
>

My system supports both -m32 and -m64.  Depending on CC, configure
selects i686 or x86-64 target.

--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: V2 [PATCH] PKG_CHECK_MODULES: Check if $pkg_cv_[]$1[]_LIBS works

Andreas Schwab-2
On Jul 28 2020, H.J. Lu wrote:

> My system supports both -m32 and -m64.

My system also supports m68k.  That doesn't make it "native".

$ file hello.m68k
hello.m68k: ELF 32-bit MSB executable, Motorola m68k, 68020, version 1 (SYSV), dynamically linked, interpreter /lib/ld., for GNU/Linux 2.6.32, BuildID[sha1]=becec3e5b8309f88c861d0a1725586cbdc5ecfc9, with debug_info, not stripped
$ ./hello.m68k
Hello World!

Andreas.

--
Andreas Schwab, [hidden email]
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Reply | Threaded
Open this post in threaded view
|

Re: V2 [PATCH] PKG_CHECK_MODULES: Check if $pkg_cv_[]$1[]_LIBS works

Sourceware - gdb-patches mailing list
In reply to this post by Sourceware - gdb-patches mailing list
On Tue, Jul 28, 2020 at 7:34 AM Simon Marchi <[hidden email]> wrote:

>
> On 2020-07-28 10:11 a.m., H.J. Lu wrote:
> > On Tue, Jul 28, 2020 at 7:01 AM Simon Marchi <[hidden email]> wrote:
> >>
> >> On 2020-07-28 9:56 a.m., H.J. Lu wrote:
> >>> On Tue, Jul 28, 2020 at 6:51 AM Andreas Schwab <[hidden email]> wrote:
> >>>>
> >>>> On Jul 28 2020, H.J. Lu via Binutils wrote:
> >>>>
> >>>>> On x86, the native GCC can support -m32 and -m64.  "gcc -m32" or "gcc -m64"
> >>>>>  are not cross compiling.
> >>>>
> >>>> You cannot link -m64 and -m32 together.
> >>>>
> >>>>> I didn't set PKG_CONFIG_LIBDIR and I don't want to set it.
> >>>>
> >>>> Then use the correct pkg-config for your target.  If you think
> >>>> pkg-config is broken, then fix _that_.
> >>>>
> >>>
> >>> I did:
> >>>
> >>> RUNTESTFLAGS="--target_board 'unix{-m32}'" CC="gcc -m32 -fno-lto
> >>> -fcf-protection"
> >>>  CXX="g++ -fno-lto -m32 -fcf-protection" /exp
> >>> ort/gnu/import/git/gitlab/x86-binutils/configure \
> >>> --enable-targets=x86_64-linux \
> >>> i686-linux \
> >>> --enable-plugins --disable-gdb --disable-gdbserver --disable-libdecnumbe
> >>> r --disable-readline --disable-sim --with-sysroot=/ --with-system-zlib \
> >>> --prefix=/usr/local \
> >>> --with-local-prefix=/usr/local
> >>> configure: WARNING: you should use --build, --host, --target
> >>> checking build system type... i686-pc-linux-gnu
> >>> checking host system type... i686-pc-linux-gnu
> >>> checking target system type... i686-pc-linux-gnu
> >>
> >> So... is your build system a 32-bit one?  Why does the above say i686-pc-linux-gnu
> >> and not x86_64-something?
> >>
> >
> > My system supports both -m32 and -m64.  Depending on CC, configure
> > selects i686 or x86-64 target.
>
> Can you clarify how this magic works, is this standard autoconf?  Because I am trying this
> on Fedora, so pretty much the same setup as you, and I don't see this behavior:
>
> $ /home/simark/src/binutils-gdb/configure CC="gcc -m32" CXX="g++ -m32"
> checking build system type... x86_64-pc-linux-gnu
> checking host system type... x86_64-pc-linux-gnu
> checking target system type... x86_64-pc-linux-gnu
> ...
>

I checked it again.  I also passed i686-linux, not  --host=,  to configure.

>
> And even if it worked, why would it set "build" to i686, it doesn't make sense.  The gcc you
> compile with, and its environment, is still x86_64, not i686.
>
> So *if* it works, it would be a shortcut for setting --host=i686-something, maybe.  So
> regardless of how that above works, that doesn't remove the need to configure pkg-config
> correctly for the host system.
>
> If you don't want to learn about with pkg-config and deal with it, then please say "I think
> we should not use pkg-config", and ideally give supporting points.  Please don't unilaterally
> push patches just to paper over your own problems.  You just make it so that somebody will
> need to untangle more mess later.

I am OK to remove pkg.m4.

--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: V2 [PATCH] PKG_CHECK_MODULES: Check if $pkg_cv_[]$1[]_LIBS works

Sourceware - gdb-patches mailing list
On Tue, Jul 28, 2020 at 8:13 AM Simon Marchi <[hidden email]> wrote:

>
> On 2020-07-28 11:05 a.m., H.J. Lu via Gdb-patches wrote:
> >> Can you clarify how this magic works, is this standard autoconf?  Because I am trying this
> >> on Fedora, so pretty much the same setup as you, and I don't see this behavior:
> >>
> >> $ /home/simark/src/binutils-gdb/configure CC="gcc -m32" CXX="g++ -m32"
> >> checking build system type... x86_64-pc-linux-gnu
> >> checking host system type... x86_64-pc-linux-gnu
> >> checking target system type... x86_64-pc-linux-gnu
> >> ...
> >>
> >
> > I checked it again.  I also passed i686-linux, not  --host=,  to configure.
>
> Ok I see, the configure line you pasted was wrapped by your email client so was not very readable.
>
> Doing `./configure <triplet>` looks like a deprecated way to set all build/host/target, as the
> warning message it shows implies:
>
>   configure: WARNING: you should use --build, --host, --target
>
> Anyway, my point still stands: the problem is you not using a pkg-config configured properly for
> the cross compilation you are attempting, not pkg.m4.
>
> >>
> >> And even if it worked, why would it set "build" to i686, it doesn't make sense.  The gcc you
> >> compile with, and its environment, is still x86_64, not i686.
> >>
> >> So *if* it works, it would be a shortcut for setting --host=i686-something, maybe.  So
> >> regardless of how that above works, that doesn't remove the need to configure pkg-config
> >> correctly for the host system.
> >>
> >> If you don't want to learn about with pkg-config and deal with it, then please say "I think
> >> we should not use pkg-config", and ideally give supporting points.  Please don't unilaterally
> >> push patches just to paper over your own problems.  You just make it so that somebody will
> >> need to untangle more mess later.
> >
> > I am OK to remove pkg.m4.
>
> Well I am not.  I find it quite handy to avoid having to hardcode necessary CFLAGS and LDFLAGS
> required to build against a library, so I think it's better to use pkg-config if the libraries
> we want to use provide a .pc file.
>
> I propose that we revert the patch for now to go back to the pristing pkg.m4 version.
>

What doesn't work with my pkg.m4 change?


--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: V2 [PATCH] PKG_CHECK_MODULES: Check if $pkg_cv_[]$1[]_LIBS works

Sourceware - gdb-patches mailing list
On Tue, Jul 28, 2020 at 9:28 AM Simon Marchi <[hidden email]> wrote:
>
> On 2020-07-28 12:07 p.m., H.J. Lu via Gdb-patches wrote:
> > What doesn't work with my pkg.m4 change?
>
> (1) It deviates from upstream.  I don't think we should do this unless
>     absolutely needed.  That's not the case here, the change is just there
>     because you don't want to set up pkg-config properly for cross-compiling.

Since when binutils can't fix issues in other packages?

> (2) I don't think it's necessarily bad to try to do a link to confirm the lib
>     is indeed there, but as I said earlier I don't think that doing as if the
>     package was not there is the right response.  This can happen if you have
>     a mis-configured pkg-config (like you have) or a broken installation (for
>     example, the .pc is there but the lib is not).  In either case, there's
>     something wrong with the build environment and I think it's more useful
>     to abort and tell the user rather than silently failing.
>
> Point 2 should be discussed upstream anyway, there's no point making a decision
> local to binutils-gdb.
>

Unlike gdb, binutils should have as few external depecies as possible.
debuginfod brings in some so many external depecies.

--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: V2 [PATCH] PKG_CHECK_MODULES: Check if $pkg_cv_[]$1[]_LIBS works

Simon Marchi-4
On 2020-07-28 2:31 p.m., H.J. Lu wrote:

>>> Unlike gdb, binutils should have as few external depecies as possible.
>>> debuginfod brings in some so many external depecies.
>>
>> I'm not a binutils maintainer, so that's not my role to decide about that
>> tradeoff... but we are talking about having an optional (only needed when
>> enabling support for libdebuginfod) *build* dependency on a quite standard
>> tool.  That's not very demanding.
>>
>> If you don't want to deal with libdebuginfod, you can also just build with
>> --without-debuginfod.
>
> My binutils script had been working fine until pkg.m4 was added

Ok but... that doesn't mean anything.  I think we made it quite clear that the
issue is with your build environment, not the build system (pkg.m4).

>Can it be moved to gdb directory?

It can, but I don't think it would be a good idea.  It would just be confusing
for binutils and GDB to both use libdebuginfod but use different methods of
finding it.  Somebody building binutils + GDB with libdebuginfod support against
a libdebuginfod in a non-default location would have to specify the location of
the library in two different ways.

Simon
Reply | Threaded
Open this post in threaded view
|

Re: V2 [PATCH] PKG_CHECK_MODULES: Check if $pkg_cv_[]$1[]_LIBS works

Sourceware - gdb-patches mailing list
On Tue, Jul 28, 2020 at 11:47 AM Simon Marchi <[hidden email]> wrote:

>
> On 2020-07-28 2:31 p.m., H.J. Lu wrote:
> >>> Unlike gdb, binutils should have as few external depecies as possible.
> >>> debuginfod brings in some so many external depecies.
> >>
> >> I'm not a binutils maintainer, so that's not my role to decide about that
> >> tradeoff... but we are talking about having an optional (only needed when
> >> enabling support for libdebuginfod) *build* dependency on a quite standard
> >> tool.  That's not very demanding.
> >>
> >> If you don't want to deal with libdebuginfod, you can also just build with
> >> --without-debuginfod.
> >
> > My binutils script had been working fine until pkg.m4 was added
>
> Ok but... that doesn't mean anything.  I think we made it quite clear that the
> issue is with your build environment, not the build system (pkg.m4).

Binutils configure script is supposed to detect if a feature is usable.
In my perspective, pkg.m4 failed on my build environment.

> >Can it be moved to gdb directory?
>
> It can, but I don't think it would be a good idea.  It would just be confusing
> for binutils and GDB to both use libdebuginfod but use different methods of
> finding it.  Somebody building binutils + GDB with libdebuginfod support against
> a libdebuginfod in a non-default location would have to specify the location of
> the library in two different ways.
>
> Simon



--
H.J.