mbox series

[00/19] libctf: various bugfixes (including a write into freed memory), and loosen constraints on enums

Message ID 20240730153707.168357-1-nick.alcock@oracle.com
Headers
Series libctf: various bugfixes (including a write into freed memory), and loosen constraints on enums |

Message

Nick Alcock July 30, 2024, 3:36 p.m. UTC 
  So a few months ago (as part of the work to add functions letting you find
the enumerator value corresponding to an enumerator without having to check
every enum in the dict) we tightened the constraints on enumerator naming in
CTF, and fixed up the deduplicator correspondingly: you could no longer add
multiple enumerators with the same name in one dict, just like you can't
do that in one translation unit in C.  This matches the general principle
"dicts are kind of like translation units", and seemed uncontroversial
(and was requested by multiple users).

Alas, it turns out that real software actually does add duplicate
enumerators to different enums in dicts -- in particular, pahole emits lots
of BTF like this, and tools that convert BTF to CTF are going to want to
emit something at least fairly similar into the CTF dict, rather than having
to reimplement half the deduplicator just in order to push those types and
all the types they reference into child dicts to get some enumerators added.

But detecting duplicate enumerators is still a nice thing to be able to do!
So fix this by adding a new concept of dict-wide flags the user can turn on
via a new ctf_dict_set_flag(), and make one of them a flag to turn on strict
enumerator checking: it's off by default, unbreaking such apps.


This is just one of a bunch of bugfixes that turned up as part of other
(upcoming) work, some fairly serious: in particular, after commit 483546ce4f3
("libctf: make ctf_serialize() actually serialize") CTF dicts have not been
compressed; linking a dict with non-root-visible types in it made them all
root-visible again (!), counting the number of dicts in an archive went wrong
on big-endian platforms, and we were leaking string refs when
freeing non-root-visible types.

This latter thing sounds harmless, but a 'string ref' is basically a reloc:
an instruction to please modify the value at particular address (to record a
final strtab offset) when serializing a strtab. Leaking it by not freeing
the ref when freeing the thing it points to means that on serialization
we'll update the ref and write into freed memory! It seems likely that the
combination of API calls that would do that has never been issued by
anything outside the libctf testsuite (they're all used here or there, but
some are pretty obscure), but in an abundance of caution I'll backport that
one commit all the way to 2.35, where the bug was introduced.

The rest are only going into master and the release branch, unless people
would rather they didn't.


All the usual tests passed. There are more of them now: in particular we are
now lightly testing the ctf_link machinery everywhere, including on
platforms without CTF support in GNU ld. This has shaken out one hard-to-fix
outstanding bug: ctf_link is broken on mingw because it uses tmpfile() and
tmpfile() is utterly broken on mingw. For now I've not fixed this, but just
skipped those tests on gnulib: I tried, but gnulib fought back too hard.

