NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

lib/59391: unnecessary __PIC__ conditionals clutter .S files



>Number:         59391
>Category:       lib
>Synopsis:       unnecessary __PIC__ conditionals clutter .S files
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat May 03 13:40:00 +0000 2025
>Originator:     Taylor R Campbell
>Release:        current
>Organization:
Piccy Eaters Anonymous
>Environment:
>Description:
Example:

     81 #ifdef __PIC__
     82 	call	PIC_PLT(_C_LABEL(__sigprocmask14))
     83 #else
     84 	call	_C_LABEL(__sigprocmask14)
     85 #endif

https://nxr.netbsd.org/xref/src/lib/libc/arch/i386/gen/setjmp.S?r=1.18#81

This is unnecessary because PIC_PLT(x) is already defined to expand to x@PLT if __PIC__, and x if !__PIC__.

We have various cases of this in-tree.  Sometimes entire files are conditionalized on __PIC__, such as:

https://nxr.netbsd.org/xref/src/lib/libc/arch/sparc/gen/sigsetjmp.S?r=1.7

It would be nice if had fewer of these conditionals in .S files to make the .S files easier to read and audit with less duplication of logic.
>How-To-Repeat:
code inspection
>Fix:
Yes, please!

In some cases we can just eliminate the #ifdef __PIC__.  In other cases like the sparc files it may require some judicious #ifdef __PIC__ macros like this:

https://nxr.netbsd.org/xref/src/tests/kernel/arch/sparc/execsp.S?r=1.1#35

Maybe that's too much abstraction, but I think it's easier to read the slightly abstracted code than to juggle two whole copies of the definitions.



Home | Main Index | Thread Index | Old Index