[PATCH 1/8] libctf, dump: do not emit size or alignment if it would error

[PATCH 1/8] libctf, dump: do not emit size or alignment if it would error

[Nick Alcock]

When we dump normal types, we emit their size and/or alignment:
but size and alignment dumping can return errors if the type is
part of a chain that terminates in a forward.

Emitting 0xffffffff as a size or alignment is unhelpful, so simply
skip emitting this info for any type for which size or alignment
checks return an error, no matter what the error is.

libctf/ChangeLog
2021-03-23  Nick Alcock  <nick.alcock@oracle.com>

	* ctf-dump.c (ctf_dump_format_type): Don't emit size or alignment
	on error.
---
 libctf/ctf-dump.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/libctf/ctf-dump.c b/libctf/ctf-dump.c
index 8540212eadd..bd9b50d9479 100644
--- a/libctf/ctf-dump.c
+++ b/libctf/ctf-dump.c
@@ -100,6 +100,7 @@ ctf_dump_format_type (ctf_dict_t *fp, ctf_id_t id, int flag)
       ctf_encoding_t ep;
       ctf_arinfo_t ar;
       int kind, unsliced_kind;
+      ssize_t size, align;
       const char *nonroot_leader = "";
       const char *nonroot_trailer = "";
       const char *idstr = "";
@@ -180,10 +181,10 @@ ctf_dump_format_type (ctf_dict_t *fp, ctf_id_t id, int flag)
 	  bit = NULL;
 	}
 
-      if (kind != CTF_K_FUNCTION && kind != CTF_K_FORWARD)
+      size = ctf_type_size (fp, id);
+      if (kind != CTF_K_FUNCTION && size >= 0)
 	{
-	  if (asprintf (&bit, " (size 0x%lx)",
-			(unsigned long) ctf_type_size (fp, id)) < 0)
+	  if (asprintf (&bit, " (size 0x%lx)", (unsigned long int) size) < 0)
 	    goto oom;
 
 	  str = str_append (str, bit);
@@ -191,10 +192,11 @@ ctf_dump_format_type (ctf_dict_t *fp, ctf_id_t id, int flag)
 	  bit = NULL;
 	}
 
-      if (kind != CTF_K_FORWARD)
+      align = ctf_type_align (fp, id);
+      if (align >= 0)
 	{
 	  if (asprintf (&bit, " (aligned at 0x%lx)",
-			(unsigned long) ctf_type_align (fp, id)) < 0)
+			(unsigned long int) align) < 0)
 	    goto oom;
 
 	  str = str_append (str, bit);

base-commit: 70e958370c8f4dc3222b658a725571d8d2f5f98a
-- 
2.31.0.253.gdec51257f3

[PATCH 2/8] include: always do unsigned left-shift in CTF_SET_STID

[Nick Alcock]

This turns into a signed left shift by 31 bits, otherwise.  This is an
offset and is always treated as unsigned in any case, so add an
appropriate cast.

include/ChangeLog
2021-03-23  Nick Alcock  <nick.alcock@oracle.com>

	PR libctf/27628
	* ctf-api.h: Fix some indentation.
	(CTF_SET_STID): Always do an unsigned shift, even if STID is
	signed.
---
 include/ctf.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/ctf.h b/include/ctf.h
index 50605743ab6..90631fccbd6 100644
--- a/include/ctf.h
+++ b/include/ctf.h
@@ -358,9 +358,9 @@ union
    c.ctt_info = CTF_TYPE_INFO(kind, vlen);
    c.ctt_name = CTF_TYPE_NAME(stid, offset);  */
 
-# define CTF_V1_INFO_KIND(info)		(((info) & 0xf800) >> 11)
-# define CTF_V1_INFO_ISROOT(info)	(((info) & 0x0400) >> 10)
-# define CTF_V1_INFO_VLEN(info)		(((info) & CTF_MAX_VLEN_V1))
+#define CTF_V1_INFO_KIND(info)		(((info) & 0xf800) >> 11)
+#define CTF_V1_INFO_ISROOT(info)	(((info) & 0x0400) >> 10)
+#define CTF_V1_INFO_VLEN(info)		(((info) & CTF_MAX_VLEN_V1))
 
 #define CTF_V2_INFO_KIND(info)		(((info) & 0xfc000000) >> 26)
 #define CTF_V2_INFO_ISROOT(info)	(((info) & 0x2000000) >> 25)
