NetBSD Problem Report #59402

From www@netbsd.org  Tue May  6 23:29:28 2025
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
	 client-signature RSA-PSS (2048 bits) client-digest SHA256)
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id ED4D81A923D
	for <gnats-bugs@gnats.NetBSD.org>; Tue,  6 May 2025 23:29:27 +0000 (UTC)
Message-Id: <20250506232926.9523D1A9241@mollari.NetBSD.org>
Date: Tue,  6 May 2025 23:29:26 +0000 (UTC)
From: david@gutteridge.ca
Reply-To: david@gutteridge.ca
To: gnats-bugs@NetBSD.org
Subject: Inconsistently-guarded macro definitions in ptrace.h header files
X-Send-Pr-Version: www-1.0

>Number:         59402
>Notify-List:    riastradh@NetBSD.org
>Category:       port-i386
>Synopsis:       Inconsistently-guarded macro definitions in ptrace.h header files
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    gutteridge
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue May 06 23:30:00 +0000 2025
>Closed-Date:    Tue Aug 12 23:42:54 +0000 2025
>Last-Modified:  Tue Aug 12 23:42:54 +0000 2025
>Originator:     David H. Gutteridge
>Release:        -current
>Organization:
TNF
>Environment:
>Description:
In sys/arch/i386/include/ptrace.h, __HAVE_PROCFS_MACHDEP is not guarded
by _KERNEL, despite the fact it only seems relevant in conjunction with
other situations that are restricted to _KERNEL, e.g., to contextually
reference PROCFS_MACHDEP_NODE_TYPES, or in use in source files that are
implicitly kernel internals. While this seems probably harmless in the
reasonable usages envisioned, it happened to cause a complication
failure with pkgsrc's sysutils/lsof (on i386 only), which, yes,
probably abuses header files (it is kind of a mess).

In comparison, sys/arch/powerpc/include/ptrace.h does guard
__HAVE_PROCFS_MACHDEP with _KERNEL, and does define it immediately
preceding PROCFS_MACHDEP_NODE_TYPES. I also see that
sys/arch/usermode/include/ptrace.h goes to the trouble to
#undef __HAVE_PROCFS_MACHDEP
perhaps to work around this very issue.

It seems for readability as much as anything else to make more sense to
guard it with _KERNEL and move it down to be contiguous with
PROCFS_MACHDEP_NODE_TYPES for i386. It's possible I've missed something
and this isn't correct, of course.

The same situation seems relevant to __HAVE_PTRACE_MACHDEP on i386;
I'm guessing that's why usermode also un-defines it, too.

(Again, while I'm not arguing lsof is "correct" here, making my
suggested change does fix compilation; it has no need of that
particular enum value provided by PROCFS_MACHDEP_NODE_TYPES, anyway.)

