Discussion:
[PATCH 0/9] Gccgo port to s390[x] -- part I
Dominik Vogt
2014-09-09 12:44:47 UTC
Permalink
The following series' of patches introduce s390[x] support for
Gccgo. For practical reasons they will be submitted in separate
parts. This is the first part of the submissions with
architecture independent bug fixes and enhancements that affect
the following directories:

gcc (Gcc)
gcc/testsuite/gcc.misc-tests (Gcc)
gcc/go/gofrontend (gofrontend)
gcc/testsuite/go.test (golang)
libgo (Gcc?)
libgo/go (gofrontend)
libgo/runtime (gofrontend)

It seemed infeasible to split this patch series and send the
patches only to the "right" places. The following s390[x] patch
series' depend on all of them.

Summary of this series:

0001: Fix for bug #60406. (gofrontend)
0002: Test case for 0001. (golang)
0003: Bug fix for opening s390 libs. (gofrontend)
0004: Libgo compile fix. (gofrontend)
0005: Cleanup of x86 relocations in libgo. (gofrontend, optional)
0006: Fix in libgo/runtime/proc.c (Gcc?)
0007: Extend -fdump-go-spec (bitfields, unions and a bugfix) (Gcc)
0008: Extend -fdump-go-spec (complex types) (Gcc, optional)
0009: Fix handling of whitespace in configure script. (Gcc?)

Ciao

Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
Dominik Vogt
2014-09-09 12:48:26 UTC
Permalink
This patch fixes bug 60406 in an architecture independent way.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60406

The key change in this patch is to use
_Unwind_FindEnclosingFunction() to identify the defering function.
However, this does not work on platforms that still use SJLJ
exceptions.

ChangeLog
2014-09-05 Dominik Vogt <***@linux.vnet.ibm.com>

* libgo/runtime/proc.c (runtime_main): Initialize new structure fields.
* libgo/runtime/go-recover.c (__go_can_recover): Rewrite logic to detect
recover by return address.
* libgo/runtime/go-defer.c (__go_set_defer_retaddr): Removed.
(__go_set_defering_fn): Replacement for __go_set_defer_retaddr.
(__go_defer): Initialize new structure fields.
* libgo/runtime/go-defer.h (struct __go_defer_stack): Replace __retaddr
with __defering_fn.

gcc/go/ChangeLog
2014-09-05 Dominik Vogt <***@linux.vnet.ibm.com>

* gofrontend/statements.cc (build_thunk): The thunk uses the new
libgo runtime runctions __go_set_defering_fn and no longer depends
on the fake label and fake return value.
* gofrontend/runtime.def: Replace SET_DEFER_RETADDR with
SET_DEFERING_FN.

Ciao

Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
Dominik Vogt
2014-09-09 12:49:42 UTC
Permalink
A test case added to golang for the previous patch.

gcc/testsuite/ChangeLog
2014-09-05 Dominik Vogt <***@linux.vnet.ibm.com>

* go.test/test/recover.go (test1): Test recover() from deferred
recursive function.

Ciao

Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
Dominik Vogt
2014-09-09 12:51:31 UTC
Permalink
The current Gccgo cannot handle 64 bit symbol tables on s390[x].
The attached patch fixes that.

gcc/go/ChangeLog
2014-09-05 Dominik Vogt <***@linux.vnet.ibm.com>

* gofrontend/import-archive.cc (interpret_header): Recognize 64-bit
symbol tables ("/SYM64/ ").

Ciao

Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
Ian Lance Taylor
2014-10-03 18:27:58 UTC
Permalink
Post by Dominik Vogt
The current Gccgo cannot handle 64 bit symbol tables on s390[x].
The attached patch fixes that.
gcc/go/ChangeLog
* gofrontend/import-archive.cc (interpret_header): Recognize 64-bit
symbol tables ("/SYM64/ ").
Thanks. Committed.

Ian
Dominik Vogt
2014-09-09 12:53:37 UTC
Permalink
This patch fixes the compiler flags in libgo/mksysinfo.sh. In one
place, some compiler flags were missing that are consistently used
elswhere, resulting in an error message.

ChangeLog
2014-09-05 Dominik Vogt <***@linux.vnet.ibm.com>

* libgo/mksysinfo.sh (OUT):
Add the same compile flags that configure uses to detect whether off64_t
is present or not when generating the go structures for the C types.
Otherwise the go type for off64_t may not be generated.


