- Download Diff

Summary:

6885 CTF Everywhere Part 1

Review Request #1379 — Created Jan. 18, 2019 and submitted June 6, 2019, 3:56 a.m.

Information
Owner:	rm
Repository:	illumos-gate
Branch:	master
Bugs:	6885
Depends On:
Commit:	5931ecc...
Reviewers
Groups:	general
People:

Description

6885 CTF Everywhere Part 1

6886 Want ctfdiff

6887 ctfdump should be written in terms of libctf

6888 ctfmerge should be implemented in terms of libctf

6889 ctfconvert should be implemented in terms of libctf

6890 Want general workq 6891 Want general mergeq

6892 ctf_add_encoded assigns() incorrect byte size to types

6893 ctf_add_{struct,union,enum} can reuse forwards

6894 ctf_add_{struct,union,enum} occasionally forget to dirty the ctf_file_t

6895 ctf_add_member could better handle bitfields

6896 ctf_type_size() reports wrong size for forwards

6897 Want libctf ctf_kind_name() function

6898 Want libctf function to set struct/union size

Portions contributed by: John Levon <john.levon@joyent.com>

Portions contributed by: Richard Lowe <richlowe@richlowe.net>

Testing Done

Using ctfdump/ctfdiff to compare and contrast older CTF tools, extensive use at Joyent over past several years.

Issues

0
0
35
0
All Issues: 35

