Re: [PATCH] Correct -fdump-go-spec's handling of incomplete types

[Ian Lance Taylor <iant@google.com>]

[-- Attachment #1: Type: text/plain, Size: 1313 bytes --]

On Tue, Dec 8, 2020 at 2:57 PM Nikhil Benesch <nikhil.benesch@gmail.com> wrote:
>
> This patch corrects -fdump-go-spec's handling of incomplete types.
> To my knowledge the issue fixed here has not been previously
> reported. It was exposed by an in-progress port of gccgo to FreeBSD.
>
> Given the following C code
>
>         struct s_fwd v_fwd;
>         struct s_fwd { };
>
> -fdump-go-spec currently produces the following Go code
>
>         var v_fwd struct {};
>         type s_fwd s_fwd;
>
> whereas the correct Go code is:
>
>         var v_fwd s_fwd;
>         type s_fwd struct {};
>
> (Go is considerably more permissive than C with out-of-order
> declarations, so anywhere an out-of-order declaration is valid in
> C it is valid in Go.)
>
> gcc/:
>         * godump.c (go_format_type): Don't consider whether a type has
>         been seen when determining whether to output a type by name.
>         Consider only the use_type_name parameter.
>         (go_output_typedef): When outputting a typedef, format the
>         declaration's original type, which contains the name of the
>         underlying type rather than the name of the typedef.
> gcc/testsuite:
>         * gcc.misc-tests/godump-1.c: Add test case.

Thanks.  I changed function types to use type names, and committed like so.

Ian

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 4228 bytes --]

73cf5da233b4cd0f140dd997270e88de63e27db7
diff --git a/gcc/godump.c b/gcc/godump.c
index 29a45ce8979..033b2c59f3c 100644
--- a/gcc/godump.c
+++ b/gcc/godump.c
@@ -697,9 +697,8 @@ go_format_type (class godump_container *container, tree type,
   ret = true;
   ob = &container->type_obstack;
 
-  if (TYPE_NAME (type) != NULL_TREE
-      && (container->decls_seen.contains (type)
-	  || container->decls_seen.contains (TYPE_NAME (type)))
+  if (use_type_name
+      && TYPE_NAME (type) != NULL_TREE
       && (AGGREGATE_TYPE_P (type)
 	  || POINTER_TYPE_P (type)
 	  || TREE_CODE (type) == FUNCTION_TYPE))
@@ -707,6 +706,12 @@ go_format_type (class godump_container *container, tree type,
       tree name;
       void **slot;
 
+      /* References to complex builtin types cannot be translated to
+	Go.  */
+      if (DECL_P (TYPE_NAME (type))
+	  && DECL_IS_UNDECLARED_BUILTIN (TYPE_NAME (type)))
+	ret = false;
+
       name = TYPE_IDENTIFIER (type);
 
       slot = htab_find_slot (container->invalid_hash, IDENTIFIER_POINTER (name),
@@ -714,13 +719,17 @@ go_format_type (class godump_container *container, tree type,
       if (slot != NULL)
 	ret = false;
 
+      /* References to incomplete structs are permitted in many
+	 contexts, like behind a pointer or inside of a typedef. So
+	 consider any referenced struct a potential dummy type.  */
+      if (RECORD_OR_UNION_TYPE_P (type))
+       container->pot_dummy_types.add (IDENTIFIER_POINTER (name));
+
       obstack_1grow (ob, '_');
       go_append_string (ob, name);
       return ret;
     }
 
-  container->decls_seen.add (type);
-
   switch (TREE_CODE (type))
     {
     case TYPE_DECL:
@@ -821,34 +830,6 @@ go_format_type (class godump_container *container, tree type,
       break;
 
     case POINTER_TYPE:
-      if (use_type_name
-          && TYPE_NAME (TREE_TYPE (type)) != NULL_TREE
-          && (RECORD_OR_UNION_TYPE_P (TREE_TYPE (type))
-	      || (POINTER_TYPE_P (TREE_TYPE (type))
-                  && (TREE_CODE (TREE_TYPE (TREE_TYPE (type)))
-		      == FUNCTION_TYPE))))
-        {
-	  tree name;
-	  void **slot;
-
-	  name = TYPE_IDENTIFIER (TREE_TYPE (type));
-
-	  slot = htab_find_slot (container->invalid_hash,
-				 IDENTIFIER_POINTER (name), NO_INSERT);
-	  if (slot != NULL)
-	    ret = false;
-
-	  obstack_grow (ob, "*_", 2);
-	  go_append_string (ob, name);
-
-	  /* The pointer here can be used without the struct or union
-	     definition.  So this struct or union is a potential dummy
-	     type.  */
-	  if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (type)))
-	    container->pot_dummy_types.add (IDENTIFIER_POINTER (name));
-
-	  return ret;
-        }
       if (TREE_CODE (TREE_TYPE (type)) == FUNCTION_TYPE)
 	obstack_grow (ob, "func", 4);
       else
@@ -1107,7 +1088,7 @@ go_output_type (class godump_container *container)
 static void
 go_output_fndecl (class godump_container *container, tree decl)
 {
-  if (!go_format_type (container, TREE_TYPE (decl), false, true, NULL, false))
+  if (!go_format_type (container, TREE_TYPE (decl), true, true, NULL, false))
     fprintf (go_dump_file, "// ");
   fprintf (go_dump_file, "func _%s ",
 	   IDENTIFIER_POINTER (DECL_NAME (decl)));
@@ -1182,8 +1163,8 @@ go_output_typedef (class godump_container *container, tree decl)
 	return;
       *slot = CONST_CAST (void *, (const void *) type);
 
-      if (!go_format_type (container, TREE_TYPE (decl), true, false, NULL,
-			   false))
+      if (!go_format_type (container, DECL_ORIGINAL_TYPE (decl), true, false,
+			   NULL, false))
 	{
 	  fprintf (go_dump_file, "// ");
 	  slot = htab_find_slot (container->invalid_hash, type, INSERT);
diff --git a/gcc/testsuite/gcc.misc-tests/godump-1.c b/gcc/testsuite/gcc.misc-tests/godump-1.c
index f97bbecc9bc..96c25863374 100644
--- a/gcc/testsuite/gcc.misc-tests/godump-1.c
+++ b/gcc/testsuite/gcc.misc-tests/godump-1.c
@@ -471,6 +471,9 @@ typedef struct s_undef_t s_undef_t2;
 typedef struct s_fwd *s_fwd_p;
 /* { dg-final { scan-file godump-1.out "(?n)^type _s_fwd_p \\*_s_fwd$" } } */
 
+struct s_fwd v_fwd;
+/* { dg-final { scan-file godump-1.out "(?n)^var _v_fwd _s_fwd" } } */
+
 struct s_fwd { };
 /* { dg-final { scan-file godump-1.out "(?n)^type _s_fwd struct \{ \}$" } } */