On 12/14/20 10:48 PM, Nikhil Benesch wrote:
On 12/14/20 10:34 PM, Ian Lance Taylor wrote:
On Mon, Dec 14, 2020 at 7:14 PM Nikhil Benesch <nikhil.bene...@gmail.com> wrote:
Also godump now emits a dummy `type _u?pad128_t struct {}` entry,
so we just suppress that and conditionally add it back.
I don't understand this bit. Why are we seeing an empty struct
definition, and why is this change the right fix?
Is the problem that because we don't know how to emit the definition
of the struct, godump.c winds up emitting an empty definition? That
doesn't sound like the right thing to do for this case.
That's a good question. u?pad128_t is clearly getting marked as a
potential dummy type. But then you'd think it'd *also* get marked as
an invalid type, and then find_dummy_types would suppress it.
I misunderstood how find_dummy_types worked. Here is a correct
description.
find_dummy_types will output a dummy type for an entry in
pot_dummy_types if:
(1) we never observed a definition for that type name, or
(2) we observed an invalid definition for that type name.
I don't actually understand why condition (2) is considered. Since
types that refer to invalid types are themselves marked as invalid,
godump will recursively comment out all types that refer even
transitively to an invalid dummy type. So it doesn't seem necessary to
emit a dummy type in condition (2).
Ian, you added this behavior way back in 2012:
https://github.com/gcc-mirror/gcc/commit/3eb9e389a6460b6bd9400a3f4acf88fb2e258e07
Perhaps you remember why?
If consensus is this behavior is wrong, I've attached a patch
(invalid-dummy.patch) that removes condition (2) from find_dummy_types
along with some new test cases. I don't have enough context to say
whether this patch is the right solution though.
----
Separately, I'm actually not sure why we're extracting _u?pad128_t from
gen-sysinfo.go at all. Nothing in libgo refers to those types. And no
types in [runtime_]sysinfo.go could refer to them either, because if
they did they'd *also* be marked as invalid (right?). So I suspect we
can just nuke those extractions from mk[r]sysinfo.sh. I've attached a
patch that does so (solaris-godump.patch). I'm still waiting on my
compile to complete, but initial signs look promising.
If this patch looks good, I'll submit it upstream tomorrow.
The nice thing about this approach, if it works, is that it works both
with and without the change to find_dummy_types, so we can consider
that patch independently.
Cheers,
Nikhil
diff --git a/gcc/godump.c b/gcc/godump.c
index ff3a4a9c52c..78ff22d5f13 100644
--- a/gcc/godump.c
+++ b/gcc/godump.c
@@ -1351,11 +1351,9 @@ find_dummy_types (const char *const &ptr,
godump_container *adata)
class godump_container *data = (class godump_container *) adata;
const char *type = (const char *) ptr;
void **slot;
- void **islot;
slot = htab_find_slot (data->type_hash, type, NO_INSERT);
- islot = htab_find_slot (data->invalid_hash, type, NO_INSERT);
- if (slot == NULL || islot != NULL)
+ if (slot == NULL)
fprintf (go_dump_file, "type _%s struct {}\n", type);
return true;
}
diff --git a/gcc/testsuite/gcc.misc-tests/godump-1.c
b/gcc/testsuite/gcc.misc-tests/godump-1.c
index d37ab0b5af4..e22fb6b8a80 100644
--- a/gcc/testsuite/gcc.misc-tests/godump-1.c
+++ b/gcc/testsuite/gcc.misc-tests/godump-1.c
@@ -304,6 +304,11 @@ long double ld_v1;
ld_t ld_v2;
/* { dg-final { scan-file godump-1.out "(?n)^// var _ld_v2
INVALID-float-\[0-9\]*$" } } */
+typedef struct ld_s { long double ld_s_f; } ld_s_t;
+/* { dg-final { scan-file godump-1.out "(?n)^// type _ld_s struct \{ ld_s_f
INVALID-float-\[0-9\]*; \}$" } } */
+/* { dg-final { scan-file godump-1.out "(?n)^// type _ld_s_t _ld_s$" } } *
+/* { dg-final { scan-file-not godump-1.out "(?n)^type _ld_s struct \{\}$" } }
*/
+
/*** complex types ***/
typedef _Complex cx_t;
/* { dg-final { scan-file godump-1.out "(?n)^type _cx_t complex\[0-9\]*$" } }
*/
@@ -476,6 +481,8 @@ struct { double d; uint8_t : 0; } sd_not_equiv;
/* { dg-final { scan-file godump-1.out "(?n)^var _sd_not_equiv struct \{ d
float\[0-9\]*; \}$" } } */
typedef struct s_undef_t s_undef_t2;
+/* { dg-final { scan-file godump-1.out "(?n)^type _s_undef_t2 _s_undef_t$" } }
*/
+/* { dg-final { scan-file godump-1.out "(?n)^type _s_undef_t struct \{\}$" } }
*/
typedef struct s_fwd *s_fwd_p;
/* { dg-final { scan-file godump-1.out "(?n)^type _s_fwd_p \\*_s_fwd$" } } */
@@ -816,6 +823,8 @@ union { uint64_t : 1; uint8_t ca[5]; } u2_size;
/* { dg-final { scan-file godump-1.out "(?n)^var _u2_size struct \{ ca
\\\[4\\+1\\\]uint8; \}$" } } */
typedef union u_undef_t u_undef_t2;
+/* { dg-final { scan-file godump-1.out "(?n)^type _u_undef_t2 _u_undef_t$" } }
*/
+/* { dg-final { scan-file godump-1.out "(?n)^type _u_undef_t struct \{\}$" } }
*/
typedef union { uint64_t b : 1; uint8_t ca[5]; } tu3_size;
/* { dg-final { scan-file godump-1.out "(?n)^type _tu3_size struct \{ ca
\\\[4\\+1\\\]uint8; Godump_0_pad \\\[.\\\]byte; Godump_1_align
\\\[0\\\]u?int64; \}$" } } */
diff --git a/libgo/mkrsysinfo.sh b/libgo/mkrsysinfo.sh
index c28f0e5..1bd0956 100755
--- a/libgo/mkrsysinfo.sh
+++ b/libgo/mkrsysinfo.sh
@@ -24,11 +24,18 @@ grep -v '^// ' gen-sysinfo.go | \
grep -v '^type _timespec ' | \
grep -v '^type _epoll_' | \
grep -v '^type _*locale[_ ]' | \
- grep -v 'in6_addr' | \
+ grep -v '^type _in6_addr' | \
grep -v 'sockaddr_in6' | \
sed -e 's/\([^a-zA-Z0-9_]\)_timeval\([^a-zA-Z0-9_]\)/\1timeval\2/g' \
+ -e 's/\([^a-zA-Z0-9_]\)_timeval$/\1timeval/g' \
-e 's/\([^a-zA-Z0-9_]\)_timespec_t\([^a-zA-Z0-9_]\)/\1timespec\2/g' \
+ -e 's/\([^a-zA-Z0-9_]\)_timespec_t$/\1timespec_t/g' \
-e 's/\([^a-zA-Z0-9_]\)_timespec\([^a-zA-Z0-9_]\)/\1timespec\2/g' \
+ -e 's/\([^a-zA-Z0-9_]\)_timespec$/\1timespec/g' \
+ -e 's/\([^a-zA-Z0-9_]\)_in6_addr\([^a-zA-Z0-9_]\)/\1[16]byte\2/g' \
+ -e 's/\([^a-zA-Z0-9_]\)_in6_addr$/\1[16]byte/g' \
+ -e 's/\([^a-zA-Z0-9_]\)_in6_addr_t\([^a-zA-Z0-9_]\)/\1[16]byte\2/g' \
+ -e 's/\([^a-zA-Z0-9_]\)_in6_addr_t$/\1[16]byte/g' \
>> ${OUT}
# The C long type, needed because that is the type that ptrace returns.
@@ -44,16 +51,6 @@ else
exit 1
fi
-# On AIX, the _arpcom struct, is filtered by the above grep sequence, as it as
-# a field of type _in6_addr, but other types depend on _arpcom, so we need to
-# put it back.
-grep '^type _arpcom ' gen-sysinfo.go | \
- sed -e 's/_in6_addr/[16]byte/' >> ${OUT}
-
-# Same on Solaris for _mld_hdr_t.
-grep '^type _mld_hdr_t ' gen-sysinfo.go | \
- sed -e 's/_in6_addr/[16]byte/' >> ${OUT}
-
# The time structures need special handling: we need to name the
# types, so that we can cast integers to the right types when
# assigning to the structures.
@@ -165,40 +162,6 @@ fi
grep '^type _sem_t ' gen-sysinfo.go | \
sed -e 's/_sem_t/semt/' >> ${OUT}
-# Solaris 2 needs _u?pad128_t, but its default definition in terms of long
-# double is commented by -fdump-go-spec.
-if grep "^// type _pad128_t" gen-sysinfo.go > /dev/null 2>&1; then
- echo "type _pad128_t struct { _l [4]int32; }" >> ${OUT}
-fi
-if grep "^// type _upad128_t" gen-sysinfo.go > /dev/null 2>&1; then
- echo "type _upad128_t struct { _l [4]uint32; }" >> ${OUT}
-fi
-
-# The Solaris 11 Update 1 _zone_net_addr_t struct.
-grep '^type _zone_net_addr_t ' gen-sysinfo.go | \
- sed -e 's/_in6_addr/[16]byte/' \
- >> ${OUT}
-
-# The Solaris 11.4 _flow_arp_desc_t struct.
-grep '^type _flow_arp_desc_t ' gen-sysinfo.go | \
- sed -e 's/_in6_addr_t/[16]byte/g' \
- >> ${OUT}
-
-# The Solaris 11.4 _flow_l3_desc_t struct.
-grep '^type _flow_l3_desc_t ' gen-sysinfo.go | \
- sed -e 's/_in6_addr_t/[16]byte/g' \
- >> ${OUT}
-
-# The Solaris 11.3 _mac_ipaddr_t struct.
-grep '^type _mac_ipaddr_t ' gen-sysinfo.go | \
- sed -e 's/_in6_addr_t/[16]byte/g' \
- >> ${OUT}
-
-# The Solaris 11.3 _mactun_info_t struct.
-grep '^type _mactun_info_t ' gen-sysinfo.go | \
- sed -e 's/_in6_addr_t/[16]byte/g' \
- >> ${OUT}
-
# The Solaris port_event_t struct.
grep '^type _port_event_t ' gen-sysinfo.go | \
sed -e s'/_port_event_t/portevent/' \
diff --git a/libgo/mksysinfo.sh b/libgo/mksysinfo.sh
index b32a026..e214d63 100755
--- a/libgo/mksysinfo.sh
+++ b/libgo/mksysinfo.sh
@@ -36,24 +36,22 @@ grep -v '^// ' gen-sysinfo.go | \
grep -v '^type _timestruc_t ' | \
grep -v '^type _epoll_' | \
grep -v '^type _*locale[_ ]' | \
- grep -v 'in6_addr' | \
+ grep -v '^type _in6_addr' | \
grep -v 'sockaddr_in6' | \
sed -e 's/\([^a-zA-Z0-9_]\)_timeval\([^a-zA-Z0-9_]\)/\1Timeval\2/g' \
+ -e 's/\([^a-zA-Z0-9_]\)_timeval$/\1Timeval/g' \
-e 's/\([^a-zA-Z0-9_]\)_timespec_t\([^a-zA-Z0-9_]\)/\1Timespec\2/g' \
+ -e 's/\([^a-zA-Z0-9_]\)_timespec_t$/\1Timespec/g' \
-e 's/\([^a-zA-Z0-9_]\)_timespec\([^a-zA-Z0-9_]\)/\1Timespec\2/g' \
+ -e 's/\([^a-zA-Z0-9_]\)_timespec$/\1Timespec/g' \
-e 's/\([^a-zA-Z0-9_]\)_timestruc_t\([^a-zA-Z0-9_]\)/\1Timestruc\2/g' \
+ -e 's/\([^a-zA-Z0-9_]\)_timestruc_t$/\1Timestruc/g' \
+ -e 's/\([^a-zA-Z0-9_]\)_in6_addr\([^a-zA-Z0-9_]\)/\1[16]byte\2/g' \
+ -e 's/\([^a-zA-Z0-9_]\)_in6_addr$/\1[16]byte/g' \
+ -e 's/\([^a-zA-Z0-9_]\)_in6_addr_t\([^a-zA-Z0-9_]\)/\1[16]byte\2/g' \
+ -e 's/\([^a-zA-Z0-9_]\)_in6_addr_t$/\1[16]byte/g' \
>> ${OUT}
-# On AIX, the _arpcom struct, is filtered by the above grep sequence, as it as
-# a field of type _in6_addr, but other types depend on _arpcom, so we need to
-# put it back.
-grep '^type _arpcom ' gen-sysinfo.go | \
- sed -e 's/_in6_addr/[16]byte/' >> ${OUT}
-
-# Same on Solaris for _mld_hdr_t.
-grep '^type _mld_hdr_t ' gen-sysinfo.go | \
- sed -e 's/_in6_addr/[16]byte/' >> ${OUT}
-
# The errno constants. These get type Errno.
egrep '#define E[A-Z0-9_]+ [0-9E]' errno.i | \
sed -e 's/^#define \(E[A-Z0-9_]*\) .*$/const \1 = Errno(_\1)/' >> ${OUT}
@@ -441,15 +439,6 @@ else
exit 1
fi
-# Solaris 2 needs _u?pad128_t, but its default definition in terms of long
-# double is commented by -fdump-go-spec.
-if grep "^// type _pad128_t" gen-sysinfo.go > /dev/null 2>&1; then
- echo "type _pad128_t struct { _l [4]int32; }" >> ${OUT}
-fi
-if grep "^// type _upad128_t" gen-sysinfo.go > /dev/null 2>&1; then
- echo "type _upad128_t struct { _l [4]uint32; }" >> ${OUT}
-fi
-
# The time_t type.
if grep '^type _time_t ' gen-sysinfo.go > /dev/null 2>&1; then
echo 'type Time_t _time_t' >> ${OUT}
@@ -483,7 +472,9 @@ echo $timespec | \
-e 's/tv_nsec *[a-zA-Z0-9_]*/Nsec Timespec_nsec_t/' >> ${OUT}
timestruc=`grep '^type _timestruc_t ' gen-sysinfo.go || true`
-if test "$timestruc" != ""; then
+if test "$timestruc" = "type _timestruc_t _timespec"; then
+ echo "type Timestruc Timespec" >> ${OUT}
+elif test "$timestruc" != ""; then
timestruc_sec=`echo $timestruc | sed -n -e 's/^.*tv_sec \([^ ]*\);.*$/\1/p'`
timestruc_nsec=`echo $timestruc | sed -n -e 's/^.*tv_nsec \([^
]*\);.*$/\1/p'`
echo "type Timestruc_sec_t = $timestruc_sec" >> ${OUT}
@@ -1523,31 +1514,6 @@ if ! grep 'const SizeofICMPv6Filter ' ${OUT} >/dev/null
2>&1; then
echo 'const SizeofICMPv6Filter = 32' >> ${OUT}
fi
-# The Solaris 11 Update 1 _zone_net_addr_t struct.
-grep '^type _zone_net_addr_t ' gen-sysinfo.go | \
- sed -e 's/_in6_addr/[16]byte/' \
- >> ${OUT}
-
-# The Solaris 11.4 _flow_arp_desc_t struct.
-grep '^type _flow_arp_desc_t ' gen-sysinfo.go | \
- sed -e 's/_in6_addr_t/[16]byte/g' \
- >> ${OUT}
-
-# The Solaris 11.4 _flow_l3_desc_t struct.
-grep '^type _flow_l3_desc_t ' gen-sysinfo.go | \
- sed -e 's/_in6_addr_t/[16]byte/g' \
- >> ${OUT}
-
-# The Solaris 11.3 _mac_ipaddr_t struct.
-grep '^type _mac_ipaddr_t ' gen-sysinfo.go | \
- sed -e 's/_in6_addr_t/[16]byte/g' \
- >> ${OUT}
-
-# The Solaris 11.3 _mactun_info_t struct.
-grep '^type _mactun_info_t ' gen-sysinfo.go | \
- sed -e 's/_in6_addr_t/[16]byte/g' \
- >> ${OUT}
-
# Type 'uint128' is needed in a couple of type definitions on arm64,such
# as _user_fpsimd_struct, _elf_fpregset_t, etc.
if ! grep '^type uint128' ${OUT} > /dev/null 2>&1; then
diff --git a/libgo/sysinfo.c b/libgo/sysinfo.c
index a060ea8..8ce061e 100644
--- a/libgo/sysinfo.c
+++ b/libgo/sysinfo.c
@@ -11,6 +11,7 @@
#include <stddef.h>
#include <stdlib.h>
+#include <stdio.h>
#include <sys/types.h>
#include <dirent.h>
#include <errno.h>
