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 wrote: > > On Sat, Apr 18, 2015, at 11:44 PM, Mark Kettenis wrote: > >> > From: Adam Wolk > >> > 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 > > > > 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
Re: sys/ucontext.h - dead code walking?
On Sat, Apr 18, 2015 at 2:56 PM, Adam Wolk wrote: > On Sat, Apr 18, 2015, at 11:44 PM, Mark Kettenis wrote: >> > From: Adam Wolk >> > 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 > > 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 Sat, Apr 18, 2015, at 11:44 PM, Mark Kettenis wrote: > > From: Adam Wolk > > 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 > header has been removed from POSIX, but POSIX still > refers to ucontext_t in its signal handler description. > > In the end the reason 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 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
Re: sys/ucontext.h - dead code walking?
> From: Adam Wolk > 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 header has been removed from POSIX, but POSIX still refers to ucontext_t in its signal handler description. In the end the reason 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.
