public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* 2.36 branch created.
@ 2021-01-09 11:39 Nick Clifton
  2021-01-09 14:09 ` [PATCH] ld/x86-64: Properly Handle -z lam-u48/lam-u57 (2.36 branch created.) H.J. Lu
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Nick Clifton @ 2021-01-09 11:39 UTC (permalink / raw)
  To: binutils

Hi Everyone, 

  The 2.36 branch has now been created:

     git clone git://sourceware.org/git/binutils-gdb.git -b binutils-2.36-branch 2.36

  A snapshot of the sources is also available here:

    https://sourceware.org/pub/binutils/snapshots/binutils-2.35.90.tar.xz

  Please could all patches for the branch be run by me.
  The rules for the branch are:

    * No new features.
    * Target specific bug fixes are OK.
    * Generic bug fixes are OK if they are important and widely tested.
    * Documentation updates/fixes are OK.
    * Translation updates are OK.
    * Fixes for testsuite failures are OK.

  Ideally I would like to make the release happen in two weeks time,
  i.e. 24 Jan 2021.  Which I hope will be enough time for everyone
  to get their final fixes in.

Cheers
  Nick


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ld/x86-64: Properly Handle -z lam-u48/lam-u57 (2.36 branch created.)
  2021-01-09 11:39 2.36 branch created Nick Clifton
@ 2021-01-09 14:09 ` H.J. Lu
  2021-01-12 13:23 ` 2.36 branch created H.J. Lu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: H.J. Lu @ 2021-01-09 14:09 UTC (permalink / raw)
  To: Nick Clifton; +Cc: Binutils

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

On Sat, Jan 9, 2021 at 3:40 AM Nick Clifton via Binutils
<binutils@sourceware.org> wrote:
>
> Hi Everyone,
>
>   The 2.36 branch has now been created:
>
>      git clone git://sourceware.org/git/binutils-gdb.git -b binutils-2.36-branch 2.36
>
>   A snapshot of the sources is also available here:
>
>     https://sourceware.org/pub/binutils/snapshots/binutils-2.35.90.tar.xz
>
>   Please could all patches for the branch be run by me.
>   The rules for the branch are:
>
>     * No new features.
>     * Target specific bug fixes are OK.
>     * Generic bug fixes are OK if they are important and widely tested.
>     * Documentation updates/fixes are OK.
>     * Translation updates are OK.
>     * Fixes for testsuite failures are OK.
>
>   Ideally I would like to make the release happen in two weeks time,
>   i.e. 24 Jan 2021.  Which I hope will be enough time for everyone
>   to get their final fixes in.

Hi Nick,

I am checking this patch into master and backporting it to 2.36
branch.

-- 
H.J.

[-- Attachment #2: 0001-ld-x86-64-Properly-Handle-z-lam-u48-lam-u57.patch --]
[-- Type: text/x-patch, Size: 3814 bytes --]

From fd4535ea6771e7c76c3f40cf980614303e9891aa Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 9 Jan 2021 06:02:19 -0800
Subject: [PATCH] ld/x86-64: Properly Handle -z lam-u48/lam-u57

Properly merge GNU properties for LAM_U48 and LAM_U57.

bfd/

	PR ld/27166
	* elfxx-x86.c (_bfd_x86_elf_merge_gnu_properties): Handle
	-z lam-u48 and -z lam-u57.

ld/

	PR ld/27166
	* testsuite/ld-x86-64/lam-u48.rd: New file.
	* testsuite/ld-x86-64/lam-u57.rd: Likewise.
	* testsuite/ld-x86-64/x86-64.exp: Add PR ld/27166 tests.
---
 bfd/elfxx-x86.c                   | 16 ++++++++++++++--
 ld/testsuite/ld-x86-64/lam-u48.rd |  6 ++++++
 ld/testsuite/ld-x86-64/lam-u57.rd |  6 ++++++
 ld/testsuite/ld-x86-64/x86-64.exp | 16 ++++++++++++++++
 4 files changed, 42 insertions(+), 2 deletions(-)
 create mode 100644 ld/testsuite/ld-x86-64/lam-u48.rd
 create mode 100644 ld/testsuite/ld-x86-64/lam-u57.rd

diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
index c47f48257ea..3a0dffc4e6e 100644
--- a/bfd/elfxx-x86.c
+++ b/bfd/elfxx-x86.c
@@ -2449,8 +2449,15 @@ _bfd_x86_elf_merge_gnu_properties (struct bfd_link_info *info,
 		features = GNU_PROPERTY_X86_FEATURE_1_IBT;
 	      if (htab->params->shstk)
 		features |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
-	      /* Add GNU_PROPERTY_X86_FEATURE_1_IBT and
-		 GNU_PROPERTY_X86_FEATURE_1_SHSTK.  */
+	      if (htab->params->lam_u48)
+		features |= (GNU_PROPERTY_X86_FEATURE_1_LAM_U48
+			     | GNU_PROPERTY_X86_FEATURE_1_LAM_U57);
+	      else if (htab->params->lam_u57)
+		features |= GNU_PROPERTY_X86_FEATURE_1_LAM_U57;
+	      /* Add GNU_PROPERTY_X86_FEATURE_1_IBT,
+		 GNU_PROPERTY_X86_FEATURE_1_SHSTK,
+		 GNU_PROPERTY_X86_FEATURE_1_LAM_U48 and
+		 GNU_PROPERTY_X86_FEATURE_1_LAM_U57.  */
 	      aprop->u.number |= features;
 	    }
 	  updated = number != (unsigned int) aprop->u.number;
@@ -2470,6 +2477,11 @@ _bfd_x86_elf_merge_gnu_properties (struct bfd_link_info *info,
 		features = GNU_PROPERTY_X86_FEATURE_1_IBT;
 	      if (htab->params->shstk)
 		features |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
+	      if (htab->params->lam_u48)
+		features |= (GNU_PROPERTY_X86_FEATURE_1_LAM_U48
+			     | GNU_PROPERTY_X86_FEATURE_1_LAM_U57);
+	      else if (htab->params->lam_u57)
+		features |= GNU_PROPERTY_X86_FEATURE_1_LAM_U57;
 	    }
 	  if (features)
 	    {
diff --git a/ld/testsuite/ld-x86-64/lam-u48.rd b/ld/testsuite/ld-x86-64/lam-u48.rd
new file mode 100644
index 00000000000..ad312627059
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/lam-u48.rd
@@ -0,0 +1,6 @@
+Displaying notes found in: .note.gnu.property
+[ 	]+Owner[ 	]+Data size[ 	]+Description
+  GNU                  0x[0-9a-f]+	NT_GNU_PROPERTY_TYPE_0
+#...
+      Properties: x86 feature: .*LAM_U48, LAM_U57.*
+#pass
diff --git a/ld/testsuite/ld-x86-64/lam-u57.rd b/ld/testsuite/ld-x86-64/lam-u57.rd
new file mode 100644
index 00000000000..8b77e6311c0
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/lam-u57.rd
@@ -0,0 +1,6 @@
+Displaying notes found in: .note.gnu.property
+[ 	]+Owner[ 	]+Data size[ 	]+Description
+  GNU                  0x[0-9a-f]+	NT_GNU_PROPERTY_TYPE_0
+#...
+      Properties: x86 feature: .*LAM_U57.*
+#pass
diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
index 37a2e50341d..83fdaa7db65 100644
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -1452,6 +1452,22 @@ if { [isnative] && [check_compiler_available] } {
 		{{error_output "pr21997-1b.err"}} \
 		"pr21997-1b" \
 	    ] \
+	    [list \
+		"Build lam-u48.so" \
+		"-shared -Wl,-z,lam-u48" \
+		"" \
+		{dummy.s} \
+		{{readelf -n lam-u48.rd}}  \
+		"lam-u48.so" \
+	    ] \
+	    [list \
+		"Build lam-u57.so" \
+		"-shared -Wl,-z,lam-u57" \
+		"" \
+		{dummy.s} \
+		{{readelf -n lam-u57.rd}}  \
+		"lam-u57.so" \
+	    ] \
 	]
     }
 
-- 
2.29.2


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: 2.36 branch created.
  2021-01-09 11:39 2.36 branch created Nick Clifton
  2021-01-09 14:09 ` [PATCH] ld/x86-64: Properly Handle -z lam-u48/lam-u57 (2.36 branch created.) H.J. Lu
