truss: centralize pointer-constructing casts.

[brooks]

Summary

Arguments pointers are passed around as integrer types and need
to be converted to pointers when performing ptrace IO operations such as
in get_struct(). Move the casts into get_struct() and friends and take
a uintptr_t argument rather than a void *.

Add a print_pointer function that takes a uintptr_t and uses a single
case plus "%p" to print pointers.

[brooks]

This is extracted from CheriBSD. I'm not 100% it's something we want, but figure we should either upstream it (so I don't have compile errors every time a call to get_struct() is added) or remove it.

[jhb]

I think the original commit message for the void * -> uintptr_t change might be slightly better (removes a bunch of casts is a good reason). s/case/cast/ in the last sentence, though you could also just say "add a print_pointer() function to centralize printing of pointers". Even without CHERI print_pointer can be useful if we decided we wanted to pretty-print NULL as "NULL" instead of 0 for example.

[brooks]

For the records, the CheriBSD commit is: https://github.com/CTSRD-CHERI/cheribsd/commit/805aa75408d105dd2c069c04e3b551caae24c558

[cem]

This is a weird and not obviously better change for FreeBSD. I assume it makes sense for some particular reason on CHERI, but CHERI isn't even a Tier 4 FreeBSD arch. Most of the changes are just obfuscating formatted prints rather than any actual change to casts.

Anyway: I'm not keen on this kind of CHERI patch that just shuffles code around for CHERI's convenience with non-improvement or negative impact to FreeBSD.

[imp]

In D22212#485397, @cem wrote:

Anyway: I'm not keen on this kind of CHERI patch that just shuffles code around for CHERI's convenience with non-improvement or negative impact to FreeBSD.

The only obscuring thing here is the print_pointer() stuff, and even that's not terrible. The casting stuff is actually more correct than what we've had before. Given that CHERI++ is based on arm + FreeBSD, and I'd like to see that be well supported if it takes of, I'm willing to put up with a small amount of niggles like print_pointer() to enable it.

[cem]

Is "not terrible" the bar now? What casting stuff is more correct?

head/usr.bin/truss/syscalls.c

1189

This regresses the NULL case for a number of callers that explicitly prefixed 0x before (and the rest probably should have).

I am guessing you do it this way because uintmax_t is still broken on CHERIBSD, so you can't just do fprintf(fp, "0x%jx", (uintmax_t)arg);.

[jhb]

head/usr.bin/truss/syscalls.c

1189

Eh, %p on FreeBSD includes a 0x prefix. FreeBSD is not Linux:

		case 'p':
			/*-
			 * ``The argument shall be a pointer to void.  The
			 * value of the pointer is converted to a sequence
			 * of printable characters, in an implementation-
			 * defined manner.''
			 *	-- ANSI X3J11
			 */
			ujval = (uintmax_t)(uintptr_t)GETARG(void *);
			base = 16;
			xdigs = xdigs_lower;
			flags = flags | INTMAXT;
			ox[1] = 'x';
			goto nosign;
....
		if (ox[1]) {	/* ox[1] is either x, X, or \0 */
			ox[0] = '0';
			PRINT(ox, 2);
		}

The other reason for print_pointer is that it gives us a place to pretty-print NULL (as I said earlier) which I had wanted to do in truss back when doing the initial sysdecode work just hadn't gotten around to.

[cem]

head/usr.bin/truss/syscalls.c

1189

Ah, I didn't realize the userspace version of printf did that. The kernel version does not:

 796                 case 'p':
 797                         base = 16;
 798                         sharpflag = (width == 0);
 799                         sign = 0;
 800                         num = (uintptr_t)va_arg(ap, void *);
 801                         goto number;
...
 914                         if (sharpflag && num != 0) {   <<<<<<
 915                                 if (base == 8) {
 916                                         PCHAR('0');
 917                                 } else if (base == 16) {
 918                                         PCHAR('0');
 919                                         PCHAR('x');