Re: struct user conflicts on arm
On 17 December 2011 09:17, peter green peter.gr...@postgrad.manchester.ac.uk wrote: While we are talking about modifying sys/ucontext.h David Given raised another issue with that header (for those reading on the linaro list his post can be found at http://lists.debian.org/debian-arm/2011/12/msg00048.html) David GivenThis might be a good time to mention that on ARM, sys/ucontext.h defines David Givenboth symbols and preprocessor macros called R0, R1, R2, etc in the David Givenglobal namespace; this is causing one of my packages to fail to build David Givendue to symbol collision. David Given David GivenI'm fixing the package, but naming symbols stuff like that is still David Givenpretty antisocial... Does anyone know what the impact of renaming these to use a REG_ prefix like i386, amd64 and sparc do* would be? at worst the packages that had to be workaround on arm* for this, can have the workaround removed. Since you're in the process of fixing things for glibc/arm, would you mind also looking at WCHAR_MIN/MAX undefined for arm? http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=598937 Thanks Konstantinos -- To UNSUBSCRIBE, email to debian-glibc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/cabsevwuj2s_yfmvkgb_tvbhdhqg94tyryxeyvzlwc05g9tc...@mail.gmail.com
Re: struct user conflicts on arm
Konstantinos Margaritis wrote: Does anyone know what the impact of renaming these to use a REG_ prefix like i386, amd64 and sparc do* would be? at worst the packages that had to be workaround on arm* for this, can have the workaround removed. I have prepared a patch (attatched) that eliminates the dependency on sys/procfs.h and renames R? to REG_R?. I have tested it by modifying the header locally. I am now attempting to rebuild glibc with the patch. After rebuilding glibc I will install it and attempt to rebuild GDB. Please comment on the patch and advise on the best route for submission (that is should I go through debian, through eglibc or direct to glibc?) Since you're in the process of fixing things for glibc/arm Note that I am not a glibc developer nor am I a dd (and even if I was a dd I don't think I would NMU glibc). I'm just a user with an interest in the future of debian on arm. would you mind also looking at WCHAR_MIN/MAX undefined for arm? http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=598937 They most certainly are defined. The trouble is that WCHAR_MAX is defined in a strange way. #define __WCHAR_MAX ( (wchar_t) - 1 ) This definition is fine for normal c or c++ code but it cannot be properly evaluated in the context of a preprocessor conditional. The bug report has a patch (actually a replacement for an existing patch) which looks fine to me. Index: eglibc-2.13/ports/sysdeps/unix/sysv/linux/arm/sys/ucontext.h === --- eglibc-2.13.orig/ports/sysdeps/unix/sysv/linux/arm/sys/ucontext.h 2006-08-17 01:23:58.0 + +++ eglibc-2.13/ports/sysdeps/unix/sysv/linux/arm/sys/ucontext.h 2011-12-17 12:52:43.0 + @@ -23,7 +23,6 @@ #include features.h #include signal.h -#include sys/procfs.h /* We need the signal context definitions even if they are not used included in signal.h. */ @@ -35,47 +34,64 @@ #define NGREG 18 /* Container for all general registers. */ -typedef elf_gregset_t gregset_t; +typedef greg_t gregset_t[NGREG]; /* Number of each register is the `gregset_t' array. */ enum { - R0 = 0, -#define R0 R0 - R1 = 1, -#define R1 R1 - R2 = 2, -#define R2 R2 - R3 = 3, -#define R3 R3 - R4 = 4, -#define R4 R4 - R5 = 5, -#define R5 R5 - R6 = 6, -#define R6 R6 - R7 = 7, -#define R7 R7 - R8 = 8, -#define R8 R8 - R9 = 9, -#define R9 R9 - R10 = 10, -#define R10 R10 - R11 = 11, -#define R11 R11 - R12 = 12, -#define R12 R12 - R13 = 13, -#define R13 R13 - R14 = 14, -#define R14 R14 - R15 = 15 -#define R15 R15 + REG_R0 = 0, +#define REG_R0 REG_R0 + REG_R1 = 1, +#define REG_R1 REG_R1 + REG_R2 = 2, +#define REG_R2 REG_R2 + REG_R3 = 3, +#define REG_R3 REG_R3 + REG_R4 = 4, +#define REG_R4 REG_R4 + REG_R5 = 5, +#define REG_R5 REG_R5 + REG_R6 = 6, +#define REG_R6 REG_R6 + REG_R7 = 7, +#define REG_R7 REG_R7 + REG_R8 = 8, +#define REG_R8 REG_R8 + REG_R9 = 9, +#define REG_R9 REG_R9 + REG_R10 = 10, +#define REG_R10 REG_R10 + REG_R11 = 11, +#define REG_R11 REG_R11 + REG_R12 = 12, +#define REG_R12 REG_R12 + REG_R13 = 13, +#define REG_R13 REG_R13 + REG_R14 = 14, +#define REG_R14 REG_R14 + REG_R15 = 15 +#define REG_R15 REG_R15 }; +struct _libc_fpstate +{ + struct + { +unsigned int sign1:1; +unsigned int unused:15; +unsigned int sign2:1; +unsigned int exponent:14; +unsigned int j:1; +unsigned int mantissa1:31; +unsigned int mantissa0:32; + } fpregs[8]; + unsigned int fpsr:32; + unsigned int fpcr:32; + unsigned char ftype[8]; + unsigned int init_flag; +}; /* Structure to describe FPU registers. */ -typedef elf_fpregset_t fpregset_t; +typedef struct _libc_fpstate fpregset_t; /* Context to describe whole processor state. This only describes the core registers; coprocessor registers get saved elsewhere
Re: struct user conflicts on arm
On Sat, Dec 17, 2011 at 7:57 AM, peter green plugw...@p10link.net wrote: mind also looking at WCHAR_MIN/MAX undefined for arm? http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=598937 They most certainly are defined. The trouble is that WCHAR_MAX is defined in a strange way. #define __WCHAR_MAX ( (wchar_t) - 1 ) This definition is fine for normal c or c++ code but it cannot be properly evaluated in the context of a preprocessor conditional. The bug report has a patch (actually a replacement for an existing patch) which looks fine to me. ISO C99 says that WCHAR_MAX must be a constant expression and the above definition is such an expression. Technically the program needs fixing (see below though for the standards matter but so do users), there is nothing wrong with a type cast and a constant value e.g. signed -1 converted to unsigned int (ARM GNU/Linux value for wchar_t). However, the real issue here is that it differs from x86, the most common architecture, and differences from x86 cause porting problems. The patch itself is insufficient because it doesn't take into account wordsize. When we switch to the 64-bit ARM ABI it should just work. Therefore you need to check for __WORDSIZE and *only* define a value if we are *not* 64-bits. You don't want to define anything for the 64-bit case until the 64-bit ARM ABI is out and finalized. Your patch to fix ucontext namespace pollution looks good, please post that to libc-ports for review and make sure to state what testing you've done with the patch. At a minimum you should run the glibc testsuite and build gdb with those newly installed headers. Cheers, Carlos. -- To UNSUBSCRIBE, email to debian-glibc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/CADZpyiys+=ldp6x1tnycqh+cwo-ixajsr5dhu-vmn_b13hg...@mail.gmail.com
Re: struct user conflicts on arm
ISO C99 says that WCHAR_MAX must be a constant expression and the above definition is such an expression. Technically the program needs fixing (see below though for the standards matter but so do users), there is nothing wrong with a type cast and a constant value e.g. signed -1 converted to unsigned int (ARM GNU/Linux value for wchar_t). However, the real issue here is that it differs from x86, the most common architecture, and differences from x86 cause porting problems. The patch itself is insufficient because it doesn't take into account wordsize. When we switch to the 64-bit ARM ABI it should just work. Therefore you need to check for __WORDSIZE and *only* define a value if we are *not* 64-bits. You don't want to define anything for the 64-bit case until the 64-bit ARM ABI is out and finalized. Thanks for the info, I may look at this later. The ucontext namespace pollution seems to be a bigger issue though. Your patch to fix ucontext namespace pollution looks good, please post that to libc-ports for review should I send it immidiately or should I wait until I have test results to give them? and make sure to state what testing you've done with the patch. At a minimum you should run the glibc testsuite Afaict the debian packaging automatically runs the testsuite and compares it against a list of expected failures (ideally that list would be empty but in real life). Right now i'm running into unexpected testsuite failures (unfortunately the last test build I didn't take a log of so i've got to run it again to find out details of the failures) but I do not know if those failures are related to my patch, related to changes in the build environment since the package was last built in debian or related to my hardware. Further testing will be neeed to establish that (and said testing takes a while, a beagleboard xm isn't exactly a speed demon). and build gdb with those newly installed headers. Will do once I get glibc built and installed, are there any specific tests you want doing with gdb or is testing it still builds sufficient?? Cheers, Carlos. Thanks for the help and advice so-far. -- To UNSUBSCRIBE, email to debian-glibc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/4eed39e0.30...@p10link.net
Re: struct user conflicts on arm
Ulrich WeigandNow, glibc also provides a file sys/ucontext.h that defines Ulrich Weigandlayouts of register sets for use with signal handlers as well Ulrich Weigandas the makecontext/setcontext/getcontext family of routines. Ulrich Weigand Ulrich WeigandUsually, those layouts tend to be in fact identical to the Ulrich Weigandlayouts described in sys/user.h / sys/procfs.h. Apparently, Ulrich Weigandwhoever implemented the ARM version of sys/ucontext.h was Ulrich Weigandtrying to avoid some seemingly unnecessary code duplication Ulrich Weigandby making sys/ucontext.h *include* sys/procfs.h and using Ulrich Weigandthe information from there directly. This is not done on other Ulrich Weigandplatforms, for precisely the reason that the sys/procfs.h Ulrich Weigandand sys/user.h headers do pollute the name space ... Ulrich Weigand Ulrich WeigandSo I think the right thing to do in the short term would be to Ulrich Weigandstop sys/ucontext.h including sys/procfs.h, and instead add the Ulrich Weigandregister set information there directly, even if that means some Ulrich Weigandduplication of code. (Again, since this is never-changing ABI, Ulrich Weigandduplication isn't actually all that bad. Also, all the other Ulrich Weigandplatforms do it that way too, so why should ARM be different ...) Makes sense to me While we are talking about modifying sys/ucontext.h David Given raised another issue with that header (for those reading on the linaro list his post can be found at http://lists.debian.org/debian-arm/2011/12/msg00048.html) David GivenThis might be a good time to mention that on ARM, sys/ucontext.h defines David Givenboth symbols and preprocessor macros called R0, R1, R2, etc in the David Givenglobal namespace; this is causing one of my packages to fail to build David Givendue to symbol collision. David Given David GivenI'm fixing the package, but naming symbols stuff like that is still David Givenpretty antisocial... Does anyone know what the impact of renaming these to use a REG_ prefix like i386, amd64 and sparc do* would be? * ia64,mips, mipsel, powerpc, 390 and s390x don't seem to define such symbols at all. -- To UNSUBSCRIBE, email to debian-glibc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/4eec420f.8010...@postgrad.manchester.ac.uk
Re: struct user conflicts on arm
This subject came up on debian-arm: http://lists.debian.org/debian-arm/2011/12/msg00025.html And it seems like the sort of architectural issue linaro should take an interest in fixing to avoid making people's lives difficult when building on arm. Is it on anyone's list already? In short: A C app that defines a struct 'user' and uses wait.h or signal.h will find a clash with the system struct of the same name, which should only be needed for GDB (when built on/for arm). On other arches this is not pulled in by default so the problem doesn't arise. We can 1) Change every app in the world that defines a 'struct user' 2) Stop these headers getting brought in when not actually needed (it's a relatively recent change that brings it in) 3) Change the name in glibc/GDB to something less likely to clash disclaimer: I know nothing at all about this except what I just read in that thread. Please refer to the thread for details. Wookey -- Principal hats: Linaro, Emdebian, Wookware, Balloonboard, ARM http://wookware.org/ -- To UNSUBSCRIBE, email to debian-glibc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20111215110917.gc28...@dream.aleph1.co.uk
Re: struct user conflicts on arm
Wookey wrote: We can 1) Change every app in the world that defines a 'struct user' 2) Stop these headers getting brought in when not actually needed (it's a relatively recent change that brings it in) 3) Change the name in glibc/GDB to something less likely to clash Some background from a GDB perspective: the files sys/procfs.h and sys/user.h define the layout of the various register sets the kernel provides to user space debuggers via ptrace and/or core file register note sections. In the past, it was considered a good idea for this information to be provided by kernel headers via inclusion into glibc to be used by GDB sources. However, this turned out to not be really feasible with modern GDB: since this information is needed to access core files, and with a cross-debugger we need to access core files produced on some other architecture than the host, we cannot actually rely on system headers (which only provide information about the host architecture). Fortunately, the layout is part of the unchanging kernel ABI, so there is no real need for GDB to rely on any header files in the first place. Therefore, GDB is currently in the process to switching to simply hard-coding register set layouts for various architectures. Once this process is complete, sys/user.h and sys/procfs.h will no longer be needed to build GDB. (The process is mostly complete for sys/user.h; it will still take a while for sys/procfs.h.) However, from a system perspective, it is probably still not a good idea to just remove (or incompatibly change) those headers: you want to keep the capability to compile older GDB sources (and possibly other debuggers). Now, glibc also provides a file sys/ucontext.h that defines layouts of register sets for use with signal handlers as well as the makecontext/setcontext/getcontext family of routines. Usually, those layouts tend to be in fact identical to the layouts described in sys/user.h / sys/procfs.h. Apparently, whoever implemented the ARM version of sys/ucontext.h was trying to avoid some seemingly unnecessary code duplication by making sys/ucontext.h *include* sys/procfs.h and using the information from there directly. This is not done on other platforms, for precisely the reason that the sys/procfs.h and sys/user.h headers do pollute the name space ... So I think the right thing to do in the short term would be to stop sys/ucontext.h including sys/procfs.h, and instead add the register set information there directly, even if that means some duplication of code. (Again, since this is never-changing ABI, duplication isn't actually all that bad. Also, all the other platforms do it that way too, so why should ARM be different ...) Mit freundlichen Gruessen / Best Regards Ulrich Weigand -- Dr. Ulrich Weigand | Phone: +49-7031/16-3727 STSM, GNU compiler and toolchain for Linux on System z and Cell/B.E. IBM Deutschland Research Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter | Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht Stuttgart, HRB 243294 -- To UNSUBSCRIBE, email to debian-glibc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/of944a0ed5.1ea1377b-onc1257967.00636680-c1257967.0066b...@de.ibm.com