@ 2021-01-12 13:23 ` H.J. Lu
  2021-01-12 13:27   ` Nick Clifton
  2021-01-12 19:11 ` Nick Alcock
  2021-01-18 12:53 ` [PATCH 1/3 trunk] libctf, ld: fix data symbol test with newer GCC Nick Alcock
  3 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2021-01-12 13:23 UTC (permalink / raw)
  To: Nick Clifton; +Cc: Binutils

On Sat, Jan 9, 2021 at 3:40 AM Nick Clifton via Binutils
<binutils@sourceware.org> wrote:
>
> Hi Everyone,
>
>   The 2.36 branch has now been created:
>
>      git clone git://sourceware.org/git/binutils-gdb.git -b binutils-2.36-branch 2.36
>
>   A snapshot of the sources is also available here:
>
>     https://sourceware.org/pub/binutils/snapshots/binutils-2.35.90.tar.xz
>
>   Please could all patches for the branch be run by me.
>   The rules for the branch are:
>
>     * No new features.
>     * Target specific bug fixes are OK.
>     * Generic bug fixes are OK if they are important and widely tested.
>     * Documentation updates/fixes are OK.
>     * Translation updates are OK.
>     * Fixes for testsuite failures are OK.
>
>   Ideally I would like to make the release happen in two weeks time,
>   i.e. 24 Jan 2021.  Which I hope will be enough time for everyone
>   to get their final fixes in.
>