Ciao

Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
Ian Lance Taylor
2014-10-04 01:19:46 UTC
Permalink
Post by Dominik Vogt
This patch fixes the compiler flags in libgo/mksysinfo.sh. In one
place, some compiler flags were missing that are consistently used
elswhere, resulting in an error message.
ChangeLog
Add the same compile flags that configure uses to detect whether off64_t
is present or not when generating the go structures for the C types.
Otherwise the go type for off64_t may not be generated.
I don't understand why this patch is necessary. The invocation of
mksysinfo in libgo/Makefile.am sets the CC environment variable to
include $(OSCFLAGS), and OSCFLAGS should include those three options.

Trying to list the flags separately in mksysinfo.sh is going to be
incomplete on some systems--see OSCFLAGS in libgo/configure.ac.

Can you explain further why this patch is required? Thanks.

Ian
Dominik Vogt
2014-10-06 06:51:54 UTC
Permalink
Post by Ian Lance Taylor
Post by Dominik Vogt
This patch fixes the compiler flags in libgo/mksysinfo.sh. In one
place, some compiler flags were missing that are consistently used
elswhere, resulting in an error message.
ChangeLog
Add the same compile flags that configure uses to detect whether off64_t
is present or not when generating the go structures for the C types.
Otherwise the go type for off64_t may not be generated.
I don't understand why this patch is necessary. The invocation of
mksysinfo in libgo/Makefile.am sets the CC environment variable to
include $(OSCFLAGS), and OSCFLAGS should include those three options.
Trying to list the flags separately in mksysinfo.sh is going to be
incomplete on some systems--see OSCFLAGS in libgo/configure.ac.
With gcc-4.8 compilation failed on s390 (i.e. 31 bit) without it.
It seems that this patch is no longer necessary with gcc-5.0, so
it can be dropped.

Ciao

Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
Dominik Vogt
2014-09-09 12:56:10 UTC
Permalink
This optional cleanup patch fixes some sloppy programming in the
x86 libgo/go/debug/elf library that had given me a very hard time
to debug and fix when porting the code to s390[x]. See commit
comment for details.

ChangeLog
2014-09-05 Dominik Vogt <***@linux.vnet.ibm.com>

* libgo/go/debug/elf/file.go (applyRelocationsAMD64):
Fix the calculation of some relocations; do not assume that the symbol
value is always zero.

Ciao

Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
Ian Lance Taylor
2014-10-04 01:25:55 UTC
Permalink
Post by Dominik Vogt
This optional cleanup patch fixes some sloppy programming in the
x86 libgo/go/debug/elf library that had given me a very hard time
to debug and fix when porting the code to s390[x]. See commit
comment for details.
ChangeLog
Fix the calculation of some relocations; do not assume that the symbol
value is always zero.
The code checks that it is using only STT_SECTION symbols. An
STT_SECTION symbol in an object file can be reasonably expected to
have a value of zero. Since in practice this only applies to debug
sections, I doubt it would work at all if the STT_SECTION symbol had a
non-zero value and that value were added in. So I'm not convinced
that this patch is necessary.

I'm sorry that the code gave you a hard time, but as far as I can see
it's not sloppy and it's not wrong.

Ian
Dominik Vogt
2014-10-06 07:42:30 UTC
Permalink
Post by Ian Lance Taylor
Post by Dominik Vogt
Fix the calculation of some relocations; do not assume that the symbol
value is always zero.
The code checks that it is using only STT_SECTION symbols. An
STT_SECTION symbol in an object file can be reasonably expected to
have a value of zero. Since in practice this only applies to debug
sections, I doubt it would work at all if the STT_SECTION symbol had a
non-zero value and that value were added in. So I'm not convinced
that this patch is necessary.
Well, my x86 knowledge is limited, and the patch is neither
required for the s390[x] port nor (at the moment) for x64_64.

On s390[x] the symbol value of a section symbol is definitely not
zero. In the amd64 Abi I cannot see anything that would enforce
that sym.Value is 0 for section symbols, although it seems to be
the case on the tested x86_64 system. In any case, if the patch
is not committed, we should at least add a comment that the symbol
value is omitted from the calculation because it is assumed to be
zero.

Ciao

Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
Ian Lance Taylor
2014-10-06 14:29:33 UTC
Permalink
Post by Dominik Vogt
On s390[x] the symbol value of a section symbol is definitely not
zero.
Is true even in an object file? I agree that in an executable a
section symbol will have a non-zero value, but that case doesn't arise
since an executable won't have (non-dynamic) relocations. But I'm
quite surprised that hear that the section symbol would be non-zero in
an object file.