@@ -368,7 +368,7 @@ union
 
 #define CTF_NAME_STID(name)		((name) >> 31)
 #define CTF_NAME_OFFSET(name)		((name) & CTF_MAX_NAME)
-#define CTF_SET_STID(name, stid)	((name) | (stid) << 31)
+#define CTF_SET_STID(name, stid)	((name) | ((unsigned int) stid) << 31)
 
 /* V2 only. */
 #define CTF_TYPE_INFO(kind, isroot, vlen) \
@@ -387,10 +387,10 @@ union
 #define CTF_V2_TYPE_TO_INDEX(id)	((id) & CTF_MAX_PTYPE)
 #define CTF_V2_INDEX_TO_TYPE(id, child) ((child) ? ((id) | (CTF_MAX_PTYPE+1)) : (id))
 
-# define CTF_V1_TYPE_ISPARENT(fp, id)	((id) <= CTF_MAX_PTYPE_V1)
-# define CTF_V1_TYPE_ISCHILD(fp, id)	((id) > CTF_MAX_PTYPE_V1)
-# define CTF_V1_TYPE_TO_INDEX(id)	((id) & CTF_MAX_PTYPE_V1)
-# define CTF_V1_INDEX_TO_TYPE(id, child) ((child) ? ((id) | (CTF_MAX_PTYPE_V1+1)) : (id))
+#define CTF_V1_TYPE_ISPARENT(fp, id)	((id) <= CTF_MAX_PTYPE_V1)
+#define CTF_V1_TYPE_ISCHILD(fp, id)	((id) > CTF_MAX_PTYPE_V1)
+#define CTF_V1_TYPE_TO_INDEX(id)	((id) & CTF_MAX_PTYPE_V1)
+#define CTF_V1_INDEX_TO_TYPE(id, child) ((child) ? ((id) | (CTF_MAX_PTYPE_V1+1)) : (id))
 
 /* Valid for both V1 and V2. */
 #define CTF_TYPE_LSIZE(cttp) \
-- 
2.31.0.253.gdec51257f3

[PATCH 3/8] libctf, serialize: functions with no args have a NULL dtd_vlen

[Nick Alcock]

Every place that accesses a function's dtd_vlen accesses it only if the
number of args is nonzero, except the serializer, which always tries to
memcpy it.  The number of bytes it memcpys in this case is zero, but it
is still undefined behaviour to copy zero bytes from a null pointer.
So check for this case explicitly.

libctf/ChangeLog
2021-03-23  Nick Alcock  <nick.alcock@oracle.com>

	PR libctf/27628
	* ctf-serialize.c (ctf_emit_type_sect): Allow for a NULL vlen in
	CTF_K_FUNCTION types.
---
 libctf/ctf-serialize.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libctf/ctf-serialize.c b/libctf/ctf-serialize.c
index 0811b7b6efe..9f50280176c 100644
--- a/libctf/ctf-serialize.c
+++ b/libctf/ctf-serialize.c
@@ -849,7 +849,9 @@ ctf_emit_type_sect (ctf_dict_t *fp, unsigned char **tptr)
 	  break;
 
 	case CTF_K_FUNCTION:
-	  memcpy (t, dtd->dtd_vlen, sizeof (uint32_t) * (vlen + (vlen & 1)));
+	  /* Functions with no args also have no vlen.  */
+	  if (dtd->dtd_vlen)
+	    memcpy (t, dtd->dtd_vlen, sizeof (uint32_t) * (vlen + (vlen & 1)));
 	  t += sizeof (uint32_t) * (vlen + (vlen & 1));
 	  break;
 
-- 
2.31.0.253.gdec51257f3

[PATCH 4/8] libctf: make ctf_bfdopen_ctfsect a debugger entry point

