public inbox for gdb-testers@sourceware.org
help / color / mirror / Atom feed
From: gdb-buildbot@sergiodj.net
To: gdb-testers@sourceware.org
Subject: [binutils-gdb] libctf: teach ctf_add_type how forwards work
Date: Tue, 08 Oct 2019 00:06:00 -0000	[thread overview]
Message-ID: <5de9eada3b4e39c89431765b1c59159cb04878ed@gdb-build> (raw)

*** TEST RESULTS FOR COMMIT 5de9eada3b4e39c89431765b1c59159cb04878ed ***

commit 5de9eada3b4e39c89431765b1c59159cb04878ed
Author:     Nick Alcock <nick.alcock@oracle.com>
AuthorDate: Sat Aug 3 00:46:01 2019 +0100
Commit:     Nick Alcock <nick.alcock@oracle.com>
CommitDate: Thu Oct 3 17:04:55 2019 +0100

    libctf: teach ctf_add_type how forwards work
    
    This machinery has been broken for as long as Solaris has existed.
    Forwards are meant to encode "struct foo;", "enum foo;" or "union
    foo;".  Obviously these all exist in distinct namespaces, so forwards
    store the type kind they forward to in their ctt_type member
    (which makes conceptual sense if you squint at it).  The addition
    machinery uses this to promote forwards to the appropriate type as
    needed.
    
    Unfortunately ctf_add_type does not: it checks the global namespace
    (which is always wrong), and so fails with a spurious conflict if you
    have, say, a typedef and then a forward comes along with the same name,
    even if it's a forward to something like a struct.  (This was observed
    with <libio.h>, which has "struct _IO_FILE;" and also
    "typedef struct _IO_FILE _IO_FILE").  We should look at the recorded
    type kind and look in the appropriate namespace.   We should also,
    when creating the forward in the new container, use that type kind,
    rather than just defaulting to CTF_K_STRUCT and hoping that what
    eventually comes along is a struct.
    
    This bug is as old as the first implementation of ctf_add_type in
    Solaris.  But we also want a new feature for the linker, closely-related
    and touching the same code so we add it here: not only do we want a
    forward followed by a struct/union/enum to promote the forward, but
    we want want a struct/union/enum followed by a forward to act as a NOP
    and return the existing type, because when we're adding many files
    in succession to a target link, there will often be already-promoted
    forwards (in the shape of a struct/union/enum) that want to unify
    with duplicate forwards coming from other object files.
    
    v5: fix tabdamage.
    
    libctf/
            * ctf-create.c (ctf_add_type): Look up and use the forwarded-to
            type kind.  Allow forwards to unify with pre-existing structs/
            unions/enums.

diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index b2726488cb..28be757966 100644
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,3 +1,9 @@
+2019-08-03  Nick Alcock  <nick.alcock@oracle.com>
+
+	* ctf-create.c (ctf_add_type): Look up and use the forwarded-to
+	type kind.  Allow forwards to unify with pre-existing structs/
+	unions/enums.
+
 2019-07-30  Nick Alcock  <nick.alcock@oracle.com>
 
 	* ctf-impl.h (ctf_file_t) <ctf_link_cu_mappping>: New.
diff --git a/libctf/ctf-create.c b/libctf/ctf-create.c
index 19da29c5db..8eb16738a1 100644
--- a/libctf/ctf-create.c
+++ b/libctf/ctf-create.c
@@ -1552,7 +1552,7 @@ ctf_add_type (ctf_file_t *dst_fp, ctf_file_t *src_fp, ctf_id_t src_type)
   ctf_id_t tmp;
 
   const char *name;
-  uint32_t kind, flag, vlen;
+  uint32_t kind, forward_kind, flag, vlen;
 
   const ctf_type_t *src_tp, *dst_tp;
   ctf_bundle_t src, dst;
@@ -1576,7 +1576,11 @@ ctf_add_type (ctf_file_t *dst_fp, ctf_file_t *src_fp, ctf_id_t src_type)
   flag = LCTF_INFO_ISROOT (src_fp, src_tp->ctt_info);
   vlen = LCTF_INFO_VLEN (src_fp, src_tp->ctt_info);
 
