| Message ID | mcrzk3dokii.fsf@google.com |
|---|---|
| State | New |
| Headers | show |
Hello Ian, On Tue, 23 Oct 2012 06:55:01 +0200, Ian Lance Taylor wrote: > PR 54918 points out that libgo is not using version numbers as it > should. At present none of libgo in 4.6, 4.7 and mainline are > compatible with each other. This patch to the 4.7 branch sets the > version number for libgo there. Bootstrapped and ran Go testsuite on > x86_64-unknown-linux-gnu. Committed to 4.7 branch. it has regressed GDB testsuite: -PASS: gdb.go/handcall.exp: print add (1, 2) +FAIL: gdb.go/handcall.exp: print add (1, 2) GNU gdb (GDB) 7.5.50.20121022-cvs before: (gdb) print add (1, 2) $1 = 3 (gdb) ptype add type = int32 (int, int) (gdb) info line add Line 219 of "../../../libgo/runtime/cpuprof.c" starts at address 0x7ffff55c0884 <tick+52> and ends at 0x7ffff55c0898 <tick+72>. now: (gdb) print add (1, 2) Too few arguments in function call. (gdb) ptype add type = void (Profile *, uintptr *, int32) (gdb) info line add Line 212 of "../../../gcc47/libgo/runtime/cpuprof.c" starts at address 0x7ffff55b05fe <add> and ends at 0x7ffff55b0609 <add+11>. Regards, Jan
On Tue, Oct 23, 2012 at 12:37 AM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote: > > On Tue, 23 Oct 2012 06:55:01 +0200, Ian Lance Taylor wrote: >> PR 54918 points out that libgo is not using version numbers as it >> should. At present none of libgo in 4.6, 4.7 and mainline are >> compatible with each other. This patch to the 4.7 branch sets the >> version number for libgo there. Bootstrapped and ran Go testsuite on >> x86_64-unknown-linux-gnu. Committed to 4.7 branch. > > it has regressed GDB testsuite: > -PASS: gdb.go/handcall.exp: print add (1, 2) > +FAIL: gdb.go/handcall.exp: print add (1, 2) > > GNU gdb (GDB) 7.5.50.20121022-cvs > > before: > (gdb) print add (1, 2) > $1 = 3 > (gdb) ptype add > type = int32 (int, int) > (gdb) info line add > Line 219 of "../../../libgo/runtime/cpuprof.c" starts at address 0x7ffff55c0884 <tick+52> and ends at 0x7ffff55c0898 <tick+72>. > > now: > (gdb) print add (1, 2) > Too few arguments in function call. > (gdb) ptype add > type = void (Profile *, uintptr *, int32) > (gdb) info line add > Line 212 of "../../../gcc47/libgo/runtime/cpuprof.c" starts at address 0x7ffff55b05fe <add> and ends at 0x7ffff55b0609 <add+11>. In both the before and after, gdb seems to be picking up the wrong version of add, according to the line number information. It's picking up the static function add defined in the libgo library, not the function defined in the user code. The type information is different but in fact the type of the static function in the libgo library has not changed since it was introduced in March, 2011. So somehow in the before case gdb is displaying the line number of the static function add but the type of the Go function add, but in the after case gdb is displaying the line number and the type of the static function. I don't have any explanation for this difference but it's hard for me to believe that the root cause is in libgo. In effect there are two functions named add: one is in libgo with C mangling, and one is in the user code with Go mangling. gdb is not picking the one that the testsuite expects it to pick. Ian
On 23.10.2012 06:55, Ian Lance Taylor wrote: > PR 54918 points out that libgo is not using version numbers as it > should. At present none of libgo in 4.6, 4.7 and mainline are > compatible with each other. This patch to the 4.7 branch sets the > version number for libgo there. Bootstrapped and ran Go testsuite on > x86_64-unknown-linux-gnu. Committed to 4.7 branch. changing the soname of a runtime library on the branch? I don't like this idea at all. Matthias
On Tue, Oct 23, 2012 at 06:16:25PM +0200, Matthias Klose wrote: > On 23.10.2012 06:55, Ian Lance Taylor wrote: > > PR 54918 points out that libgo is not using version numbers as it > > should. At present none of libgo in 4.6, 4.7 and mainline are > > compatible with each other. This patch to the 4.7 branch sets the > > version number for libgo there. Bootstrapped and ran Go testsuite on > > x86_64-unknown-linux-gnu. Committed to 4.7 branch. > > changing the soname of a runtime library on the branch? I don't like this idea > at all. Me neither. You should just make sure you increment the soname on the trunk whenever new major release has ABI changes, and don't do ABI changes on the release branches. Jakub
On Tue, Oct 23, 2012 at 9:27 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Tue, Oct 23, 2012 at 06:16:25PM +0200, Matthias Klose wrote: >> On 23.10.2012 06:55, Ian Lance Taylor wrote: >> > PR 54918 points out that libgo is not using version numbers as it >> > should. At present none of libgo in 4.6, 4.7 and mainline are >> > compatible with each other. This patch to the 4.7 branch sets the >> > version number for libgo there. Bootstrapped and ran Go testsuite on >> > x86_64-unknown-linux-gnu. Committed to 4.7 branch. >> >> changing the soname of a runtime library on the branch? I don't like this idea >> at all. > > Me neither. You should just make sure you increment the soname on the trunk > whenever new major release has ABI changes, and don't do ABI changes on the release > branches. The problem is that I forgot to do that when the 4.7 branch was created. So the 4.7 branch and the 4.6 branch were using the same SONAME although they had completely different ABIs. That is, there is no ABI change on the 4.7 branch. I'm setting the SONAME to distinguish it from the 4.6 branch. I agree is not ideal but it seems like the best approach under the circumstances. I'll roll it back if y'all continue to think it is a bad idea. Ian
On Tue, Oct 23, 2012 at 09:57:21AM -0700, Ian Lance Taylor wrote: > The problem is that I forgot to do that when the 4.7 branch was > created. So the 4.7 branch and the 4.6 branch were using the same > SONAME although they had completely different ABIs. > > That is, there is no ABI change on the 4.7 branch. I'm setting the > SONAME to distinguish it from the 4.6 branch. > > I agree is not ideal but it seems like the best approach under the > circumstances. I think it is too late for such a change on the 4.7 branch, better just say that the go support in 4.6 is experimental, without stable ABI, otherwise there will be ABI incompatibility also on the 4.7 branch between patchlevel versions thereof. Jakub
On Tue, Oct 23, 2012 at 9:59 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Tue, Oct 23, 2012 at 09:57:21AM -0700, Ian Lance Taylor wrote: >> The problem is that I forgot to do that when the 4.7 branch was >> created. So the 4.7 branch and the 4.6 branch were using the same >> SONAME although they had completely different ABIs. >> >> That is, there is no ABI change on the 4.7 branch. I'm setting the >> SONAME to distinguish it from the 4.6 branch. >> >> I agree is not ideal but it seems like the best approach under the >> circumstances. > > I think it is too late for such a change on the 4.7 branch, better > just say that the go support in 4.6 is experimental, without stable ABI, > otherwise there will be ABI incompatibility also on the 4.7 branch > between patchlevel versions thereof. OK, I reverted the patch on the 4.7 branch. Ian
Index: configure.ac =================================================================== --- configure.ac (revision 191576) +++ configure.ac (working copy) @@ -11,7 +11,7 @@ AC_INIT(package-unused, version-unused,, AC_CONFIG_SRCDIR(Makefile.am) AC_CONFIG_HEADER(config.h) -libtool_VERSION=1:0:0 +libtool_VERSION=2:1:0 AC_SUBST(libtool_VERSION) AM_ENABLE_MULTILIB(, ..) Index: Makefile.am =================================================================== --- Makefile.am (revision 192024) +++ Makefile.am (working copy) @@ -1753,7 +1753,8 @@ libgo_go_objs = \ libgo_la_SOURCES = $(runtime_files) -libgo_la_LDFLAGS = $(PTHREAD_CFLAGS) $(AM_LDFLAGS) +libgo_la_LDFLAGS = \ + -version-info $(libtool_VERSION) $(PTHREAD_CFLAGS) $(AM_LDFLAGS) libgo_la_LIBADD = \ $(libgo_go_objs) $(LIBFFI) $(PTHREAD_LIBS) $(MATH_LIBS) $(NET_LIBS)