| Message ID | 20180909163750.14196-2-romain.naour@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] package/gdb: move patch directory | expand |
Hello, On Sun, 9 Sep 2018 18:37:50 +0200, Romain Naour wrote: > Use the same workaround [1] as gnulib use to get the original > definition of stat. Otherwise with musl toolchains, gnulib try to use > rpl_stat which is not defined. > > Fixes: > https://gitlab.com/free-electrons/toolchains-builder/-/jobs/95552308 > > [1] http://git.savannah.gnu.org/cgit/gnulib.git/commit/lib/stat.c?id=c9d72f69bd201a1ab31464d91f234ea1817fe0e1 > > Signed-off-by: Romain Naour <romain.naour@gmail.com> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> I am confused by this patch. Why do we need that? The <sys/stat.h> on my system doesn't test __need_system_sys_stat_h. Is this a workaround to force gnulib to not provide its own stat() replacement ? Why is gnulib misbehaving here ? We have tons of gnulib related hacks in gdb.mk, and this start to pile up quite a bit. Why do we have all those gnulib issues with gdb ? Why not with tons of other packages that also use gnulib ? > +Use the same workaround [1] as gnulib use to get the original > +definition of stat. Otherwise with musl toolchains, gnulib try to use > +rpl_stat which is not defined. Well rpl_stat() is supposed to be implemented by gnulib. So basically gnulib tells gdb: please don't use stat() but my rpl_stat() wrapper, but then gnulib doesn't provide rpl_stat(). Any idea what's happening here ? Thomas
Hi Thomas, Adding the gdb-patches ml and Rich Felker in Cc. Le 10/09/2018 à 17:49, Thomas Petazzoni a écrit : > Hello, > > On Sun, 9 Sep 2018 18:37:50 +0200, Romain Naour wrote: >> Use the same workaround [1] as gnulib use to get the original >> definition of stat. Otherwise with musl toolchains, gnulib try to use >> rpl_stat which is not defined. >> >> Fixes: >> https://gitlab.com/free-electrons/toolchains-builder/-/jobs/95552308 >> >> [1] http://git.savannah.gnu.org/cgit/gnulib.git/commit/lib/stat.c?id=c9d72f69bd201a1ab31464d91f234ea1817fe0e1 >> >> Signed-off-by: Romain Naour <romain.naour@gmail.com> >> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > > I am confused by this patch. Why do we need that? The <sys/stat.h> on > my system doesn't test __need_system_sys_stat_h. Is this a workaround > to force gnulib to not provide its own stat() replacement ? > > Why is gnulib misbehaving here ? We have tons of gnulib related hacks > in gdb.mk, and this start to pile up quite a bit. Why do we have all > those gnulib issues with gdb ? Why not with tons of other packages that > also use gnulib ? There are too many questions here, I can't answer. There are some (old) hack with coreutils like gl_cv_func_gettimeofday_clobber which is in Buildroot since a long time. I can't tell for every gnulib based packages... > >> +Use the same workaround [1] as gnulib use to get the original >> +definition of stat. Otherwise with musl toolchains, gnulib try to use >> +rpl_stat which is not defined. > > Well rpl_stat() is supposed to be implemented by gnulib. So basically > gnulib tells gdb: please don't use stat() but my rpl_stat() wrapper, > but then gnulib doesn't provide rpl_stat(). > > Any idea what's happening here ? As far I can tell, the regression has been introduced by this commit: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=2441702a72f324e41a1624dc042b334f375e2d81 Best regards, Romain > > Thomas >
Hello, On Mon, 10 Sep 2018 18:41:28 -0400, Rich Felker wrote: > I'm not aware of all the context, but it looks like different source > files disagree on whether gnulib has replaced stat or not -- the > gnulib source file thinks it hasn't, so the rpl_stat function isn't > defined, but gdb's common-utils-ipa.c file (or rather the gnulib > stat.h included into it?) thinks it has been replaced and is trying to > use the replacement. This is likely the result of an incorrect hack > somewhere. Do you know if it happens with upstream gdb and musl or > just in buildroot's package? Well, Buildroot is using upstream musl and gdb. For both packages, we have only very few patches: https://git.buildroot.org/buildroot/tree/package/musl/ https://git.buildroot.org/buildroot/tree/package/gdb/8.1.1/ Note that we already have a number of gnulib related hacks in gdb.mk: https://git.buildroot.org/buildroot/tree/package/gdb/gdb.mk#n77 Best regards, Thomas
Hello, On Sun, 9 Sep 2018 18:37:50 +0200, Romain Naour wrote: > Use the same workaround [1] as gnulib use to get the original > definition of stat. Otherwise with musl toolchains, gnulib try to use > rpl_stat which is not defined. > > Fixes: > https://gitlab.com/free-electrons/toolchains-builder/-/jobs/95552308 > > [1] http://git.savannah.gnu.org/cgit/gnulib.git/commit/lib/stat.c?id=c9d72f69bd201a1ab31464d91f234ea1817fe0e1 > > Signed-off-by: Romain Naour <romain.naour@gmail.com> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> Upstream has fixed the issue. Could you try to backport upstream commit 3c025cfe5efc44eb4dfb03b53dca28e75096dd1e instead, and see if it solves the problem ? Thanks! Thomas
diff --git a/package/gdb/8.1.1/0006-gdb-common-utils-use-stat-privided-by-the-system.patch b/package/gdb/8.1.1/0006-gdb-common-utils-use-stat-privided-by-the-system.patch new file mode 100644 index 0000000000..650d963b9e --- /dev/null +++ b/package/gdb/8.1.1/0006-gdb-common-utils-use-stat-privided-by-the-system.patch @@ -0,0 +1,37 @@ +From a7848d4244ffa8db7bf4ab0d5152f57e71600420 Mon Sep 17 00:00:00 2001 +From: Romain Naour <romain.naour@gmail.com> +Date: Sun, 9 Sep 2018 12:53:34 +0200 +Subject: [PATCH] gdb/common-utils: use stat() privided by the system + +Use the same workaround [1] as gnulib use to get the original +definition of stat. Otherwise with musl toolchains, gnulib try to use +rpl_stat which is not defined. + +Fixes: +https://gitlab.com/free-electrons/toolchains-builder/-/jobs/95552308 + +[1] http://git.savannah.gnu.org/cgit/gnulib.git/commit/lib/stat.c?id=c9d72f69bd201a1ab31464d91f234ea1817fe0e1 + +Signed-off-by: Romain Naour <romain.naour@gmail.com> +--- + gdb/common/common-utils.c | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/gdb/common/common-utils.c b/gdb/common/common-utils.c +index 80de826ba78..d3577c7ff5c 100644 +--- a/gdb/common/common-utils.c ++++ b/gdb/common/common-utils.c +@@ -20,7 +20,10 @@ + #include "common-defs.h" + #include "common-utils.h" + #include "host-defs.h" ++/* Get the original definition of stat. It might be defined as a macro. */ ++#define __need_system_sys_stat_h + #include <sys/stat.h> ++#undef __need_system_sys_stat_h + #include <ctype.h> + + /* The xmalloc() (libiberty.h) family of memory management routines. +-- +2.14.4 +
Use the same workaround [1] as gnulib use to get the original definition of stat. Otherwise with musl toolchains, gnulib try to use rpl_stat which is not defined. Fixes: https://gitlab.com/free-electrons/toolchains-builder/-/jobs/95552308 [1] http://git.savannah.gnu.org/cgit/gnulib.git/commit/lib/stat.c?id=c9d72f69bd201a1ab31464d91f234ea1817fe0e1 Signed-off-by: Romain Naour <romain.naour@gmail.com> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> --- ...mon-utils-use-stat-privided-by-the-system.patch | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 package/gdb/8.1.1/0006-gdb-common-utils-use-stat-privided-by-the-system.patch