[Nick Alcock]

This makes it possible to use LIBCTF_DEBUG to debug things that happen
before the ctf_bfdopen_internal call that ctf_bfdopen_ctfsect eventually
thunks down to (symtab/strtab lookup, archive opening, etc).

This is not important for ctf_open callers, since ctf_fdopen already
calls libctf_init_debug, but ctf_bfdopen_ctfsect is a public entry point
that can be called directly (e.g. objdump and readelf both do so).

libctf/ChangeLog
2021-03-23  Nick Alcock  <nick.alcock@oracle.com>

	* ctf-open-bfd.c (ctf_bfdopen_ctfsect): Initialize debugging.
---
 libctf/ctf-open-bfd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libctf/ctf-open-bfd.c b/libctf/ctf-open-bfd.c
index 84e4af6a73a..950864118fc 100644
--- a/libctf/ctf-open-bfd.c
+++ b/libctf/ctf-open-bfd.c
@@ -99,6 +99,8 @@ ctf_bfdopen_ctfsect (struct bfd *abfd _libctf_unused_,
   char *strtab_alloc = NULL;
   int symsect_endianness = -1;
 
+  libctf_init_debug();
+
 #ifdef HAVE_BFD_ELF
   ctf_sect_t symsect, strsect;
   Elf_Internal_Shdr *symhdr;
-- 
2.31.0.253.gdec51257f3

[PATCH 5/8] libctf: don't dereference out-of-bounds locations in the qualifier hashtab

[Nick Alcock]

isqualifier, which is used by ctf_lookup_by_name to figure out if a
given word in a type name is a qualifier, takes the address of a
possibly out-of-bounds location before checking its bounds.

In any reasonable compiler this will just lead to a harmless address
computation that is then discarded if out-of-bounds, but it's still
undefined behaviour and the sanitizer rightly complains.

libctf/ChangeLog
2021-03-23  Nick Alcock  <nick.alcock@oracle.com>

	PR libctf/27628
	* ctf-lookup.c (isqualifier): Don't dereference out-of-bounds
	qhash values.
---
 libctf/ctf-lookup.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libctf/ctf-lookup.c b/libctf/ctf-lookup.c
index 9d1e6d8a4a2..e50c868c5b8 100644
--- a/libctf/ctf-lookup.c
+++ b/libctf/ctf-lookup.c
@@ -111,10 +111,13 @@ isqualifier (const char *s, size_t len)
   };
 
   int h = s[len - 1] + (int) len - 105;
+
+  if (h < 0 || (size_t) h >= sizeof (qhash) / sizeof (qhash[0]))
+    return 0;
+
   const struct qual *qp = &qhash[h];
 
-  return (h >= 0 && (size_t) h < sizeof (qhash) / sizeof (qhash[0])
-	  && (size_t) len == qp->q_len &&
+  return ((size_t) len == qp->q_len &&
 	  strncmp (qp->q_name, s, qp->q_len) == 0);
 }
 
-- 
2.31.0.253.gdec51257f3

[PATCH 6/8] libctf: fix memory leak in a test

[Nick Alcock]

Harmless, but causes noise that makes it harder to spot other leaks.

libctf/ChangeLog
2021-03-23  Nick Alcock  <nick.alcock@oracle.com>

	* testsuite/libctf-writable/symtypetab-nonlinker-writeout.c: Don't
	leak buf.
---
 libctf/testsuite/libctf-writable/symtypetab-nonlinker-writeout.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libctf/testsuite/libctf-writable/symtypetab-nonlinker-writeout.c b/libctf/testsuite/libctf-writable/symtypetab-nonlinker-writeout.c
index 98144de6391..bdc3faa944c 100644
--- a/libctf/testsuite/libctf-writable/symtypetab-nonlinker-writeout.c
+++ b/libctf/testsuite/libctf-writable/symtypetab-nonlinker-writeout.c
@@ -202,6 +202,7 @@ try_maybe_reporting (int report)
     }
 
   ctf_file_close (fp);
+  free (buf);
 
   return;
 
