Move mksubr from kdump into libsysdecode.

[jhb]

Summary

Move mksubr from kdump into libsysdecode.

Restructure this script so that it generates a header of tables instead
of a source file. The tables are included in a flags.c source file which
provides functions to decode various system call arguments.

For functions that decode an enumeration, the function returns a pointer
to a string for known values and NULL for unknown values.

For functions that do more complex decoding (typically of a bitmask), the
function accepts a pointer to a FILE object (open_memstream() can be used
as a string builder) to which decoded values are written. If the
function operates on a bitmask, the function returns true if any bits
were decoded or false if the entire value was valid. Additionally, the
third argument accepts a pointer to a value to which any undecoded bits
are stored. This pointer can be NULL if the caller doesn't care about
remaining bits.

In addition, add a sysdecode_signal() which doesn't use a generated table
but instead augments sys_siglist[] with SIGTHR, SIGLIBRT, and names for
SIGRT<n> signals.

Convert kdump over to using decoder functions from libsysdecode instead of
mksubr. truss also uses decoders from libsysdecode instead of private
lookup tables, though lookup tables for objects not decoded by kdump remain
in truss for now. Eventually most of these tables should move into
libsysdecode as the automated table generation approach from mksubr is
less stale than the static tables in truss.

Some changes have been made to truss and kdump output:

• The flags passed to open() are now properly decoded in that one of O_RDONLY, O_RDWR, O_WRONLY, or O_EXEC is always included in a decoded mask.
• Optional arguments to open(), openat(), and fcntl() are only printed in kdump if they exist (e.g. the mode is only printed for open() if O_CREAT is set in the flags).
• Print argument to F_GETLK/SETLK/SETLKW in kdump as a pointer, not int.
• Include all procctl() commands.
• Correctly decode pipe2() flags in truss by not assuming full open()-like flags with O_RDONLY, etc.
• Decode file flags passed to *chflags() as file flags (UF_* and SF_*) rather than as a file mode.
• Fix decoding of quotactl() commands by splitting out the two command components instead of assuming the raw command value matches the primary command component.

In addition, truss and kdump now build without triggering any warnings.

(Other comments not for the commit log):

I originally started with a "format" enum for the differences in truss
vs kdump formatting for invalid values and mask formatting. However,
that was clumsy and wasn't something that would be friendly to other
consumers.

The second step I used was to have function callbacks the client could
register for prefix/suffix of mask values and to handle invalid values
for enumerations. However, this still seemed kludgey to me.

I then settled on the current interface described in the commit log
message.

I still need to write manpages, but I'd like to make sure the interface
is going to be ok before I commit to that. I plan on having one manpage
for all of the "enumeration" decoders, a second one for all the "mask"
decoders, and additional pages for the remaining oddball cases.

Also, I wanted to get this chunk done first, then resume working on
filling in gaps in decoding in both kdump and truss part of which will
include moving static tables out of truss and into libsysdecode.

Test Plan

• Running kdump and truss on my 'syscall exerciser' test program on both amd64 and i386.

[lwhsu]

This patch seems cannot be applied cleanly to the latest -head (r305677), the automatically test is as following:

https://ci.freebsd.org/job/FreeBSD-head-amd64-build-phabric/43/console

I've tested by hand locally, it also shows the same error:

Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|Index: lib/libsysdecode/signal.c
|===================================================================
|--- lib/libsysdecode/signal.c
|+++ lib/libsysdecode/signal.c
--------------------------
Patching file lib/libsysdecode/signal.c using Plan A...
Hunk #1 succeeded at 1.
Hunk #2 failed at 22.
1 out of 2 hunks failed--saving rejects to lib/libsysdecode/signal.c.rej
done

Could you help to check this?

[kib]

lib/libsysdecode/flags.c

64