Nick Alcock (19):
  libctf: we do in fact support foreign-endian old versions
  libctf, dedup: drop unnecessary arg from ctf_dedup()
  libctf, string: split the movable refs out of the ref list
  libctf, dump: correctly dump non-root-visible types
  libctf: fix linking of non-root-visible types
  libctf: fix CTF dict compression
  libctf: improve ECTF_NOPARENT error message
  libctf: dedup: tiny tweaks
  libctf: fix dynset insertion
  libctf, subr: don't mix up errors and warnings
  libctf, open: Fix enum error handling path
  libctf: link: fix error handling
  libctf: link: remember to turn off the LCTF_LINKING flag after
    ctf_link_write
  include, libctf: improve ECTF_DUPLICATE error message
  libctf, include: add ctf_dict_set_flag: less enum dup checking by
    default
  libctf: clean up hashtab error handling mess
  libctf: fix ref leak of names of newly-inserted non-root-visible types
  libctf: dump: fix small leak
  libctf: fix ctf_archive_count return value on big-endian

 include/ctf-api.h                             |  20 ++-
 ld/testsuite/ld-ctf/diag-parname.d            |   2 +-
 libctf/ctf-archive.c                          |  10 +-
 libctf/ctf-create.c                           |  52 +++++-
 libctf/ctf-dedup.c                            |  72 ++++----
 libctf/ctf-dump.c                             |   5 +-
 libctf/ctf-hash.c                             |   9 +-
 libctf/ctf-impl.h                             |  18 +-
 libctf/ctf-link.c                             |  37 +++-
 libctf/ctf-open.c                             |  66 +++-----
 libctf/ctf-serialize.c                        |  51 ++----
 libctf/ctf-string.c                           |  60 +++++--
 libctf/ctf-subr.c                             |  41 ++++-
 libctf/libctf.ver                             |   2 +
 .../libctf-lookup/enumerator-iteration.c      |  67 +++++++-
 .../libctf-writable/ctf-compressed.c          | 158 ++++++++++++++++++
 .../libctf-writable/ctf-compressed.lk         |   2 +
 .../libctf-writable/ctf-nonroot-linking.c     | 127 ++++++++++++++
 .../libctf-writable/ctf-nonroot-linking.lk    |   1 +
 19 files changed, 620 insertions(+), 180 deletions(-)
 create mode 100644 libctf/testsuite/libctf-writable/ctf-compressed.c
 create mode 100644 libctf/testsuite/libctf-writable/ctf-compressed.lk
 create mode 100644 libctf/testsuite/libctf-writable/ctf-nonroot-linking.c
 create mode 100644 libctf/testsuite/libctf-writable/ctf-nonroot-linking.lk

Comments

Nick Alcock Aug. 1, 2024, 11:26 a.m. UTC | #1 
On 30 Jul 2024, Nick Alcock said:

> All the usual tests passed.

... and also a bunch of unusual ones (building several thousand gentoo
packages with -gctf to make sure the linker was still happy).

Pushed to master -- one last check to make sure nobody objects to the
2.34 branch push.

(aside: my 'usual tests' for libctf are now documented here:
https://sourceware.org/binutils/wiki/CTF/Testing)
Nick Alcock Aug. 1, 2024, 11:46 a.m. UTC | #2 
On 1 Aug 2024, Nick Alcock outgrape:

> On 30 Jul 2024, Nick Alcock said:
>
>> All the usual tests passed.
>
> ... and also a bunch of unusual ones (building several thousand gentoo
> packages with -gctf to make sure the linker was still happy).
>
> Pushed to master -- one last check to make sure nobody objects to the
> 2.34 branch push.

Uh. That should obviously be 2.43: apparently I am living in the past.
(The heat is destroying my ability to think :/ )

Not pushed there yet, will do so shortly (quick subset of tests
running).
Nick Alcock Aug. 1, 2024, 2:44 p.m. UTC | #3 
On 1 Aug 2024, Nick Alcock spake thusly:

> On 1 Aug 2024, Nick Alcock outgrape:
>
>> On 30 Jul 2024, Nick Alcock said:
>>
>>> All the usual tests passed.
>>
>> ... and also a bunch of unusual ones (building several thousand gentoo
>> packages with -gctf to make sure the linker was still happy).
>>
>> Pushed to master -- one last check to make sure nobody objects to the
>> 2.34 branch push.
>
> Uh. That should obviously be 2.43: apparently I am living in the past.
> (The heat is destroying my ability to think :/ )
>
> Not pushed there yet, will do so shortly (quick subset of tests
> running).

I got paranoid and ran more tests, including a full valgrinding, leak
check, and ASAN of the libctf-related stuff. No failures.

Pushed to the binutils-2_43-branch. I hope it doesn't cause any
problems: if it does, feel free to revert.

Now to cherry-pick the nasty leak fix all the way back to 2.35 :(
(running the tests will, as usual, take the lion's share of the time).