Issue 7221080: code review 7221080: runtime: Avoid dependency on the auxillary vector, auxv... (Closed)
Descriptionruntime: Add support for loading auxv from /proc/self/auxv.
If support for creating shared libraries in go is implemented, the kernel
supplied auxv supplied on the main thread of an executable will no longer be
available. In that case, load the necessary information from /proc/self/auxv.
Patch Set 1 #Patch Set 2 : diff -r b3af92ac5a0c https://go.googlecode.com/hg/ #Patch Set 3 : diff -r b3af92ac5a0c https://go.googlecode.com/hg/ #
Total comments: 6
Patch Set 4 : diff -r a9211b512258 https://go.googlecode.com/hg/ #Patch Set 5 : diff -r a9211b512258 https://go.googlecode.com/hg/ #Patch Set 6 : diff -r 3836bcbafa69 https://go.googlecode.com/hg/ #
MessagesTotal messages: 19
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg/
Sign in to reply to this message.
Sorry if I missed something, but when is this useful?
Sign in to reply to this message.
I think the refactoring of the auxv parsing is good, but does this introduce a requirement that /proc is mounted? I believe this is a new requirement. What problem does it solve ? On Fri, Feb 1, 2013 at 10:15 AM, <iant@golang.org> wrote: > Sorry if I missed something, but when is this useful? > > https://codereview.appspot.com/7221080/ > > > -- > > ---You received this message because you are subscribed to the Google Groups > "golang-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-dev+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/groups/opt_out. > >
Sign in to reply to this message.
The context here is that this is trying to find the auxv when called as a Go .so loaded into a non-Go (let's say C) program that has already started. Is there really no way to find the auxv without using /proc? Can the C side find it and pass it in? Russ
Sign in to reply to this message.
On 2013/01/31 23:45:06, rsc wrote: > The context here is that this is trying to find the auxv when called as a > Go .so loaded into a non-Go (let's say C) program that has already started. > > Is there really no way to find the auxv without using /proc? Can the C side > find it and pass it in? > > The best I could find regarding auxv without /proc was http://sourceware.org/ml/gdb-patches/2010-01/msg00233.html It's a slightly different context (gdb wanting auxv from an inferior process), but the method seems a tad convoluted and brittle, namely finding the _dl_auxv symbol in ld.so. Auxv is located in a known position relative to the main thread stack, but as far as I can tell, that stack is gone if and when the main thread pthread_exit()s before dlopen()ing our go implemented library. From pthread_exit(3): "The value pointed to by retval should not be located on the calling thread's stack, since the contents of that stack are undefined after the thread terminates." An alternative solution is to stop relying on auxv altogether, which currently means finding a replacement for the auxv provided AT_RANDOM seed (used on arm currently). ARM also uses AT_PLATFORM and AT_HWCAPS, but I guess they can be inferred with assembly. The vdso library used by x64 can be linked in by the dynamic linker, as long as we're not building a static executable.
Sign in to reply to this message.
Looks basically fine. https://codereview.appspot.com/7221080/diff/3005/src/pkg/runtime/signal_linux... File src/pkg/runtime/signal_linux_arm.c (right): https://codereview.appspot.com/7221080/diff/3005/src/pkg/runtime/signal_linux... src/pkg/runtime/signal_linux_arm.c:174: runtime·parse_auxv_entry(uint32 key, uint32 val) { You don't need the runtime· prefix for a static function. https://codereview.appspot.com/7221080/diff/3005/src/pkg/runtime/signal_linux... src/pkg/runtime/signal_linux_arm.c:200: runtime·setup_auxv_from_proc(void) Don't need runtime· prefix. https://codereview.appspot.com/7221080/diff/3005/src/pkg/runtime/signal_linux... src/pkg/runtime/signal_linux_arm.c:215: runtime·setup_auxv_from_argv(int32 argc, byte **argv) Don't need runtime· prefix. https://codereview.appspot.com/7221080/diff/3005/src/pkg/runtime/vdso_linux_a... File src/pkg/runtime/vdso_linux_amd64.c (right): https://codereview.appspot.com/7221080/diff/3005/src/pkg/runtime/vdso_linux_a... src/pkg/runtime/vdso_linux_amd64.c:304: runtime·atolhex(byte *s, uint32 count) Don't need runtime· prefix. https://codereview.appspot.com/7221080/diff/3005/src/pkg/runtime/vdso_linux_a... src/pkg/runtime/vdso_linux_amd64.c:325: runtime·linux_setup_vdso_from_argv(int32 argc, uint8** argv) Don't need runtime· prefix. https://codereview.appspot.com/7221080/diff/3005/src/pkg/runtime/vdso_linux_a... src/pkg/runtime/vdso_linux_amd64.c:349: runtime·linux_setup_vdso_from_proc(void) Don't need runtime· prefix.
Sign in to reply to this message.
Now that we have code to read /proc/self/auxv can we drop the code to read /proc/self/maps?
Sign in to reply to this message.
PTAL On 2013/02/05 15:11:31, iant wrote: > Looks basically fine. > > https://codereview.appspot.com/7221080/diff/3005/src/pkg/runtime/signal_linux... > File src/pkg/runtime/signal_linux_arm.c (right): > > https://codereview.appspot.com/7221080/diff/3005/src/pkg/runtime/signal_linux... > src/pkg/runtime/signal_linux_arm.c:174: runtime·parse_auxv_entry(uint32 key, > uint32 val) { > You don't need the runtime· prefix for a static function. > done. > https://codereview.appspot.com/7221080/diff/3005/src/pkg/runtime/signal_linux... > src/pkg/runtime/signal_linux_arm.c:200: runtime·setup_auxv_from_proc(void) > Don't need runtime· prefix. > > done. >https://codereview.appspot.com/7221080/diff/3005/src/pkg/runtime/signal_linux_arm.c#newcode215 > src/pkg/runtime/signal_linux_arm.c:215: runtime·setup_auxv_from_argv(int32 argc, > byte **argv) > Don't need runtime· prefix. > done. > https://codereview.appspot.com/7221080/diff/3005/src/pkg/runtime/vdso_linux_a... > File src/pkg/runtime/vdso_linux_amd64.c (right): > > https://codereview.appspot.com/7221080/diff/3005/src/pkg/runtime/vdso_linux_a... > src/pkg/runtime/vdso_linux_amd64.c:304: runtime·atolhex(byte *s, uint32 count) > Don't need runtime· prefix. > done. > https://codereview.appspot.com/7221080/diff/3005/src/pkg/runtime/vdso_linux_a... > src/pkg/runtime/vdso_linux_amd64.c:325: runtime·linux_setup_vdso_from_argv(int32 > argc, uint8** argv) > Don't need runtime· prefix. > done. > https://codereview.appspot.com/7221080/diff/3005/src/pkg/runtime/vdso_linux_a... > src/pkg/runtime/vdso_linux_amd64.c:349: runtime·linux_setup_vdso_from_proc(void) > Don't need runtime· prefix. done. Thanks!
Sign in to reply to this message.
PTAL On 2013/02/05 15:46:31, rsc wrote: > Now that we have code to read /proc/self/auxv can we drop the code to read > /proc/self/maps? Done. According to proc(5), /proc/self/auxv is available from kernel version 2.6.0, while /proc/self/maps is presumably much older (in posix?). But since /proc/self/auxv is needed on linux/arm, it's much easier to load the VDSO address from /proc/self/auxv also.
Sign in to reply to this message.
I don't know the exact kernel version require but I am confident that >= 2.6.0 is an underestimate. So that seems fine. Russ
Sign in to reply to this message.
On Fri, Feb 1, 2013 at 5:16 PM, <elias.naur@gmail.com> wrote: > > An alternative solution is to stop relying on auxv altogether, which > currently means finding a replacement for the auxv provided AT_RANDOM > seed (used on arm currently). ARM also uses AT_PLATFORM and AT_HWCAPS, > but I guess they can be inferred with assembly. > The vdso library used by x64 can be linked in by the dynamic linker, as > long as we're not building a static executable. > i can eliminate the dependency on AT_PLATFORM and AT_HWCAPS now as i just discovered a kernel that's not providing the info correctly anyway. On arm, we can't get the required info in user space, but we can catch SIGILL generated from unsupported instructions, and that will make the goarm checking code a little portable. Regarding AT_RANDOM, this is unfortunate as we don't use it, we have to read random bytes from /dev/urandom at program startup; or we don't rely on random IVs for hashing.
Sign in to reply to this message.
On 2013/02/05 16:46:18, minux wrote: > On Fri, Feb 1, 2013 at 5:16 PM, <mailto:elias.naur@gmail.com> wrote: > > > > An alternative solution is to stop relying on auxv altogether, which > > currently means finding a replacement for the auxv provided AT_RANDOM > > seed (used on arm currently). ARM also uses AT_PLATFORM and AT_HWCAPS, > > but I guess they can be inferred with assembly. > > The vdso library used by x64 can be linked in by the dynamic linker, as > > long as we're not building a static executable. > > > i can eliminate the dependency on AT_PLATFORM and AT_HWCAPS now > as i just discovered a kernel that's not providing the info correctly > anyway. > On arm, we can't get the required info in user space, but we can catch > SIGILL > generated from unsupported instructions, and that will make the goarm > checking > code a little portable. > > Regarding AT_RANDOM, this is unfortunate as we don't use it, we have to > read random bytes from /dev/urandom at program startup; or we don't rely > on random IVs for hashing. Is there a reason AT_RANDOM is only used for linux/arm? I can see that AT_RANDOM seeds cputicks which in turn seeds fastrand1, but on amd64 and i386, the high precision RDTSC instruction is used for cputicks instead. Is RDTSC simply random enough to use as hash IV? And if so, is there a n equivalent feature on ARM to replace AT_RANDOM?
Sign in to reply to this message.
On Wed, Feb 6, 2013 at 4:03 AM, <elias.naur@gmail.com> wrote: > On 2013/02/05 16:46:18, minux wrote: > Regarding AT_RANDOM, this is unfortunate as we don't use it, we have > to > >> read random bytes from /dev/urandom at program startup; or we don't >> > rely > >> on random IVs for hashing. >> > > Is there a reason AT_RANDOM is only used for linux/arm? > I can see that AT_RANDOM seeds cputicks which in turn seeds fastrand1, > but on amd64 and i386, the high precision RDTSC instruction is used for > cputicks instead. Is RDTSC simply random enough to use as hash IV? And > i think it's because the cycle counter is increasing so fast that it's practically impossible to predict and thus be regarded as random enough. After Go 1.1, maybe we can switch to a collision-resistant hashing algorithm (e.g. siphash24) for map hashing and don't rely on random IV any more. > if so, is there a n equivalent feature on ARM to replace AT_RANDOM? > the sad truth is: no. although a lot of ARM cores do have cycle counters, normally they are not exposed to user space (even if they did, we still have problems for cpus that lack this features).
Sign in to reply to this message.
On 2013/02/05 16:46:18, minux wrote: > i can eliminate the dependency on AT_PLATFORM and AT_HWCAPS now > as i just discovered a kernel that's not providing the info correctly > anyway. Should we hold off on this CL until you do that?
Sign in to reply to this message.
Could I ask that this set of CLs is deferred til after Go 1.1. This plumbing sounds deep and invasive. On Wed, Feb 6, 2013 at 8:58 AM, <iant@golang.org> wrote: > On 2013/02/05 16:46:18, minux wrote: > >> i can eliminate the dependency on AT_PLATFORM and AT_HWCAPS now >> as i just discovered a kernel that's not providing the info correctly >> anyway. > > > Should we hold off on this CL until you do that? > > https://codereview.appspot.com/7221080/
Sign in to reply to this message.
On 2013/02/05 22:09:35, dfc wrote: > Could I ask that this set of CLs is deferred til after Go 1.1. This > plumbing sounds deep and invasive. I think this CL is fairly safe. The auxv parsing is already there. And this CL won't use the new code ordinarily. Ian
Sign in to reply to this message.
On Wed, Feb 6, 2013 at 5:58 AM, <iant@golang.org> wrote: > On 2013/02/05 16:46:18, minux wrote: > > i can eliminate the dependency on AT_PLATFORM and AT_HWCAPS now >> as i just discovered a kernel that's not providing the info correctly >> anyway. >> > > Should we hold off on this CL until you do that? > Once I get the recent signal handling and backtracer changes done, i will prepare CL for that.
Sign in to reply to this message.
R=iant
Sign in to reply to this message.
Message was sent while issue was closed.
Closed, superseded by https://codereview.appspot.com/9738047/
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