Bde once stated that <sys/*> must be ordered first, then userspace system includes, then everything else.

lib/libsysdecode/signal.c

38

Might be, provide thread-safe API and make a wrapper for truss/ktrace which uses static buffer for convenience ? In fact, I looked at callers and believe that all callers can easily pass on-stack buffer to sysdecode_signal().

[jhb]

In D7847#162849, @lwhsu wrote:

This patch seems cannot be applied cleanly to the latest -head (r305677), the automatically test is as following:

https://ci.freebsd.org/job/FreeBSD-head-amd64-build-phabric/43/console

I've tested by hand locally, it also shows the same error:

Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|Index: lib/libsysdecode/signal.c
|===================================================================
|--- lib/libsysdecode/signal.c
|+++ lib/libsysdecode/signal.c
--------------------------
Patching file lib/libsysdecode/signal.c using Plan A...
Hunk #1 succeeded at 1.
Hunk #2 failed at 22.
1 out of 2 hunks failed--saving rejects to lib/libsysdecode/signal.c.rej
done

Could you help to check this?

You can't apply the patch directly because git thinks it is a copy from sysdecode.h and only shows a relative patch (rather than an entirely new file). 'arc patch' in a clean checkout should give you a fully patched tree however.

[lwhsu]

In D7847#162879, @jhb wrote:

You can't apply the patch directly because git thinks it is a copy from sysdecode.h and only shows a relative patch (rather than an entirely new file). 'arc patch' in a clean checkout should give you a fully patched tree however.

Thanks for the information. I did test on a clean svn checkout and use arc patch D7847 to retrieve and apply the patch, but failed.
https://reviews.freebsd.org/D7847#00f74019 also shows it's a copy from sysdecode.h, so even I use arc export --unified D7847 cannot get a complete patch.
Did you use git for local development and generate the patch? Or should I check if this is an issue in phabricator?

[wblock]

Remember to bump .Dd in the four man pages. Thanks!

[jhb]

• Change sysdecode_signal() to be thread-safe.
• Sort includes.
• Trim includes.
• Exclude file flag masks and avoid nfssvc name collision.
• Add a _WANT_KERNEL_ERRNO to remove hack from kdump.
• Use timercmp() and timersub() and remove _KERNEL usage.
• First cut at manpages for the enum and mask decoders.
• Rename 'command' to 'cmd' to match existing prototype.
• Rename some methods and arguments to be more consistent.
• Add manpages and MLINKS.
• Add a synopsis section to sysdecode(3).
• Add page for sysdecode_atfd().
• Add sysdecode_atfd.3 to the build.
• Remove duplicate MLINKS.
• Catch up to renames.

[jhb]

Rebase.

[jhb]

Whoops, some comments I had below were never submitted (and both of these are done now).

However, in general I still have more manpages to write before this is ready for a final review.

lib/libsysdecode/flags.c

64

This is actually copied from kdump's mksubr (though with various #define _KERNEL's removed). I can try sorting it to see if it still compiles.

lib/libsysdecode/signal.c

38

Hmm, I guess I should go ahead and make it thread-safe. One could use TLS for 'sigbuf' perhaps. This is a method that I think jilles@ would like to have a variant of in libc for use by /bin/sh as well (came up in one of brooks@'s recent reviews). I believe Solaris has some sort of 'sig2str()' that is like this. Hmm, it doesn't include "SIG" prefix and it requires the caller to pass in a buffer with a size of SIG2STR_MAX (rather than accepting a caller-supplied length).

Reference to sigst2str:

http://www.manpages.spotlynx.com/solaris/man/sig2str.3C

[wblock]

lib/libsysdecode/sysdecode_atfd.3

45

Missing the word "function" here. Although I think it reads better to remove "The" on line 43.

lib/libsysdecode/sysdecode_enum.3

160

This and following entries might be easier to read without a leading "The" in the description, which are mostly redundant.

lib/libsysdecode/sysdecode_mask.3

148

As above, most of these do not need the leading "The", and are shorter and perhaps easier to read without it.

[jhb]

• Convert sysdecode_atfd() into an enum.
• Convert sysdecode_sockopt() to a regular enum function.
• Change sockopt_name to accept both level and optname.
• Add a manpage for sysdecode_sockopt_name.
• Convert sysdecode_socket_typewithflags to a mask decoder.
• Rename sysdecode_capname() to sysdecode_cap_rights() and add a manpage.
• Add manpage for sysdecode_quotactl_cmd.
• Add manpage for sysdecode_fcntl_arg.3 and sysdecode_fcntl_arg_p.3.
• Add a manpage for sysdecode_sigcode.
• Change sysdecode_signal() to be a simple enumerator.

[jhb]

• Fixes from mandoc -T lint.

[jhb]

Rebase.

[jhb]

FYI, I have written manpages for all the new functions now and think this is suitable for review. As part of writing manpages for some of the more "oddball" functions I ended up converting some of them to use semantics like the existing enum/mask functions (e.g. sysdecode_signal and sysdecode_socket_type), so there are some code changes relative to earlier revisions as well.

lib/libsysdecode/signal.c

38

FYI, even though it bloats the size somewhat I decided to use a flat table that expands all the names as that makes the user API much simpler (always returns a const char * without having to worry about having a large enough string, etc.).

lib/libsysdecode/sysdecode_enum.3

160

So I'm a bit torn. The leading article certainly reads better to me. I was parsing each table entry as "<column1> decodes <column2>", in which case the article is needed if <column2> is singular. "foo decodes resource limits" works without an article for example, but "foo decodes resource limit" does not, that needs to be "foo decodes a resource limit". Similarly, "foo decodes bar argument to baz" doesn't read correctly either. If the table had 3 columns (function, associated system call, arg) then those would not need any articles, etc., but some of these functions decode things that aren't syscall arguments. If you think it would be clearer I could split the tables to make 1 for the syscall argument case that is 3 columns as described and a second table for the remaining cases.

[wblock]

lib/libsysdecode/sysdecode_enum.3

160

I suspect the column lists will pay off in clarity, but it's a bunch more markup. Your call.

lib/libsysdecode/sysdecode_quotactl_cmd.3

61 ↗

(On Diff #21269)

This sounds a little weird, could be "neither the primary nor the secondary" or just make it "neither of the".

[jhb]

• Use separate tables for system call argument and signal code decoders.

[wblock]

Wow, the rendered version is not only shorter but easier to read. Thank you!

One comment remains about sysdecode_abi_to_freebsd_errno.3, but it's not critical. Man pages accepted!

[jhb]

lib/libsysdecode/sysdecode_quotactl_cmd.3

61 ↗

(On Diff #21269)

Oops, missed this. It is indeed weird, though I think I will resolve it by also simplifying the semantics of this function.

[jhb]

• Simplify quotatcl to only decode if the primary command is known.