Ian
Dominik Vogt
2014-10-07 10:45:17 UTC
Permalink
Post by Ian Lance Taylor
Post by Dominik Vogt
On s390[x] the symbol value of a section symbol is definitely not
zero.
Is true even in an object file?
No.
Post by Ian Lance Taylor
I agree that in an executable a
section symbol will have a non-zero value, but that case doesn't arise
since an executable won't have (non-dynamic) relocations. But I'm
quite surprised that hear that the section symbol would be non-zero in
an object file.
I spent a day looking at that issue again, and while it's true
that section symbols don't necessarily have a zero value, that is
not the problem here. The problem is about how cgo determines the
names of functions(?) from an object file. On s390 we need to do
an indirect lookup of (non-section-)symbols to find the names, and
the symbol value is not zero.

The only points in that patch are that on one hand - as far as I
know - the Abi does not guarantee that section symbols are either
zero or not relocated, even if that may be the case in reality.
And on the other hand, if that code is ever modified to handle
non-section symbols, it's not obvious that you not only need to
remove the test for the symbol type but also modify the
calculations below. So, apply the patch or drop it as you like,
but in any case, at least a comment in the code would improve
maintainability.

Ciao

Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
Dominik Vogt
2014-09-09 12:58:40 UTC
Permalink
Eases the rediculously tight minimum stack size for goprocesses on
64 bit systems.

ChangeLog
2014-09-05 Dominik Vogt <***@linux.vnet.ibm.com>

* libgo/runtime/proc.c (runtime_newosproc): Set the thread stack size
explicitly only on 32 bit systems.

Ciao

Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
Dominik Vogt
2014-09-09 13:02:07 UTC
Permalink
This patch extends the -fdump-go-spec option to handle bitfields
and unions and fixes handlinx of zero length arrays. All of this
is necessary for s390[x] since the system headers use these
features. Please check the commit comment for a detailed
description of the patch.

gcc/ChangeLog
-------------
2014-09-05 Dominik Vogt <***@linux.vnet.ibm.com>

* godump.c (precision_to_units): New helper function.
(go_append_artificial_name): Ditto.
(go_append_decl_name): Ditto.
(go_append_bitfield): Ditto.
(go_get_uinttype_for_precision): Ditto.
(go_append_padding): Ditto.
(go_force_record_alignment): Ditto.
(go_format_type): Represent unions with an array of uints of the size
of the alignment in go. This fixes the 'random' size of the union's
representation using just the first field.
(go_format_type): Add argument that indicates whether a record is
nested (used for generation of artificial go names).
(go_output_fndecl): Adapt to new go_format_type signature.
(go_output_typedef): Ditto.
(go_output_var): Ditto.
(go_output_var): Prefer to output type as alias (typedef).
(go_format_type): Bitfields in records are simulated as arrays of bytes
in go.

2014-09-05 Dominik Vogt <***@linux.vnet.ibm.com>

* godump.c (go_format_type): Fix handling of arrays with zero elements.

gcc/testsuite/ChangeLog
-----------------------
2014-09-05 Dominik Vogt <***@linux.vnet.ibm.com>

* gcc.misc-tests/godump-1.c: Add tests for bitfields and unions.

2014-09-05 Dominik Vogt <***@linux.vnet.ibm.com>

* gcc.misc-tests/godump.exp: New.
* gcc.misc-tests/godump-1.c: New.

Ciao

Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
Dominik Vogt
2014-09-09 13:04:14 UTC
Permalink
This is an optional extension of the -fgo-dump-spec option to
handle copmplex data types. It is not necessary for s390[x] but
was easy to write on top of the previous patch.

gcc/ChangeLog
2014-09-05 Dominik Vogt <***@linux.vnet.ibm.com>

* godump.c (go_format_type): Represent "float _Complex" and
"double _Complex" as complex64 or complex128 in Go, as appropriate.

gcc/testsuite/ChangeLog
2014-09-05 Dominik Vogt <***@linux.vnet.ibm.com>

* gcc.misc-tests/godump-1.c: Add tests for complex types.

Ciao

Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
Dominik Vogt
2014-09-09 13:06:34 UTC
Permalink
Quote shell variables that may contain whitespace in the configure
file. I cannot remember exactly whether I actually ran into a
problem, but the patch won't hurt anyway.

ChangeLog
2014-09-05 Dominik Vogt <***@linux.vnet.ibm.com>

* libgo/configure.ac (GO_SYSCALL_OS_ARCH_FILE): Use double quotes around
path names.
(GO_SYSCALL_OS_FILE): Ditto.
(GO_LIBCALL_OS_ARCH_FILE): Ditto.
(GO_LIBCALL_OS_FILE): Ditto.

Ciao

Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
Loading...