Re: sys/ucontext.h - dead code walking?

2015-04-18 Thread Philip Guenther
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?

2015-04-18 Thread Adam Wolk
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?

2015-04-18 Thread Adam Wolk
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?

2015-04-18 Thread Mark Kettenis
 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?

2015-04-18 Thread Adam Wolk
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