Re: sys/ucontext.h - dead code walking?
On Sat, Apr 18, 2015 at 2:56 PM, Adam Wolk adam.w...@koparo.com wrote: On Sat, Apr 18, 2015, at 11:44 PM, Mark Kettenis wrote: From: Adam Wolk adam.w...@koparo.com Date: Sat, 18 Apr 2015 23:23:40 +0200 ... Which lead me to a hunt on how other parts of base/ports handle this. I grepped /usr/src and found something interesting. /gnu/gcc/gcc/config/pa/hpux-unwind.h This is HP-UX specific code. Yes, but there are also other code paths like: ./gnu/gcc/gcc/config/i386/linux-unwind.h:#include sys/ucontext.h It's in the base system, even if it's correct for other platforms then I don't see a reason for code that will never compile on OpenBSD to be included in OpenBSD base - unless removing it from the build system is more effort than maintaining it's presence. There's always a question with 3rd party code, such as everything under gnu/, of whether local changes should be minimized or expansive. Once the changes become too expansive, it'll effectively be a fork which requires more local resources to be spent on it going forward: look how much effort has gone into libressl. It seems in this case that the benefits of removing that code are insubstantial compared to the time that would be required (would need to verify that all the archs still build unchanged). Properly done, there would be *no* effect on the binaries, and would have only limited improvements on code comprehensibility: this isn't like other programs where we can delete piles of #ifdefs that cluster the main code, and really there's very little development being done in the gcc code itself...so why bother? Philip Guenther
Re: sys/ucontext.h - dead code walking?
On Sun, Apr 19, 2015, at 12:23 AM, Philip Guenther wrote: On Sat, Apr 18, 2015 at 2:56 PM, Adam Wolk adam.w...@koparo.com wrote: On Sat, Apr 18, 2015, at 11:44 PM, Mark Kettenis wrote: From: Adam Wolk adam.w...@koparo.com Date: Sat, 18 Apr 2015 23:23:40 +0200 ... Which lead me to a hunt on how other parts of base/ports handle this. I grepped /usr/src and found something interesting. /gnu/gcc/gcc/config/pa/hpux-unwind.h This is HP-UX specific code. Yes, but there are also other code paths like: ./gnu/gcc/gcc/config/i386/linux-unwind.h:#include sys/ucontext.h It's in the base system, even if it's correct for other platforms then I don't see a reason for code that will never compile on OpenBSD to be included in OpenBSD base - unless removing it from the build system is more effort than maintaining it's presence. There's always a question with 3rd party code, such as everything under gnu/, of whether local changes should be minimized or expansive. Once the changes become too expansive, it'll effectively be a fork which requires more local resources to be spent on it going forward: look how much effort has gone into libressl. It seems in this case that the benefits of removing that code are insubstantial compared to the time that would be required (would need to verify that all the archs still build unchanged). Properly done, there would be *no* effect on the binaries, and would have only limited improvements on code comprehensibility: this isn't like other programs where we can delete piles of #ifdefs that cluster the main code, and really there's very little development being done in the gcc code itself...so why bother? Philip Guenther Understood. Thank you for the explanation. Regards, Adam
sys/ucontext.h - dead code walking?
Hi tech@, I'm working on a port for lang/dart and got stuck on ucontext.h compile errors. The first one was quite easy, changing sys/ucontext.h to signal.h since ucontext_t is defined there sys/signal.h: typedef struct sigcontext ucontext_t; This allowed me to move forward and stop on the next bit: In file included from runtime/vm/thread_interrupter.h:9:0, from runtime/vm/profiler.h:13, from runtime/vm/dart.ca c:22: runtime/vm/signal_handler.h:49:44: error: 'mcontext_t' does not name a type static uintptr_t GetProgramCounter(const mcontext_t mcontext); ^ runtime/vm/signal_handler.h:50:42: error: 'mcontext_t' does not name a type Which lead me to a hunt on how other parts of base/ports handle this. I grepped /usr/src and found something interesting. /gnu/gcc/gcc/config/pa/hpux-unwind.h which contains a: #include sys/ucontext.h Now taking this example C program: $ cat dead.c #include sys/ucontext.h int main(int argc, char *argv[]) { return 0; } $ and trying to compile it: $ make dead cc -O2 -pipe-o dead dead.c dead.c:1:26: error: sys/ucontext.h: No such file or directory *** Error 1 in /home/mulander/code/c (sys.mk:85 'dead') We can see that sys/ucontext.h is not present. Hence the hpux-unwind.h header file must be dead code. Grepping src I found some more occurrences, all in base gcc (minus some changelog/comment entries). Should header files including sys/ucontext.h be removed along with their paired .c files? ./gnu/gcc/fixincludes/ChangeLog:* tests/base/sys/ucontext.h: New file. ./gnu/gcc/fixincludes/fixincl.x: |sys/ucontext.h|; ./gnu/gcc/fixincludes/inclhack.def:/* The /usr/include/sys/ucontext.h on ia64-*linux-gnu systems defines ./gnu/gcc/fixincludes/inclhack.def:files = sys/ucontext.h; ./gnu/gcc/fixincludes/tests/base/sys/ucontext.h: fixinc/tests/inc/sys/ucontext.h ./gnu/gcc/gcc/config/alpha/linux-unwind.h:#include sys/ucontext.h ./gnu/gcc/gcc/config/i386/linux-unwind.h:#include sys/ucontext.h ./gnu/gcc/gcc/config/i386/linux-unwind.h:/* There's no sys/ucontext.h for glibc 2.0, so no ./gnu/gcc/gcc/config/i386/linux-unwind.h:#include sys/ucontext.h ./gnu/gcc/gcc/config/ia64/linux-unwind.h:#include sys/ucontext.h ./gnu/gcc/gcc/config/mips/linux-unwind.h: * struct ucontext from sys/ucontext.h so this private copy is used. */ ./gnu/gcc/gcc/config/pa/hpux-unwind.h:#include sys/ucontext.h ./gnu/gcc/gcc/config/pa/linux-unwind.h:#include sys/ucontext.h ./gnu/gcc/gcc/config/rs6000/host-darwin.c:#include sys/ucontext.h ./gnu/gcc/gcc/config/sh/linux-unwind.h:#include sys/ucontext.h ./gnu/usr.bin/binutils/gdb/s390-nat.c:#include sys/ucontext.h ./gnu/usr.bin/binutils/gdb/sparc-nat.c: fp_status' in sys/ucontext.h, which is automatically included by ./gnu/usr.bin/binutils/gdb/user-regs.c: sys/ucontext.h includes sys/procfs.h includes sys/user.h, which ./gnu/usr.bin/binutils/gdb/osf-share/cma_tcb_defs.h:# include sys/ucontext.h ./gnu/usr.bin/gcc/gcc/ChangeLog:sys/ucontext.h inclusion in ifndef USE_GNULIBC_1. ./gnu/usr.bin/gcc/gcc/ChangeLog.7: including signal.h and sys/ucontext.h, not needed. ./gnu/usr.bin/gcc/gcc/ChangeLog.7: to avoid clash with Irix header file sys/ucontext.h. Rename gp_regs ./gnu/usr.bin/gcc/gcc/config/alpha/linux.h:#include sys/ucontext.h ./gnu/usr.bin/gcc/gcc/config/i386/linux.h:/* There's no sys/ucontext.h for some (all?) libc1, so no ./gnu/usr.bin/gcc/gcc/config/i386/linux.h:#include sys/ucontext.h ./gnu/usr.bin/gcc/gcc/config/i386/linux64.h:#include sys/ucontext.h ./gnu/usr.bin/gcc/gcc/config/ia64/linux.h:#include sys/ucontext.h PS. I would greatly appreciate If anyone pointed me at a file that still defines mcontext_t or an acceptable workaround :) Regards, -- Adam Wolk adam.w...@koparo.com
Re: sys/ucontext.h - dead code walking?
From: Adam Wolk adam.w...@koparo.com Date: Sat, 18 Apr 2015 23:23:40 +0200 Hi tech@, I'm working on a port for lang/dart and got stuck on ucontext.h compile errors. The first one was quite easy, changing sys/ucontext.h to signal.h since ucontext_t is defined there sys/signal.h: typedef struct sigcontext ucontext_t; It is questionable whether ucontext_t should be defined there. The sys/ucontext.h header has been removed from POSIX, but POSIX still refers to ucontext_t in its signal handler description. In the end the reason sys/ucontext.h has been removed from POSIX is because it is impossible to use its functionality in a portable fashion. It is inherently architecture dependent, and effectively OS dependent as well. This allowed me to move forward and stop on the next bit: In file included from runtime/vm/thread_interrupter.h:9:0, from runtime/vm/profiler.h:13, from runtime/vm/dart.ca c:22: runtime/vm/signal_handler.h:49:44: error: 'mcontext_t' does not name a type static uintptr_t GetProgramCounter(const mcontext_t mcontext); ^ runtime/vm/signal_handler.h:50:42: error: 'mcontext_t' does not name a type If this bit of code is indeed essential,you'll have to write an OpenBSD-specific version of it. My advise would be to stick to using struct sigcontext. Change GetProgramCounter to take const struct sigcontext sc as an argument, and make it return sc.sc_pc; That would make it work on alpha/i386/sparc/sparc64/vax and we can add the appropriate define on other architectures. For amd64 that would be #define sc_pc sc_rip If you need more help, please post the relevant code or provide an url with (preferabley browsable) source code. Which lead me to a hunt on how other parts of base/ports handle this. I grepped /usr/src and found something interesting. /gnu/gcc/gcc/config/pa/hpux-unwind.h This is HP-UX specific code.
Re: sys/ucontext.h - dead code walking?
On Sat, Apr 18, 2015, at 11:44 PM, Mark Kettenis wrote: From: Adam Wolk adam.w...@koparo.com Date: Sat, 18 Apr 2015 23:23:40 +0200 Hi tech@, I'm working on a port for lang/dart and got stuck on ucontext.h compile errors. The first one was quite easy, changing sys/ucontext.h to signal.h since ucontext_t is defined there sys/signal.h: typedef struct sigcontext ucontext_t; It is questionable whether ucontext_t should be defined there. The sys/ucontext.h header has been removed from POSIX, but POSIX still refers to ucontext_t in its signal handler description. In the end the reason sys/ucontext.h has been removed from POSIX is because it is impossible to use its functionality in a portable fashion. It is inherently architecture dependent, and effectively OS dependent as well. This allowed me to move forward and stop on the next bit: In file included from runtime/vm/thread_interrupter.h:9:0, from runtime/vm/profiler.h:13, from runtime/vm/dart.ca c:22: runtime/vm/signal_handler.h:49:44: error: 'mcontext_t' does not name a type static uintptr_t GetProgramCounter(const mcontext_t mcontext); ^ runtime/vm/signal_handler.h:50:42: error: 'mcontext_t' does not name a type If this bit of code is indeed essential,you'll have to write an OpenBSD-specific version of it. My advise would be to stick to using struct sigcontext. Change GetProgramCounter to take const struct sigcontext sc as an argument, and make it return sc.sc_pc; That would make it work on alpha/i386/sparc/sparc64/vax and we can add the appropriate define on other architectures. For amd64 that would be #define sc_pc sc_rip If you need more help, please post the relevant code or provide an url with (preferabley browsable) source code. The browsable code can be seen on github: - https://github.com/dart-lang/bleeding_edge/blob/master/dart/runtime/vm/signal_handler.h It seems that the android path defines: typedef struct sigcontext mcontext_t; which matches your recommendation and has a high chance of the whole port going forward. I'll try adding it in the OpenBSD build path for the port. Thank you for the hint, you probably unblocked my progress on the port Which lead me to a hunt on how other parts of base/ports handle this. I grepped /usr/src and found something interesting. /gnu/gcc/gcc/config/pa/hpux-unwind.h This is HP-UX specific code. Yes, but there are also other code paths like: ./gnu/gcc/gcc/config/i386/linux-unwind.h:#include sys/ucontext.h It's in the base system, even if it's correct for other platforms then I don't see a reason for code that will never compile on OpenBSD to be included in OpenBSD base - unless removing it from the build system is more effort than maintaining it's presence. I'm not saying it's bad - just wanted to point out that I stumbled upon it. Regards, Adam