-- 
2.31.0.253.gdec51257f3

[PATCH 7/8] libctf: fix ELF-in-BFD checks in the presence of ASAN

[Nick Alcock]

The address sanitizer contains a redirector that captures dlopen calls,
so checks for dlopen with AC_SEARCH_LIBS will always conclude that
dlopen is present when the sanitizer is on.  This means it won't add
-ldl to LIBS even if needed, and the immediately-following attempt to
actually link with -lbfd will fail because libbfd also needs dlsym,
which ASAN does *not* contain a redirector for.

If we check for dlsym instead of dlopen, the check works whether ASAN is
on or off.  (bfd uses both in close proximity: if it needs one, it will
always need the other.)

libctf/ChangeLog
2021-03-23  Nick Alcock  <nick.alcock@oracle.com>

	* configure.ac: Check for dlsym, not dlopen.
	* configure: Regenerate.
---
 libctf/configure    | 24 ++++++++++++------------
 libctf/configure.ac |  2 +-
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/libctf/configure b/libctf/configure
index 61f4db47674..82bcf13a606 100755
--- a/libctf/configure
+++ b/libctf/configure
@@ -12798,9 +12798,9 @@ fi
 rm -f conftest.mmap conftest.txt
 
 # Needed for BFD capability checks.
-{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing dlopen" >&5
-$as_echo_n "checking for library containing dlopen... " >&6; }
-if ${ac_cv_search_dlopen+:} false; then :
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing dlsym" >&5
+$as_echo_n "checking for library containing dlsym... " >&6; }
+if ${ac_cv_search_dlsym+:} false; then :
   $as_echo_n "(cached) " >&6
 else
   ac_func_search_save_LIBS=$LIBS
@@ -12813,11 +12813,11 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char dlopen ();
+char dlsym ();
 int
 main ()
 {
-return dlopen ();
+return dlsym ();
   ;
   return 0;
 }
@@ -12830,25 +12830,25 @@ for ac_lib in '' dl; do
     LIBS="-l$ac_lib  $ac_func_search_save_LIBS"
   fi
   if ac_fn_c_try_link "$LINENO"; then :
-  ac_cv_search_dlopen=$ac_res
+  ac_cv_search_dlsym=$ac_res
 fi
 rm -f core conftest.err conftest.$ac_objext \
     conftest$ac_exeext
-  if ${ac_cv_search_dlopen+:} false; then :
+  if ${ac_cv_search_dlsym+:} false; then :
   break
 fi
 done
-if ${ac_cv_search_dlopen+:} false; then :
+if ${ac_cv_search_dlsym+:} false; then :
 
 else
-  ac_cv_search_dlopen=no
+  ac_cv_search_dlsym=no
 fi
 rm conftest.$ac_ext
 LIBS=$ac_func_search_save_LIBS
 fi
-{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_dlopen" >&5
-$as_echo "$ac_cv_search_dlopen" >&6; }
-ac_res=$ac_cv_search_dlopen
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_dlsym" >&5
+$as_echo "$ac_cv_search_dlsym" >&6; }
+ac_res=$ac_cv_search_dlsym
 if test "$ac_res" != no; then :
   test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
 
diff --git a/libctf/configure.ac b/libctf/configure.ac
index ae1f10d1f24..80644b89d67 100644
--- a/libctf/configure.ac
+++ b/libctf/configure.ac
@@ -68,7 +68,7 @@ ACX_PROG_CC_WARNING_OPTS([-Wall], [ac_libctf_warn_cflags])
 
 AC_FUNC_MMAP
 # Needed for BFD capability checks.
-AC_SEARCH_LIBS(dlopen, dl)
+AC_SEARCH_LIBS(dlsym, dl)
 AM_ZLIB
 
 GCC_ENABLE([libctf-hash-debugging], [no], [], [Enable expensive debugging of CTF deduplication type hashing])
-- 
2.31.0.253.gdec51257f3

[PATCH 8/8] ld: do not rely on the exact size of the CTF symtypetabs in test results

[Nick Alcock]