-  switch (kind)
+  forward_kind = kind;
+  if (kind == CTF_K_FORWARD)
+    forward_kind = src_tp->ctt_type;
+
+  switch (forward_kind)
     {
     case CTF_K_STRUCT:
       hp = dst_fp->ctf_structs;
@@ -1605,16 +1609,30 @@ ctf_add_type (ctf_file_t *dst_fp, ctf_file_t *src_fp, ctf_id_t src_type)
 
   /* If an identically named dst_type exists, fail with ECTF_CONFLICT
      unless dst_type is a forward declaration and src_type is a struct,
-     union, or enum (i.e. the definition of the previous forward decl).  */
+     union, or enum (i.e. the definition of the previous forward decl).
 
-  if (dst_type != CTF_ERR && dst_kind != kind
-      && (dst_kind != CTF_K_FORWARD
-	  || (kind != CTF_K_ENUM && kind != CTF_K_STRUCT
-	      && kind != CTF_K_UNION)))
+     We also allow addition in the opposite order (addition of a forward when a
+     struct, union, or enum already exists), which is a NOP and returns the
+     already-present struct, union, or enum.  */
+
+  if (dst_type != CTF_ERR && dst_kind != kind)
     {
-      ctf_dprintf ("Conflict for type %s: kinds differ, new: %i; "
-		   "old (ID %lx): %i\n", name, kind, dst_type, dst_kind);
-      return (ctf_set_errno (dst_fp, ECTF_CONFLICT));
+      if (kind == CTF_K_FORWARD
+	  && (dst_kind == CTF_K_ENUM || dst_kind == CTF_K_STRUCT
+	      || dst_kind == CTF_K_UNION))
+	{
+	  ctf_add_type_mapping (src_fp, src_type, dst_fp, dst_type);
+	  return dst_type;
+	}
+
+      if (dst_kind != CTF_K_FORWARD
+	  || (kind != CTF_K_ENUM && kind != CTF_K_STRUCT
+	      && kind != CTF_K_UNION))
+	{
+	  ctf_dprintf ("Conflict for type %s: kinds differ, new: %i; "
+		       "old (ID %lx): %i\n", name, kind, dst_type, dst_kind);
+	  return (ctf_set_errno (dst_fp, ECTF_CONFLICT));
+	}
     }
 
   /* We take special action for an integer, float, or slice since it is
@@ -1924,10 +1942,7 @@ ctf_add_type (ctf_file_t *dst_fp, ctf_file_t *src_fp, ctf_id_t src_type)
 
     case CTF_K_FORWARD:
       if (dst_type == CTF_ERR)
-	{
-	  dst_type = ctf_add_forward (dst_fp, flag,
-				      name, CTF_K_STRUCT); /* Assume STRUCT. */
-	}
+	  dst_type = ctf_add_forward (dst_fp, flag, name, forward_kind);
       break;
 
     case CTF_K_TYPEDEF:


             reply	other threads:[~2019-10-08  0:06 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-08  0:06 gdb-buildbot [this message]
2019-10-08  0:06 ` Failures on Ubuntu-Aarch64-m64, branch master gdb-buildbot
2019-10-08  1:36 ` Failures on Ubuntu-Aarch64-native-extended-gdbserver-m64, " gdb-buildbot
2019-10-08  2:25 ` Failures on Ubuntu-Aarch64-native-gdbserver-m64, " gdb-buildbot
2019-10-11  2:18 ` Failures on Fedora-i686, " gdb-buildbot
2019-10-11  2:43 ` Failures on Fedora-x86_64-m32, " gdb-buildbot
2019-10-11  4:01 ` Failures on Fedora-x86_64-native-extended-gdbserver-m64, " gdb-buildbot
2019-10-11  4:22 ` Failures on Fedora-x86_64-native-gdbserver-m32, " gdb-buildbot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5de9eada3b4e39c89431765b1c59159cb04878ed@gdb-build \
    --to=gdb-buildbot@sergiodj.net \
    --cc=gdb-testers@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).