Hi Nick,

Is it OK to backport the fix for

https://sourceware.org/bugzilla/show_bug.cgi?id=27171

to 2.36 branch?

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: 2.36 branch created.
  2021-01-12 13:23 ` 2.36 branch created H.J. Lu
@ 2021-01-12 13:27   ` Nick Clifton
  0 siblings, 0 replies; 12+ messages in thread
From: Nick Clifton @ 2021-01-12 13:27 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

Hi H.J.

> Is it OK to backport the fix for
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=27171
> 
> to 2.36 branch?

Yes - please do.

Cheers
   Nick



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: 2.36 branch created.
  2021-01-09 11:39 2.36 branch created Nick Clifton
  2021-01-09 14:09 ` [PATCH] ld/x86-64: Properly Handle -z lam-u48/lam-u57 (2.36 branch created.) H.J. Lu
  2021-01-12 13:23 ` 2.36 branch created H.J. Lu
@ 2021-01-12 19:11 ` Nick Alcock
  2021-01-13 10:02   ` Nick Clifton
  2021-01-18 12:53 ` [PATCH 1/3 trunk] libctf, ld: fix data symbol test with newer GCC Nick Alcock
  3 siblings, 1 reply; 12+ messages in thread
From: Nick Alcock @ 2021-01-12 19:11 UTC (permalink / raw)
  To: binutils

On 9 Jan 2021, Nick Clifton via Binutils spake thusly:

>   The 2.36 branch has now been created:
>
>      git clone git://sourceware.org/git/binutils-gdb.git -b binutils-2.36-branch 2.36
>
>   A snapshot of the sources is also available here:
>
>     https://sourceware.org/pub/binutils/snapshots/binutils-2.35.90.tar.xz
>
>   Please could all patches for the branch be run by me.
>   The rules for the branch are:
>
>     * No new features.
>     * Target specific bug fixes are OK.
>     * Generic bug fixes are OK if they are important and widely tested.
>     * Documentation updates/fixes are OK.
>     * Translation updates are OK.
>     * Fixes for testsuite failures are OK.

Presumably the actual mechanics of getting things into the branch are
the same as usual? I just spotted a regression introduced by the stuff I
did before Christmas... :( an in-hindsight obvious problematic case in
the pptrtab lookup stuff. (Fix, with testcase, already under test here.)

>                      Which I hope will be enough time for everyone
>   to get their final fixes in.

Two whole weeks, oh good, enough time for me to do a full test cycle
rather than skimping then.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: 2.36 branch created.
  2021-01-12 19:11 ` Nick Alcock
@ 2021-01-13 10:02   ` Nick Clifton
  2021-01-13 16:24     ` Nick Alcock
  0 siblings, 1 reply; 12+ messages in thread
From: Nick Clifton @ 2021-01-13 10:02 UTC (permalink / raw)
  To: Nick Alcock, binutils

Hi Nick,

>>    Please could all patches for the branch be run by me.
>>    The rules for the branch are:

> Presumably the actual mechanics of getting things into the branch are
> the same as usual?

They are, although of course you do also have to stand on your head and
whistle Rule Britannia whilst committing the changes...

Cheers
   Nick

(OK that last part was not true).


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: 2.36 branch created.
  2021-01-13 10:02   ` Nick Clifton
@ 2021-01-13 16:24     ` Nick Alcock
  0 siblings, 0 replies; 12+ messages in thread