>How-To-Repeat:
As above.
>Fix:
As above. (I haven't supplied a patch, but can do so.)

>Release-Note:

>Audit-Trail:

State-Changed-From-To: open->feedback
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Tue, 06 May 2025 23:44:57 +0000
State-Changed-Why:
I assume you're referring to something like this build failure?

https://releng.netbsd.org/pkgreports/shadow/2025Q1/NetBSD-10.0-i386/20250503.1720/lsof-4.91nb7/build.log

--- dstore.o ---
In file included from dlsof.h:382,
                 from lsof.h:221,
                 from dstore.c:39:
/usr/include/miscfs/procfs/procfs.h:120:2: error: expected ',' or '}' before 'PFSlast'
  120 |  PFSlast, /* track number of types */
      |  ^~~~~~~

arising from:

     77 #ifdef _KERNEL
...
     83 typedef enum {
     84 	PFSauxv,	/* ELF Auxiliary Vector */
...
    129 	PFSversion,	/* kernel version (if -o linux) */
    130 #ifdef __HAVE_PROCFS_MACHDEP
 => 131 	PROCFS_MACHDEP_NODE_TYPES
    132 #endif
    133 	PFSlast,	/* track number of types */
    134 } pfstype;

https://nxr.netbsd.org/xref/src/sys/miscfs/procfs/procfs.h?r=1.87#77

Presumably this happens because PROCFS_MACHDEP_NODE_TYPES is not
defined as a macro, so the C compiler thinks it's another enumerand,
and expects a comma between it and PFSlast.

But how can you get to to line 131, which is conditional on both
_KERNEL and __HAVE_PROCFS_MACHDEP, without also getting to the line
that defines PROCFS_MACHDEP_NODE_TYPES, which is conditional only on
_KERNEL and __HAVE_PROCFS_MACHDEP (and !_I386_PTRACE_H_, I suppose)?

     83 /* We have machine-dependent procfs nodes. */
     84 #define	__HAVE_PROCFS_MACHDEP
...
    125 #ifdef _KERNEL
...
    139 #define	PROCFS_MACHDEP_NODE_TYPES					\
    140 	Pmachdep_xmmregs,	/* extended FP register set */

https://nxr.netbsd.org/xref/src/sys/arch/i386/include/ptrace.h?r=1.27#83

The change you suggest is fine -- should be harmless, and it's more
consistent with the other uses -- but it also shouldn't really be
necessary (__FOO namespace is reserved to us, the implementation, so
users who rummage around where they don't belong get their just
desserts) and I'd like to better understand how sysutils/lsof has dug
itself into this hole.

Does it maybe do something really stupid like this?

#include <sys/ptrace.h>

#define _KERNEL
#include <miscfs/procfs/procfs.h>
#undef _KERNEL

That's apt to cause all kinds of havoc...  I'm also hesitant to ask
what lsof hopes to accomplish by defining _KERNEL at all (and not, say,
_KMEMUSER or _KERNTYPES) other than shooting itself in the foot.


From: "David H. Gutteridge" <gutteridge@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59402 CVS commit: pkgsrc/sysutils/lsof
Date: Tue, 6 May 2025 23:46:46 +0000

 Module Name:	pkgsrc
 Committed By:	gutteridge
 Date:		Tue May  6 23:46:45 UTC 2025

 Modified Files:
 	pkgsrc/sysutils/lsof: distinfo
 	pkgsrc/sysutils/lsof/patches: patch-af

 Log Message:
 lsof: fix NetBSD i386 builds

 ptrace.h is being included in a messy way to kludge around other build
 issues, which then leads to this particular breakage on i386 only.
 Consistently defining or not defining both macros both fix this. lsof
 has no need of that particular enum value provided by
 PROCFS_MACHDEP_NODE_TYPES, anyway.

 Tangentially related to PR port-i386/59402; this code/our patching is
 doing unexpected things, but i386's ptrace.h does seem inconsistently
 defined, too.


 To generate a diff of this commit:
 cvs rdiff -u -r1.107 -r1.108 pkgsrc/sysutils/lsof/distinfo
 cvs rdiff -u -r1.23 -r1.24 pkgsrc/sysutils/lsof/patches/patch-af

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Responsible-Changed-From-To: port-i386-maintainer->gutteridge
Responsible-Changed-By: gutteridge@NetBSD.org
Responsible-Changed-When: Sat, 10 May 2025 19:43:28 +0000
Responsible-Changed-Why:
Take this, so it doesn't get forgotten, as I owe answers, testing any
proposed changes, etc.

From: "David H. Gutteridge" <david@gutteridge.ca>
To: gnats-bugs@netbsd.org
Cc: riastradh@NetBSD.org
Subject: Re: port-i386/59402 (Inconsistently-guarded macro definitions in
 ptrace.h header files)
Date: Fri, 13 Jun 2025 21:36:14 -0400

 On Tue, 06 May 2025 at 23:44:57 +0000, riastradh@NetBSD.org wrote:
 > [...] I'd like to better understand how sysutils/lsof has dug
 > itself into this hole.
 >
 > Does it maybe do something really stupid like this?
 >
 > #include <sys/ptrace.h>
 >
 > #define _KERNEL
 > #include <miscfs/procfs/procfs.h>
 > #undef _KERNEL
 >
 > That's apt to cause all kinds of havoc...  I'm also hesitant to ask
 > what lsof hopes to accomplish by defining _KERNEL at all (and not, say,
 > _KMEMUSER or _KERNTYPES) other than shooting itself in the foot.

 Yes, by (unfortunate) necessity, per tnn@'s commit patch-af v. 1.21:
 "machine/ptrace.h pulls in sys/module_hook.h after 2019-11-27 under _KERNEL
 This corresponds to NetBSD 9.99.19.
 sys/module_hook.h is not installed.
 Ergo sys/ptrace.h must be included /without/ defining _KERNEL."

 Which leads to the question why procfs.h with _KERNEL is required.
 That's because lsof wants to access procfs internals in order to report
 on different node types from its perspective. It's using the kvm(3)
 interface, which in combination with reaching over into kernel headers
 leads to fragility. There are a lot of hacks in some of the NetBSD
 support files it uses, some from upstream and quite old, others added by
 us later. (This extends well beyond procfs, e.g., it uses kvm(3) rather
 than the sysctl(3) interface to get PCB information, which led to more
 kludges.)

 I'm aware of several packages that try to deal with issues like this.
 lsof and net-snmp do it one way (many kludges to reach over into stuff
 expected to be kernel-only in installed base header files), while the
 haxm package instead sources headers found under BSDSRCDIR.

 Dave

 PS concerning kvm(3) rather than sysctl(3) use in general by these kinds
 of packages, I haven't analyzed all the requirements, but I inferred
 this isn't just due to a legacy practice but rather sysctl(3) simply
 doesn't expose everything these packages want, even where they appear
 to overlap in general (e.g., networking-related data).

State-Changed-From-To: feedback->analyzed
State-Changed-By: gutteridge@NetBSD.org
State-Changed-When: Sat, 14 Jun 2025 01:46:21 +0000
State-Changed-Why:
Details provided about lsof, etc.

From: "David H. Gutteridge" <gutteridge@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59402 CVS commit: pkgsrc/sysutils/lsof
Date: Tue, 17 Jun 2025 02:38:49 +0000

 Module Name:	pkgsrc
 Committed By:	gutteridge
 Date:		Tue Jun 17 02:38:49 UTC 2025

 Modified Files:
 	pkgsrc/sysutils/lsof: distinfo
 	pkgsrc/sysutils/lsof/patches: patch-af

 Log Message:
 lsof: simplify a NetBSD build fix patch

 Five years ago, r. 1.19 introduced a build fix for "NetBSD 9.0", without
 any further explanation. It appears this was intended for i386,
 specifically, though the commit didn't say, since other ports should
 have built without it. i386 builds have since been fixed differently,
 this being relevant for 10.x as well, where the older approach would not
 work. (It's also confusing to see separate places with NetBSD version
 conditionals pulling in sys/ptrace.h.)

 Change compile-tested on NetBSD 9.4_STABLE/amd64, 9.4_STABLE/i386,
 9.99.14/amd64, 9.99.14/i386. (I see nothing in the state of 9.0 headers
 that differs relevantly.)

 Slightly related to PR port-i386/59402.


 To generate a diff of this commit:
 cvs rdiff -u -r1.109 -r1.110 pkgsrc/sysutils/lsof/distinfo
 cvs rdiff -u -r1.24 -r1.25 pkgsrc/sysutils/lsof/patches/patch-af

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "David H. Gutteridge" <gutteridge@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59402 CVS commit: src/sys/arch/i386/include
Date: Tue, 12 Aug 2025 23:23:59 +0000

 Module Name:	src
 Committed By:	gutteridge
 Date:		Tue Aug 12 23:23:59 UTC 2025

 Modified Files:
 	src/sys/arch/i386/include: ptrace.h

 Log Message:
 i386/include/ptrace.h: move __HAVE_PROCFS_MACHDEP under _KERNEL

 For consistency and readability, guard with _KERNEL and move to be next
 to the PROCFS_MACHDEP_NODE_TYPES definition, as is done already with
 other ports like powerpc.

 As discussed in PR port-i386/59402. NFCI.


 To generate a diff of this commit:
 cvs rdiff -u -r1.27 -r1.28 src/sys/arch/i386/include/ptrace.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: analyzed->closed
State-Changed-By: gutteridge@NetBSD.org
State-Changed-When: Tue, 12 Aug 2025 23:42:54 +0000
State-Changed-Why:
Change applied, too trivial to pull up.

>Unformatted:

Home	PR Database Search