Description	From	Last Updated
Type of iidx is ulong_t, so please use %lu conversion specifier.	domag02	Jan. 24, 2019, 7:08 p.m.
Same.	domag02	Jan. 24, 2019, 7:08 p.m.
Same.	domag02	Jan. 24, 2019, 7:08 p.m.
Same.	domag02	Jan. 24, 2019, 7:08 p.m.
These lines should be: len = snprintf(NULL, 0, " [%lu] %ld", g_stats.cs_ndata, id); ctfdump_printf(CTFDUMP_OBJECTS, " [%lu] %ld %*s%s (%lu)\n",	domag02	Jan. 24, 2019, 7:08 p.m.
" [%lu] %s (%lu) returns: %ld args: (", g_stats.cs_nfuncs, name,	domag02	Jan. 24, 2019, 7:08 p.m.
ctfdump_printf(CTFDUMP_FUNCTIONS, "%ld%s", g_fargc[i],	domag02	Jan. 24, 2019, 7:08 p.m.
ctfdump_printf(CTFDUMP_LABELS, " %5ld %s\n", li->ctb_typeidx, name);	domag02	Jan. 24, 2019, 7:08 p.m.
ctfdump_printf(CTFDUMP_TYPES, "\t%s type=%ld off=%lu\n", member, type,	domag02	Jan. 24, 2019, 7:08 p.m.
ctfdump_fatal("type %ld missing name: %s\n", id,	domag02	Jan. 24, 2019, 7:08 p.m.
ctfdump_printf(CTFDUMP_TYPES, " <%ld> ", id);	domag02	Jan. 24, 2019, 7:08 p.m.
ctfdump_printf(CTFDUMP_TYPES, " [%ld] ", id);	domag02	Jan. 24, 2019, 7:08 p.m.
ctfdump_printf(CTFDUMP_TYPES, "%s refers to %ld", name,	domag02	Jan. 24, 2019, 7:08 p.m.
ctfdump_printf(CTFDUMP_TYPES, "%s contents: %ld, index: %ld",	domag02	Jan. 24, 2019, 7:08 p.m.
"%s returns: %ld args: (", name, ctc.ctc_return);	domag02	Jan. 24, 2019, 7:08 p.m.
ctfdump_printf(CTFDUMP_TYPES, "%ld%s", g_fargc[i],	domag02	Jan. 24, 2019, 7:08 p.m.
ctfdump_printf(CTFDUMP_TYPES, "%s (%zu bytes)\n", name, size);	domag02	Jan. 24, 2019, 7:08 p.m.
ctfdump_printf(CTFDUMP_TYPES, "typedef %s refers to %ld", name,	domag02	Jan. 24, 2019, 7:08 p.m.
ctfdump_printf(CTFDUMP_TYPES, "%s refers to %ld", name,	domag02	Jan. 24, 2019, 7:08 p.m.
ctfdump_printf(CTFDUMP_TYPES, "%s refers to %ld", name,	domag02	Jan. 24, 2019, 7:08 p.m.
ctfdump_printf(CTFDUMP_TYPES, "%s refers to %ld", name,	domag02	Jan. 24, 2019, 7:08 p.m.
ctfdump_fatal("type %ld missing name: %s\n", type,	domag02	Jan. 24, 2019, 7:08 p.m.
(void) printf("typedef %s{ /* 0x%zx bytes */\n",	domag02	Jan. 24, 2019, 7:08 p.m.
Trailing space.	domag02	Jan. 24, 2019, 7:08 p.m.
ctfmerge_fatal("failed to read symbol table entry %lu "	domag02	Jan. 24, 2019, 7:09 p.m.
ctfmerge_fatal("failed to set parallelism to %u: %s\n",	domag02	Jan. 24, 2019, 7:09 p.m.
ctf_dprintf("lbloff: %u\nobjtoff: %u\nfuncoff: %u\n" "typeoff: %u\nstroff: %u\nstrlen: %u\n",	domag02	Jan. 24, 2019, 7:09 p.m.
If cast to int is unnecessary, then these lines could be: ctf_dprintf("bad contents for array: %ld\n", arp->ctr_contents);	domag02	Jan. 24, 2019, 7:09 p.m.
ctf_dprintf("bad index for array: %ld\n", arp->ctr_index);	domag02	Jan. 24, 2019, 7:09 p.m.
Trailing space.	domag02	Jan. 24, 2019, 7:09 p.m.
Space followed by tab.	domag02	Jan. 24, 2019, 7:09 p.m.
See in /usr/src/lib/libdwarf/common/libdwarf.h: /* To enable printing with printf regardless of the actual underlying data type, we define the DW_PR_xxx ...	domag02	Jan. 24, 2019, 7:09 p.m.
"failed to get die from offset %" DW_PR_DUu ": %s\n",	domag02	Jan. 24, 2019, 7:09 p.m.
"unknown ELF data encoding: %u", ehdr.e_ident[EI_DATA]);	domag02	Jan. 24, 2019, 7:09 p.m.
"doing doing"	citrus	Jan. 18, 2019, 3:52 p.m.

Ship it!

I've read through and all I got you was this s/doing/done/

Since it's been in use at Joyent for so long and also in some of the other distributions, I say ship it and thanks!

usr/src/lib/mergeq/mergeq.c (Diff revision 1)
Show all issues
```
"doing doing"
```

Commit:

-	bdb707ed4353c2122ab5e648bed782536bc83e55
+	28bedde9acfc63adce4300145ad1ac837f545386

Diff:

Revision 2 (+14891 -1358)

Show changes

	exception_lists/cstyle
	exception_lists/hdrchk
	exception_lists/packaging
	usr/src/Makefile.master
	usr/src/Makefile.lint
	usr/src/Targetdirs
	usr/src/cmd/Makefile
	usr/src/cmd/ctfconvert/Makefile
	204 more

```
Format string mismatches, etc.
```

usr/src/cmd/ctfdiff/ctfdiff.c (Diff revision 1)

Show all issues

Type of iidx is ulong_t, so please use %lu conversion specifier.

usr/src/cmd/ctfdiff/ctfdiff.c (Diff revision 1)
Show all issues
```
Same.
```
usr/src/cmd/ctfdiff/ctfdiff.c (Diff revision 1)
Show all issues
```
Same.
```
usr/src/cmd/ctfdiff/ctfdiff.c (Diff revision 1)
Show all issues
```
Same.
```

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues

These lines should be:
    len = snprintf(NULL, 0, "  [%lu] %ld", g_stats.cs_ndata, id);
    ctfdump_printf(CTFDUMP_OBJECTS, "  [%lu] %ld %*s%s (%lu)\n",

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues


        "  [%lu] %s (%lu) returns: %ld args: (", g_stats.cs_nfuncs, name,

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues


        ctfdump_printf(CTFDUMP_FUNCTIONS, "%ld%s", g_fargc[i],

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues


    ctfdump_printf(CTFDUMP_LABELS, "  %5ld %s\n", li->ctb_typeidx, name);

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues


    ctfdump_printf(CTFDUMP_TYPES, "\t%s type=%ld off=%lu\n", member, type,

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues

            ctfdump_fatal("type %ld missing name: %s\n", id,

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues


        ctfdump_printf(CTFDUMP_TYPES, "  <%ld> ", id);

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues


        ctfdump_printf(CTFDUMP_TYPES, "  [%ld] ", id);

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues

        ctfdump_printf(CTFDUMP_TYPES, "%s refers to %ld", name,

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues

        ctfdump_printf(CTFDUMP_TYPES, "%s contents: %ld, index: %ld",

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues


            "%s returns: %ld args: (", name, ctc.ctc_return);

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues


            ctfdump_printf(CTFDUMP_TYPES, "%ld%s", g_fargc[i],

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues


        ctfdump_printf(CTFDUMP_TYPES, "%s (%zu bytes)\n", name, size);

rm Jan. 24, 2019, 7:09 p.m.
```
No, should be %zd, it's an ssize_t.
```

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues

        ctfdump_printf(CTFDUMP_TYPES, "typedef %s refers to %ld", name,

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues

        ctfdump_printf(CTFDUMP_TYPES, "%s refers to %ld", name,

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues

        ctfdump_printf(CTFDUMP_TYPES, "%s refers to %ld", name,

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues

        ctfdump_printf(CTFDUMP_TYPES, "%s refers to %ld", name,

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues

            ctfdump_fatal("type %ld missing name: %s\n", type,

usr/src/cmd/ctfdump/ctfdump.c (Diff revision 1)

Show all issues


            (void) printf("typedef %s{ /* 0x%zx bytes */\n",

usr/src/cmd/ctfmerge/Makefile (Diff revision 1)
Show all issues
```
Trailing space.
```

usr/src/cmd/ctfmerge/ctfmerge.c (Diff revision 1)

Show all issues

            ctfmerge_fatal("failed to read symbol table entry %lu "

usr/src/cmd/ctfmerge/ctfmerge.c (Diff revision 1)

Show all issues

        ctfmerge_fatal("failed to set parallelism to %u: %s\n",

usr/src/common/ctf/ctf_create.c (Diff revision 1)

Show all issues

    ctf_dprintf("lbloff: %u\nobjtoff: %u\nfuncoff: %u\n"
        "typeoff: %u\nstroff: %u\nstrlen: %u\n",

usr/src/common/ctf/ctf_create.c (Diff revision 1)

Show all issues

If cast to int is unnecessary, then these lines could be:
        ctf_dprintf("bad contents for array: %ld\n",
            arp->ctr_contents);

rm Jan. 24, 2019, 7:09 p.m.

I think that was from an old lint thing, not sure why I did that at some point. Ironically it wasn't that way in the original change.

usr/src/common/ctf/ctf_create.c (Diff revision 1)

Show all issues


        ctf_dprintf("bad index for array: %ld\n", arp->ctr_index);

usr/src/lib/Makefile.targ (Diff revision 1)
Show all issues
```
Trailing space.
```
usr/src/lib/libctf/common/ctf_diff.c (Diff revision 1)
Show all issues
```
Space followed by tab.
```

usr/src/lib/libctf/common/ctf_dwarf.c (Diff revision 1)

Show all issues

See in /usr/src/lib/libdwarf/common/libdwarf.h:

/* To enable printing with printf regardless of the
actual underlying data type, we define the DW_PR_xxx macros. */

    ctf_dprintf("dwmap: %p %" DW_PR_DUx "->%ld\n", dwmap, off, id);

usr/src/lib/libctf/common/ctf_dwarf.c (Diff revision 1)

Show all issues

            "failed to get die from offset %" DW_PR_DUu ": %s\n",

usr/src/lib/libctf/common/ctf_dwarf.c (Diff revision 1)

Show all issues


            "unknown ELF data encoding: %u", ehdr.e_ident[EI_DATA]);

rm Jan. 24, 2019, 7:09 p.m.
```
Changed to %hhu.
```

Commit:

-	28bedde9acfc63adce4300145ad1ac837f545386
+	07caa946166f4df05913e76e6267545ae26ab8a1

Diff:

Revision 3 (+14866 -1362)

Show changes

	exception_lists/cstyle
	exception_lists/hdrchk
	exception_lists/packaging
	usr/src/Makefile.master
	usr/src/Makefile.lint
	usr/src/Targetdirs
	usr/src/cmd/Makefile
	usr/src/cmd/ctfconvert/Makefile
	204 more

Commit:

-	07caa946166f4df05913e76e6267545ae26ab8a1
+	b3a764193227d089137d17096f3ae4940c2ed80e

Diff:

Revision 4 (+14867 -1362)

Show changes

	exception_lists/cstyle
	exception_lists/hdrchk
	exception_lists/packaging
	usr/src/Makefile.master
	usr/src/Makefile.lint
	usr/src/Targetdirs
	usr/src/cmd/Makefile
	usr/src/cmd/ctfconvert/Makefile
	204 more

Ship it!

```
Ship It!
```

Change Summary:

Rebase, etc.

Commit:

-	b3a764193227d089137d17096f3ae4940c2ed80e
+	64f96b23e10323ee378c9172b145b786f349027c

Diff:

Revision 5 (+14867 -1362)

Show changes

	exception_lists/cstyle
	exception_lists/hdrchk
	exception_lists/packaging
	usr/src/Makefile.master
	usr/src/Makefile.lint
	usr/src/Targetdirs
	usr/src/cmd/Makefile
	usr/src/cmd/ctfconvert/Makefile
	204 more

Change Summary:

Sorry, got the wires crossed slightly.

Commit:

-	64f96b23e10323ee378c9172b145b786f349027c
+	5931ecc286af085c74ff7f25dcd8591b4a440756

Diff:

Revision 6 (+14867 -1362)

Show changes

	exception_lists/cstyle
	exception_lists/hdrchk
	exception_lists/packaging
	usr/src/Makefile.master
	usr/src/Makefile.lint
	usr/src/Targetdirs
	usr/src/cmd/Makefile
	usr/src/cmd/ctfconvert/Makefile
	204 more

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

You have a pending review.

Review Board 3.0.18

6885 CTF Everywhere Part 1

Commit:

Diff:

Commit:

Diff:

Commit:

Diff:

Change Summary:

Commit:

Diff:

Change Summary:

Commit:

Diff:

Status: Closed (submitted)