From: Nick Alcock @ 2021-01-13 16:24 UTC (permalink / raw)
  To: binutils

On 13 Jan 2021, Nick Clifton via Binutils outgrape:

> Hi Nick,
>
>>>    Please could all patches for the branch be run by me.
>>>    The rules for the branch are:
>
>> Presumably the actual mechanics of getting things into the branch are
>> the same as usual?
>
> They are,

Great! Patch series coming soon, I'll Cc you (and make sure it applies
to both branches, which I'm sure it will).

>           although of course you do also have to stand on your head and
> whistle Rule Britannia whilst committing the changes...

Oh I do that every day, as the lockdown laws require :P

> (OK that last part was not true).

(ditto)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3 trunk] libctf, ld: fix data symbol test with newer GCC
  2021-01-09 11:39 2.36 branch created Nick Clifton
                   ` (2 preceding siblings ...)
  2021-01-12 19:11 ` Nick Alcock
@ 2021-01-18 12:53 ` Nick Alcock
  2021-01-18 12:53   ` [PATCH 2/3 trunk+branch] libctf: lookup_by_name: do not return success for nonexistent pointer types Nick Alcock
                     ` (2 more replies)
  3 siblings, 3 replies; 12+ messages in thread
From: Nick Alcock @ 2021-01-18 12:53 UTC (permalink / raw)
  To: binutils

GCC 11+ spots that the extern var_1 and var_666 declarations in this
test are unused, and removes them, thus stopping them from appearing as
conflicted data symbols and rendering the test pointless.  Use them in a
function unique to this TU to prevent them from being eliminated.

ld/ChangeLog
2021-01-06  Nick Alcock  <nick.alcock@oracle.com>

	* testsuite/ld-ctf/data-func-2.c: Stop removal of the extern foo_t
	symbols by the optimizer.
	* testsuite/ld-ctf/data-func-conflicted.d: Adjust accordingly.
---
 ld/testsuite/ld-ctf/data-func-2.c          | 4 ++++
 ld/testsuite/ld-ctf/data-func-conflicted.d | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

This is going on trunk only because it applies only to unreleased compilers
and affects only the testsuite.

diff --git a/ld/testsuite/ld-ctf/data-func-2.c b/ld/testsuite/ld-ctf/data-func-2.c
index d00d2309692..575b06ef30c 100644
--- a/ld/testsuite/ld-ctf/data-func-2.c
+++ b/ld/testsuite/ld-ctf/data-func-2.c
@@ -3,3 +3,7 @@ typedef char foo_t;
 /* Conflicting, and indexed.  */
 extern foo_t var_1;
 extern foo_t *var_666;
+
+int other_func(foo_t *);
+
+int ignore (void) { other_func (&var_1); other_func (var_666); }
diff --git a/ld/testsuite/ld-ctf/data-func-conflicted.d b/ld/testsuite/ld-ctf/data-func-conflicted.d
index 6b1e9145a02..1e7c19a5b36 100644
--- a/ld/testsuite/ld-ctf/data-func-conflicted.d
+++ b/ld/testsuite/ld-ctf/data-func-conflicted.d
@@ -14,9 +14,9 @@ Contents of CTF section \.ctf:
     Version: 4 \(CTF_VERSION_3\)
 #...
     Data object section:	.* \(0xc bytes\)
-    Function info section:	.* \(0x40 bytes\)
+    Function info section:	.* \(0x44 bytes\)
     Object index section:	.* \(0xc bytes\)
-    Type section:	.* \(0xe8 bytes\)
+    Type section:	.* \(0xf4 bytes\)
     String section:	.*
 #...
   Data objects:
-- 
2.30.0.252.gc27e85e57d


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/3 trunk+branch] libctf: lookup_by_name: do not return success for nonexistent pointer types
  2021-01-18 12:53 ` [PATCH 1/3 trunk] libctf, ld: fix data symbol test with newer GCC Nick Alcock