The data object and function info sections (collectively "symtypetabs")
usually (i.e. if non-indexed) have sizes defined by the size of the ELF
dynamic symbol table in the object they are linked to.  This means test
results should not depend on the exact sizes of these sections, because
adding entirely irrelevant symbols to the dynsym can cause spurious test
failures.  (This also means we should not match the offset of sections
that follow them, since those too depend on the exact size of the
symtypetab sections.)

Spotted by turning the sanitizer on, which introduced new dynsym entries
and expanded the symtypetab sizes to match.

ld/ChangeLog
2021-03-23  Nick Alcock  <nick.alcock@oracle.com>

	* testsuite/ld-ctf/array.d: Only check that the data object
	section is nonempty: do not check its exact size.
	* testsuite/ld-ctf/diag-parlabel.d: Likewise.
	* testsuite/ld-ctf/slice.d: Likewise.
	* testsuite/ld-ctf/data-func-conflicted.d: Likewise, and for the
	func info section too.
	* testsuite/ld-ctf/function.d: Likewise, for the func info section.
---
 ld/testsuite/ld-ctf/array.d                | 2 +-
 ld/testsuite/ld-ctf/data-func-conflicted.d | 6 +++---
 ld/testsuite/ld-ctf/diag-parlabel.d        | 4 ++--
 ld/testsuite/ld-ctf/function.d             | 2 +-
 ld/testsuite/ld-ctf/slice.d                | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/ld/testsuite/ld-ctf/array.d b/ld/testsuite/ld-ctf/array.d
index 16375620c03..ab262d915b5 100644
--- a/ld/testsuite/ld-ctf/array.d
+++ b/ld/testsuite/ld-ctf/array.d
@@ -13,7 +13,7 @@ Contents of CTF section .ctf:
     Magic number: 0xdff2
     Version: 4 \(CTF_VERSION_3\)
 #...
-    Data object section:	.* \(0x8 bytes\)
+    Data object section:	.* \(0x[1-9a-f][0-9a-f]* bytes\)
     Type section:	.* \(0x5c bytes\)
     String section:	.*
 
diff --git a/ld/testsuite/ld-ctf/data-func-conflicted.d b/ld/testsuite/ld-ctf/data-func-conflicted.d
index 1e7c19a5b36..87dd4b61e3b 100644
--- a/ld/testsuite/ld-ctf/data-func-conflicted.d
+++ b/ld/testsuite/ld-ctf/data-func-conflicted.d
@@ -13,8 +13,8 @@ Contents of CTF section \.ctf:
     Magic number: 0xdff2
     Version: 4 \(CTF_VERSION_3\)
 #...
-    Data object section:	.* \(0xc bytes\)
-    Function info section:	.* \(0x44 bytes\)
+    Data object section:	.* \(0x[1-9a-f][0-9a-f]* bytes\)
+    Function info section:	.* \(0x[1-9a-f][0-9a-f]* bytes\)
     Object index section:	.* \(0xc bytes\)
     Type section:	.* \(0xf4 bytes\)
     String section:	.*
@@ -39,7 +39,7 @@ CTF archive member: .*/data-func-1\.c:
 #...
     Parent name: \.ctf
     Compilation unit name: .*/data-func-1\.c
-    Data object section:	.* \(0xfa4 bytes\)
+    Data object section:	.* \(0x[1-9a-f][0-9a-f]* bytes\)
     Type section:	.* \(0xc bytes\)
     String section:	.*
 
diff --git a/ld/testsuite/ld-ctf/diag-parlabel.d b/ld/testsuite/ld-ctf/diag-parlabel.d
index bba161cf309..9d2c0860997 100644
--- a/ld/testsuite/ld-ctf/diag-parlabel.d
+++ b/ld/testsuite/ld-ctf/diag-parlabel.d
@@ -13,8 +13,8 @@ Contents of CTF section .ctf:
     Version: 4 \(CTF_VERSION_3\)
 #...
     Compilation unit name: .*A.c