@ 2021-01-18 12:53   ` Nick Alcock
  2021-01-18 12:53   ` [PATCH 3/3 trunk+branch] libctf, create: fix ctf_type_add of structs with unnamed members Nick Alcock
  2021-01-19 14:09   ` [PATCH 1/3 trunk] libctf, ld: fix data symbol test with newer GCC Nick Alcock
  2 siblings, 0 replies; 12+ messages in thread
From: Nick Alcock @ 2021-01-18 12:53 UTC (permalink / raw)
  To: binutils

The recent work allowing lookups of pointers in child dicts when the
pointed-to type is in the parent dict broke the case where a pointer
type that does not exist at all is looked up: we mistakenly return the
pointed-to type, which is likely not a pointer at all.  This causes
considerable confusion.

Fixed, with a new testcase.

libctf/ChangeLog
2021-01-12  Nick Alcock  <nick.alcock@oracle.com>

	* ctf-lookup.c (ctf_lookup_by_name_internal): Do not return the
	base type if looking up a nonexistent pointer type.
	* testsuite/libctf-regression/pptrtab*: Test it.
---
 libctf/ctf-lookup.c                           | 34 ++++++++++++++-----
 .../testsuite/libctf-regression/pptrtab-a.c   |  2 ++
 .../testsuite/libctf-regression/pptrtab-b.c   |  3 +-
 libctf/testsuite/libctf-regression/pptrtab.c  | 10 +++++-
 4 files changed, 39 insertions(+), 10 deletions(-)

This breaks a widely-used function (probably the most widely-used querying
function in libctf given the absence of ctf_lookup_by_symbol until now)
with impacts on real software, was broken during this cycle, and is lookup
API only so cannot break the rest of binutils (it is not used by any
machinery called directly or indirectly by ld), so I'd hope it's OK on
the branch.

diff --git a/libctf/ctf-lookup.c b/libctf/ctf-lookup.c
index c7f7e297822..6d4e085838c 100644
--- a/libctf/ctf-lookup.c
+++ b/libctf/ctf-lookup.c
@@ -184,24 +184,36 @@ ctf_lookup_by_name_internal (ctf_dict_t *fp, ctf_dict_t *child,
 	     from resolving the type down to its base type and use that instead.
 	     This helps with cases where the CTF data includes "struct foo *"
 	     but not "foo_t *" and the user tries to access "foo_t *" in the
-	     debugger.  */
+	     debugger.
+
+	     There is extra complexity here because uninitialized elements in
+	     the pptrtab and ptrtab are set to zero, but zero (as the type ID
+	     meaning the unimplemented type) is a valid return type from
+	     ctf_lookup_by_name.  (Pointers to types are never of type 0, so
+	     this is unambiguous, just fiddly to deal with.)  */
 
 	  uint32_t idx = LCTF_TYPE_TO_INDEX (fp, type);
 	  int in_child = 0;
 
-	  ntype = type;
+	  ntype = CTF_ERR;
 	  if (child && idx <= child->ctf_pptrtab_len)
 	    {
 	      ntype = child->ctf_pptrtab[idx];
 	      if (ntype)
 		in_child = 1;
+	      else
+		ntype = CTF_ERR;
 	    }
 
-	  if (ntype == 0)
-	    ntype = fp->ctf_ptrtab[idx];
+	  if (ntype == CTF_ERR)
+	    {
+	      ntype = fp->ctf_ptrtab[idx];
+	      if (ntype == 0)
+		ntype = CTF_ERR;
+	    }
 
 	  /* Try resolving to its base type and check again.  */
-	  if (ntype == 0)
+	  if (ntype == CTF_ERR)
 	    {
 	      if (child)
 		ntype = ctf_type_resolve_unsliced (child, type);
@@ -213,16 +225,22 @@ ctf_lookup_by_name_internal (ctf_dict_t *fp, ctf_dict_t *child,
 
 	      idx = LCTF_TYPE_TO_INDEX (fp, ntype);
 
-	      ntype = 0;
+	      ntype = CTF_ERR;
 	      if (child && idx <= child->ctf_pptrtab_len)
 		{
 		  ntype = child->ctf_pptrtab[idx];
 		  if (ntype)
 		    in_child = 1;
+		  else
+		    ntype = CTF_ERR;
 		}
 
-	      if (ntype == 0)
-		ntype = fp->ctf_ptrtab[idx];
+	      if (ntype == CTF_ERR)
+		{
+		  ntype = fp->ctf_ptrtab[idx];
+		  if (ntype == 0)
+		    ntype = CTF_ERR;
+		}
 	      if (ntype == CTF_ERR)
 		goto notype;
 	    }
diff --git a/libctf/testsuite/libctf-regression/pptrtab-a.c b/libctf/testsuite/libctf-regression/pptrtab-a.c
index e9f656a0bc8..65414877114 100644
--- a/libctf/testsuite/libctf-regression/pptrtab-a.c
+++ b/libctf/testsuite/libctf-regression/pptrtab-a.c
@@ -1,3 +1,5 @@
 typedef long a_t;
+typedef long b_t;
 
 a_t *a;
+b_t ignore2;
diff --git a/libctf/testsuite/libctf-regression/pptrtab-b.c b/libctf/testsuite/libctf-regression/pptrtab-b.c
index 6142f194c19..e458021efb1 100644
--- a/libctf/testsuite/libctf-regression/pptrtab-b.c
+++ b/libctf/testsuite/libctf-regression/pptrtab-b.c
@@ -1,4 +1,5 @@
 typedef long a_t;
+typedef long b_t;
 
 a_t b;
-
+b_t ignore1;
diff --git a/libctf/testsuite/libctf-regression/pptrtab.c b/libctf/testsuite/libctf-regression/pptrtab.c
index 5d3c2f2ee93..fe1b8fe2b43 100644
--- a/libctf/testsuite/libctf-regression/pptrtab.c
+++ b/libctf/testsuite/libctf-regression/pptrtab.c
@@ -23,13 +23,18 @@ main (int argc, char *argv[])
     goto open_err;
 
   /* Make sure we can look up a_t * by name in all non-parent dicts, even though
-     the a_t * and the type it points to are in distinct dicts.  */
+     the a_t * and the type it points to are in distinct dicts; make sure we
+     cannot look up b_t *.  */
 
   while ((fp = ctf_archive_next (ctf, &i, &arcname, 1, &err)) != NULL)
     {
       if ((type = ctf_lookup_by_name (fp, "a_t *")) == CTF_ERR)
 	goto err;
 
+      if ((ctf_lookup_by_name (fp, "b_t *")) != CTF_ERR ||
+          ctf_errno (fp) != ECTF_NOTYPE)
+	goto noerr;
+
       if (ctf_type_reference (fp, type) == CTF_ERR)
 	goto err;
 
@@ -51,4 +56,7 @@ main (int argc, char *argv[])
  err:
   fprintf (stderr, "Lookup failed in %s: %s\n", arcname, ctf_errmsg (ctf_errno (fp)));
   return 1;
+ noerr:
+  fprintf (stderr, "Lookup unexpectedly succeeded in %s\n", arcname);
+  return 1;
 }
-- 
2.30.0.252.gc27e85e57d


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/3 trunk+branch] libctf, create: fix ctf_type_add of structs with unnamed members
  2021-01-18 12:53 ` [PATCH 1/3 trunk] libctf, ld: fix data symbol test with newer GCC Nick Alcock
  2021-01-18 12:53   ` [PATCH 2/3 trunk+branch] libctf: lookup_by_name: do not return success for nonexistent pointer types Nick Alcock
@ 2021-01-18 12:53   ` Nick Alcock
  2021-01-19 14:09   ` [PATCH 1/3 trunk] libctf, ld: fix data symbol test with newer GCC Nick Alcock
  2 siblings, 0 replies; 12+ messages in thread
From: Nick Alcock @ 2021-01-18 12:53 UTC (permalink / raw)
  To: binutils

Our recent commit to support unnamed structure members better ditched
the old ctf_member_iter iterator body in favour of ctf_member_next.
However, these functions treat unnamed structure members differently:
ctf_member_iter just returned whatever the internal representation
contained, while ctf_member_next took care to always return "" rather
than sometimes returning "" and sometimes NULL depending on whether the
dict was dynamic (a product of ctf_create) or not (a product of
ctf_open).  After this commit, ctf_member_iter did the same.

It was always a bug for external callers not to treat a "" return from
these functions as if it were NULL, so only buggy callers could be
affected -- but one of those buggy callers was ctf_add_type, which
assumed that it could just take whatever name was returned from
ctf_member_iter and slam it directly into the internal representation of
a dynamic dict -- which expects NULL for unnamed members, not "".  The
net effect of all of this is that taking a struct containing unnamed
members and ctf_add_type'ing it into a dynamic dict produced a dict
whose unnamed members were inaccessible to ctf_member_info (though if
you wrote that dict out and then ctf_open'ed it, they would magically
reappear again).

Compensate for this by suitably transforming a "" name into NULL in the
internal representation, as should have been done all along.

libctf/ChangeLog
2021-01-12  Nick Alcock  <nick.alcock@oracle.com>

	* ctf-create.c (membadd): Transform ""-named members into
	NULL-named ones.
	* testsuite/libctf-regression/type-add-unnamed-struct*: New test.
---
 libctf/ctf-create.c                           |  6 ++
 .../type-add-unnamed-struct-ctf.c             | 19 +++++
 .../type-add-unnamed-struct.c                 | 72 +++++++++++++++++++
 .../type-add-unnamed-struct.lk                |  3 +
 4 files changed, 100 insertions(+)
 create mode 100644 libctf/testsuite/libctf-regression/type-add-unnamed-struct-ctf.c
 create mode 100644 libctf/testsuite/libctf-regression/type-add-unnamed-struct.c
 create mode 100644 libctf/testsuite/libctf-regression/type-add-unnamed-struct.lk

This was recently introduced and can actually break the linker (though only in the
deprecated non-deduplicating mode), and the fix is suitably simple.  I hope it's
OK on the branch.

diff --git a/libctf/ctf-create.c b/libctf/ctf-create.c
index 651d39d23c8..50f48eb1bb9 100644
--- a/libctf/ctf-create.c
+++ b/libctf/ctf-create.c
@@ -2403,6 +2403,12 @@ membadd (const char *name, ctf_id_t type, unsigned long offset, void *arg)
   if ((dmd = malloc (sizeof (ctf_dmdef_t))) == NULL)
     return (ctf_set_errno (ctb->ctb_dict, EAGAIN));
 
+  /* Unnamed members in non-dynamic dicts have a name of "", while dynamic dicts
+     use NULL.  Adapt.  */
+
+  if (name[0] == 0)
+    name = NULL;
+
   if (name != NULL && (s = strdup (name)) == NULL)
     {
       free (dmd);
diff --git a/libctf/testsuite/libctf-regression/type-add-unnamed-struct-ctf.c b/libctf/testsuite/libctf-regression/type-add-unnamed-struct-ctf.c
new file mode 100644
index 00000000000..d319aafbac1
--- /dev/null
+++ b/libctf/testsuite/libctf-regression/type-add-unnamed-struct-ctf.c
@@ -0,0 +1,19 @@
+struct foo
+{
+  union
+  {
+    struct
+    {
+      int bar;
+    };
+  };
+  union
+  {
+    struct
+    {
+      int baz;
+    };
+  };
+};
+
+struct foo *bar;
diff --git a/libctf/testsuite/libctf-regression/type-add-unnamed-struct.c b/libctf/testsuite/libctf-regression/type-add-unnamed-struct.c
new file mode 100644
index 00000000000..98be257991a
--- /dev/null
+++ b/libctf/testsuite/libctf-regression/type-add-unnamed-struct.c
@@ -0,0 +1,72 @@
+#include <ctf-api.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int
+main (int argc, char *argv[])
+{
+  ctf_dict_t *fp;
+  ctf_archive_t *ctf;
+  ctf_dict_t *dyn;
+  ctf_next_t *i = NULL;
+  ctf_id_t type;
+  ctf_id_t newtype;
+  const char *memb;
+  ctf_membinfo_t mi;
+  const char *membs[] = { "bar", "baz", NULL };
+  const char **walk;
+  int err;
+
+  if (argc != 2)
+    {
+      fprintf (stderr, "Syntax: %s PROGRAM\n", argv[0]);
+      exit(1);
+    }
+
+  if ((ctf = ctf_open (argv[1], NULL, &err)) == NULL)
+    goto open_err;
+
+  if ((fp = ctf_dict_open (ctf, NULL, &err)) == NULL)
+    goto open_err;
+
+  if ((dyn = ctf_create (&err)) == NULL)
+    goto create_err;
+
+  /* Copy 'struct foo' into the dynamic dict, then make sure we can look up a
+     member situated inside an unnamed struct.  */
+
+  if ((type = ctf_lookup_by_name (fp, "struct foo")) == CTF_ERR)
+    {
+      fprintf (stderr, "Cannot look up struct foo: %s\n", ctf_errmsg (ctf_errno (dyn)));
+      return 1;
+    }
+
+  if ((newtype = ctf_add_type (dyn, fp, type)) == CTF_ERR)
+    goto copy_err;
+
+  for (walk = membs; *walk != NULL; walk++)
+    {
+      if (ctf_member_info (dyn, newtype, *walk, &mi) < 0)
+        goto lookup_err;
+      printf ("Looked up %s, type %lx, offset %lx\n", *walk, (long) mi.ctm_type, mi.ctm_offset);
+    }
+
+  ctf_dict_close (dyn);
+  ctf_dict_close (fp);
+  ctf_close (ctf);
+
+  return 0;
+
+ open_err:
+  fprintf (stderr, "%s: cannot open: %s\n", argv[0], ctf_errmsg (err));
+  return 1;
+ create_err:
+  fprintf (stderr, "%s: cannot create: %s\n", argv[0], ctf_errmsg (err));
+  return 1;
+ copy_err:
+  fprintf (stderr, "Type addition failed: %s\n", ctf_errmsg (ctf_errno (dyn)));
+  return 1;
+ lookup_err:
+  fprintf (stderr, "Cannot look up %s: %s\n", *walk, ctf_errmsg (ctf_errno (dyn)));
+  return 1;
+}
diff --git a/libctf/testsuite/libctf-regression/type-add-unnamed-struct.lk b/libctf/testsuite/libctf-regression/type-add-unnamed-struct.lk
new file mode 100644
index 00000000000..caa8934ed2e
--- /dev/null
+++ b/libctf/testsuite/libctf-regression/type-add-unnamed-struct.lk
@@ -0,0 +1,3 @@
+# source: type-add-unnamed-struct-ctf.c
+Looked up bar, type [0-9a-f]*, offset [0-9a-f]*
+Looked up baz, type [0-9a-f]*, offset [0-9a-f]*
-- 
2.30.0.252.gc27e85e57d


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3 trunk] libctf, ld: fix data symbol test with newer GCC
  2021-01-18 12:53 ` [PATCH 1/3 trunk] libctf, ld: fix data symbol test with newer GCC Nick Alcock
  2021-01-18 12:53   ` [PATCH 2/3 trunk+branch] libctf: lookup_by_name: do not return success for nonexistent pointer types Nick Alcock
  2021-01-18 12:53   ` [PATCH 3/3 trunk+branch] libctf, create: fix ctf_type_add of structs with unnamed members Nick Alcock