-    Data object section:	0x0 -- 0x3 \(0x4 bytes\)
-    Type section:	0x4 -- 0x33 \(0x30 bytes\)
+    Data object section:	.* \(0x[1-9a-f][0-9a-f]* bytes\)
+    Type section:	.* \(0x30 bytes\)
     String section:	.*
 
   Labels:
diff --git a/ld/testsuite/ld-ctf/function.d b/ld/testsuite/ld-ctf/function.d
index 1584bdbedfd..e6cb20951f4 100644
--- a/ld/testsuite/ld-ctf/function.d
+++ b/ld/testsuite/ld-ctf/function.d
@@ -13,7 +13,7 @@ Contents of CTF section .ctf:
     Version: 4 \(CTF_VERSION_3\)
 #...
     Compilation unit name: .*function.c
-    Function info section:	.* \(0x4 bytes\)
+    Function info section:	.* \(0x[1-9a-f][0-9a-f]* bytes\)
     Type section:	.* \(0x90 bytes\)
     String section:	.*
 #...
diff --git a/ld/testsuite/ld-ctf/slice.d b/ld/testsuite/ld-ctf/slice.d
index 72a7074f9ef..efe32a87523 100644
--- a/ld/testsuite/ld-ctf/slice.d
+++ b/ld/testsuite/ld-ctf/slice.d
@@ -14,7 +14,7 @@ Contents of CTF section .ctf:
 #...
     Compilation unit name: .*slice.c
 #...
-    Data object section:	.* \(0x4 bytes\)
+    Data object section:	.* \(0x[1-9a-f][0-9a-f]* bytes\)
     Type section:	.* \(0xd0 bytes\)
     String section:	.*
 #...
-- 
2.31.0.253.gdec51257f3

Re: [PATCH 5/8] libctf: don't dereference out-of-bounds locations in the qualifier hashtab

[Hans-Peter Nilsson]

On Wed, 24 Mar 2021, Nick Alcock via Binutils wrote:

> diff --git a/libctf/ctf-lookup.c b/libctf/ctf-lookup.c
> index 9d1e6d8a4a2..e50c868c5b8 100644
> --- a/libctf/ctf-lookup.c
> +++ b/libctf/ctf-lookup.c
> @@ -111,10 +111,13 @@ isqualifier (const char *s, size_t len)
>    };
>
>    int h = s[len - 1] + (int) len - 105;
> +
> +  if (h < 0 || (size_t) h >= sizeof (qhash) / sizeof (qhash[0]))
> +    return 0;
> +
>    const struct qual *qp = &qhash[h];

Do we allow C99 these days?  In recent messages I got the
impression that we're still battling with pre-C90 artefacts.

If not, watch out for the declaration-after-statement there.

brgds, H-P

Re: [PATCH 5/8] libctf: don't dereference out-of-bounds locations in the qualifier hashtab

[Nick Alcock]

On 25 Mar 2021, Hans-Peter Nilsson uttered the following:

> On Wed, 24 Mar 2021, Nick Alcock via Binutils wrote:
>
>> diff --git a/libctf/ctf-lookup.c b/libctf/ctf-lookup.c
>> index 9d1e6d8a4a2..e50c868c5b8 100644
>> --- a/libctf/ctf-lookup.c
>> +++ b/libctf/ctf-lookup.c
>> @@ -111,10 +111,13 @@ isqualifier (const char *s, size_t len)
>>    };
>>
>>    int h = s[len - 1] + (int) len - 105;
>> +
>> +  if (h < 0 || (size_t) h >= sizeof (qhash) / sizeof (qhash[0]))
>> +    return 0;
>> +
>>    const struct qual *qp = &qhash[h];
>
> Do we allow C99 these days?  In recent messages I got the
> impression that we're still battling with pre-C90 artefacts.
> 
> If not, watch out for the declaration-after-statement there.

We have declaration-after-statements all over libctf, so if people
really do try to compile with a pre-C99 compiler, we'll know (and I'll
fix them all then and growl loudly).

For that matter there are also some in bfd, so it's not just me.

(But this one is totally gratuitous and doesn't even improve clarity, so
I'll fix it :) )

-- 
NULL && (void)