@ 2021-01-19 14:09   ` Nick Alcock
  2021-01-19 14:14     ` Nick Clifton
  2 siblings, 1 reply; 12+ messages in thread
From: Nick Alcock @ 2021-01-19 14:09 UTC (permalink / raw)
  To: binutils

On 18 Jan 2021, Nick Alcock via Binutils uttered the following:

> GCC 11+ spots that the extern var_1 and var_666 declarations in this
> test are unused, and removes them, thus stopping them from appearing as
> conflicted data symbols and rendering the test pointless.  Use them in a
> function unique to this TU to prevent them from being eliminated.

This, and all other (more significant!) commits in this series, are now
pushed to master, but not yet to the release branch until Nick acks my
doing that.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3 trunk] libctf, ld: fix data symbol test with newer GCC
  2021-01-19 14:09   ` [PATCH 1/3 trunk] libctf, ld: fix data symbol test with newer GCC Nick Alcock
@ 2021-01-19 14:14     ` Nick Clifton
  0 siblings, 0 replies; 12+ messages in thread
From: Nick Clifton @ 2021-01-19 14:14 UTC (permalink / raw)
  To: Nick Alcock, binutils

Hi Nick,

> This, and all other (more significant!) commits in this series, are now
> pushed to master, but not yet to the release branch until Nick acks my
> doing that.

Please consider them ACK'ed.

Cheers
   Nick



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-01-19 14:15 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-09 11:39 2.36 branch created Nick Clifton
2021-01-09 14:09 ` [PATCH] ld/x86-64: Properly Handle -z lam-u48/lam-u57 (2.36 branch created.) H.J. Lu
2021-01-12 13:23 ` 2.36 branch created H.J. Lu
2021-01-12 13:27   ` Nick Clifton
2021-01-12 19:11 ` Nick Alcock
2021-01-13 10:02   ` Nick Clifton
2021-01-13 16:24     ` Nick Alcock
2021-01-18 12:53 ` [PATCH 1/3 trunk] libctf, ld: fix data symbol test with newer GCC Nick Alcock
2021-01-18 12:53   ` [PATCH 2/3 trunk+branch] libctf: lookup_by_name: do not return success for nonexistent pointer types Nick Alcock
2021-01-18 12:53   ` [PATCH 3/3 trunk+branch] libctf, create: fix ctf_type_add of structs with unnamed members Nick Alcock
2021-01-19 14:09   ` [PATCH 1/3 trunk] libctf, ld: fix data symbol test with newer GCC Nick Alcock
2021-01-19 14:14     ` Nick Clifton

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).