From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 111259 invoked by alias); 6 Jul 2016 12:29:44 -0000 Mailing-List: contact systemtap-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: systemtap-owner@sourceware.org Received: (qmail 108586 invoked by uid 89); 6 Jul 2016 12:29:43 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=HTo:U*jistone, 3.4, Stone, enviroment X-HELO: unimail.uni-dortmund.de Received: from mx1.HRZ.tu-dortmund.de (HELO unimail.uni-dortmund.de) (129.217.128.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 06 Jul 2016 12:29:32 +0000 Received: from [192.168.178.104] (x5d83fa37.dyn.telefonica.de [93.131.250.55]) (authenticated bits=0) by unimail.uni-dortmund.de (8.16.0.16/8.16.0.16) with ESMTPSA id u66CTNci007293 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Wed, 6 Jul 2016 14:29:24 +0200 (CEST) Subject: Re: SystemTap for Android - patchset To: Josh Stone , systemtap@sourceware.org References: <56e0c7f4-d317-f76b-5156-3569a6097b62@tu-dortmund.de> From: Alexander Lochmann Message-ID: <577CF9A2.7050008@tu-dortmund.de> Date: Wed, 06 Jul 2016 12:29:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="JoGO19ow0JrglvFOhL6ofwWvOkcB81bLE" X-IsSubscribed: yes X-SW-Source: 2016-q3/txt/msg00008.txt.bz2 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --JoGO19ow0JrglvFOhL6ofwWvOkcB81bLE Content-Type: multipart/mixed; boundary="vKn3HbtTLKQTe6i0OM0EUrjW4dT2VAG9j" From: Alexander Lochmann To: Josh Stone , systemtap@sourceware.org Message-ID: <577CF9A2.7050008@tu-dortmund.de> Subject: Re: SystemTap for Android - patchset References: <56e0c7f4-d317-f76b-5156-3569a6097b62@tu-dortmund.de> In-Reply-To: --vKn3HbtTLKQTe6i0OM0EUrjW4dT2VAG9j Content-Type: multipart/mixed; boundary="------------050109060007060706040205" This is a multi-part message in MIME format. --------------050109060007060706040205 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Content-length: 4120 So. Let me start. First of all, I extracted the patches properly, and attached the files. (Btw, I found a third bug. :) ) FYI, I just fixed the bugs for the kernel versions I'm dealing with, because I don't know which other versions are affected as well. On 01.07.2016 19:46, Josh Stone wrote: > On 07/01/2016 09:15 AM, Alexander Lochmann wrote: >> >> Hi folks! >> >> Finally, I decided to submit my patch, which makes SystemTap work for >> Android. Moreover, it adds two new features: >> - Support for ignoring all available tapset directories, except the one >> that is provided by -K >=20 > Can you explain why you need this? >=20 > One can already override the primary tapset directory with the > environment SYSTEMTAP_TAPSET. Ignoring -I options also seems > questionable, especially since this depends on the order -- it looks > like -I specified after -K will still be used, while those before will > be ignored. >=20 Honestly? I don't know. :D I had some trouble with SystemTap looking for tapsets systemwide, which made me write that patch/fix. It's been a long time since I wrote it. Anyway, I tried your proposed solution, and it works. I'll adapt my build script to use the enviroment variable. You can skip that part of the patch if you want. >> - Support for a pid file in staprun, parameter is -U >=20 > And you added -M on stap itself. I don't understand either of those > letter choices. Did I? It might be possible that I want be able to pass the argument to staprun. I chose the letters randomly. For me, it doesn't matter. I just need an argument to tell staprun to create a pid file. :) >=20 > The functionality itself is not too controversial, but I'd still like to > see your justification in the commit log, and some examples how this is > used in Android. Yeah, sure. Since I run stap on Android, I have a background service, which periodically checks if every single stap instance is still running. Yes, it might be possible having more than one stap script running. :) Moreover, if I start it more than once, I want to be able to kill specific instance. For that I need its process id. - Alex >=20 >> I had to modify several source files of staprun. Those changes are >> mostly copied from the corresponding files contained in commit >> 2c10863bfe41b51272eff714a837f4977bdc257a. For some reasons, those ifdef >> parts have been removed. I readded them, and changed the macro, which >> activates them. >=20 > That commit is "man stap: fixed typos". Did you mean something else? >=20 >> The patch contains two bugfixes for the SystemTap as well. >> Unfortunately, I failed to extract those fixes properly. :( >=20 > As David said, it would help us a lot if you separated each of these > changes into distinct commits. If you have the changes in a working git > directory, "git add -p" can help you mark specific changes to be > committed. "git-gui" is also pretty good for selecting hunks. Feel > free to ask for help on #systemtap if you still have trouble. >=20 >> The first fix starts at line 510, and goes until line 555. >> Since an older kernel like 3.0 does not support uprobes, systemtap >> includes 'runtime/linux/task_finder_stubs.c'. That file itself does >> *not* include 'syscall.h', which declares several syscall-related functi= ons. >> The second fix starts at line 1106. For some reasons in the Linux kernel >> 3.0 the macro cputime_to_usecs() has a semicolon at the end of its >> definition. Therefore, the defition of cputime_to_msecs() in ' >> tapset/linux/task_time.stp' must be modified to deal with that fact. >> >> Cheers, >> Alex >> >> --- >> Technische Universit=C3=A4t Dortmund >> Alexander Lochmann PGP key: 0xBC3EF6FD >> Otto-Hahn-Str. 16 phone: +49.231.7556141 >> D-44227 Dortmund fax: +49.231.7556116 >> http://ess.cs.tu-dortmund.de/Staff/al >> >=20 --=20 Technische Universit=C3=A4t Dortmund Alexander Lochmann PGP key: 0xBC3EF6FD Otto-Hahn-Str. 16 phone: +49.231.7556141 D-44227 Dortmund fax: +49.231.7556116 http://ess.cs.tu-dortmund.de/Staff/al --------------050109060007060706040205 Content-Type: text/x-patch; name="0001-Definition-of-cputime_to_usecs-in-Linux-kernel-3.0-i.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename*0="0001-Definition-of-cputime_to_usecs-in-Linux-kernel-3.0-i.pa"; filename*1="tch" Content-length: 1220 =46rom f0ca40f9aaa5ba9c39ffcefc39e450443603db44 Mon Sep 17 00:00:00 2001 From: Alexander Lochmann Date: Wed, 6 Jul 2016 13:55:07 +0200 Subject: [PATCH 1/3] Definition of cputime_to_usecs in Linux kernel 3.0 is broken The above macro is definined with a semicolon at the end. Using it as a function argument leads to a compiler error. Hence, the necessary braces have been added. --- tapset/linux/task_time.stp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tapset/linux/task_time.stp b/tapset/linux/task_time.stp index f86f984..f3c276c 100644 --- a/tapset/linux/task_time.stp +++ b/tapset/linux/task_time.stp @@ -27,8 +27,12 @@ * Yet note some kernels (RHEL6) may already have both... */ #if defined(cputime_to_usecs) #if !defined(cputime_to_msecs) +#if LINUX_VERSION_CODE <=3D KERNEL_VERSION(3,0,200) && LINUX_VERSION_CODE = >=3D KERNEL_VERSION(3,0,0) +#define cputime_to_msecs(__ct) _stp_div64(NULL, ({cputime_to_usecs(__ct)}= ), 1000ULL) +#else #define cputime_to_msecs(__ct) _stp_div64(NULL, cputime_to_usecs(__ct), 1= 000ULL) #endif +#endif =20 /* Kernels before 2.6.37 have cputime_to_msecs, but not usecs. */ #elif defined(cputime_to_msecs) --=20 2.7.4 --------------050109060007060706040205 Content-Type: text/x-patch; name="0002-The-syscall-defines-were-not-compatible-with-older-k.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename*0="0002-The-syscall-defines-were-not-compatible-with-older-k.pa"; filename*1="tch" Content-length: 2559 =46rom 8f3817586f87cb65bc3b8b320ad8252a6a11f7e5 Mon Sep 17 00:00:00 2001 From: Alexander Lochmann Date: Wed, 6 Jul 2016 13:59:23 +0200 Subject: [PATCH 2/3] The syscall defines were not compatible with older kernels, namely 3.0.x Older kernels, at least the 3.0.x, define their syscalls in different files. The stap runtime has to include those file properly. Moreover, for pre-upro= be kernels no syscall file has been included at all. This job is normally done by task= _finder.c. The task_finder_stubs.c has been extended by the needed include directive. --- runtime/linux/autoconf-asm-syscall.c | 8 +++++++- runtime/linux/task_finder_stubs.c | 1 + runtime/syscall.h | 8 +++++++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/runtime/linux/autoconf-asm-syscall.c b/runtime/linux/autoconf-= asm-syscall.c index bf7a273..6bfcd55 100644 --- a/runtime/linux/autoconf-asm-syscall.c +++ b/runtime/linux/autoconf-asm-syscall.c @@ -1,2 +1,8 @@ +#include +#if LINUX_VERSION_CODE <=3D KERNEL_VERSION(3,0,200) && LINUX_VERSION_CODE = >=3D KERNEL_VERSION(3,0,0) +#include +#include +#include +#else #include - +#endif diff --git a/runtime/linux/task_finder_stubs.c b/runtime/linux/task_finder_= stubs.c index 700bb3d..39f9ec2 100644 --- a/runtime/linux/task_finder_stubs.c +++ b/runtime/linux/task_finder_stubs.c @@ -1,6 +1,7 @@ #ifndef TASK_FINDER_STUBS_C #define TASK_FINDER_STUBS_C =20 +#include "syscall.h" /* Stubs of last resort for when utrace type functionality is not available. Nothing should actually work, but things compile properly, and silently return dummy data or noisily fail as diff --git a/runtime/syscall.h b/runtime/syscall.h index b959d46..b652946 100644 --- a/runtime/syscall.h +++ b/runtime/syscall.h @@ -7,7 +7,6 @@ * Public License (GPL); either version 2, or (at your option) any * later version. */ - #ifndef _SYSCALL_H_ /* -*- linux-c -*- */ #define _SYSCALL_H_ =20 @@ -110,7 +109,14 @@ #ifdef STAPCONF_ASM_SYSCALL_H =20 /* If the system has asm/syscall.h, use defines from it. */ +#include +#if LINUX_VERSION_CODE <=3D KERNEL_VERSION(3,0,200) && LINUX_VERSION_CODE = >=3D KERNEL_VERSION(3,0,0) +#include +#include +#include +#else #include +#endif =20 #if defined(__arm__) /* The syscall_get_nr() function on 3.17.1-302.fc21.armv7hl always --=20 2.7.4 --------------050109060007060706040205 Content-Type: text/x-patch; name="0003-Linux-kernel-3.4-does-not-define-PR_SET_MM_ARG_START.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename*0="0003-Linux-kernel-3.4-does-not-define-PR_SET_MM_ARG_START.pa"; filename*1="tch" Content-length: 1049 =46rom 4c6cb3c2d2ce969a16fe91b2a06ed78d68b33644 Mon Sep 17 00:00:00 2001 From: Alexander Lochmann Date: Tue, 5 Jul 2016 19:05:06 +0200 Subject: [PATCH 3/3] Linux kernel 3.4 does not define PR_SET_MM_ARG_START. Used KERNEL_VERSION to permit compilatioon of those macros --- tapset/linux/aux_syscalls.stp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tapset/linux/aux_syscalls.stp b/tapset/linux/aux_syscalls.stp index bec71c5..88db0b7 100644 --- a/tapset/linux/aux_syscalls.stp +++ b/tapset/linux/aux_syscalls.stp @@ -4599,12 +4599,14 @@ static const _stp_val_array const _stp_prctl_mm_opt= ion_list[] =3D { V(PR_SET_MM_START_STACK), V(PR_SET_MM_START_BRK), V(PR_SET_MM_BRK), +#if LINUX_VERSION_CODE >=3D KERNEL_VERSION(3,4,200) && LINUX_VERSION_CODE = <=3D KERNEL_VERSION(3,4,0) V(PR_SET_MM_ARG_START), V(PR_SET_MM_ARG_END), V(PR_SET_MM_ENV_START), V(PR_SET_MM_ENV_END), V(PR_SET_MM_AUXV), V(PR_SET_MM_EXE_FILE), +#endif #ifdef PR_SET_MM_MAP V(PR_SET_MM_MAP), #endif --=20 2.7.4 --------------050109060007060706040205-- --vKn3HbtTLKQTe6i0OM0EUrjW4dT2VAG9j-- --JoGO19ow0JrglvFOhL6ofwWvOkcB81bLE Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" Content-length: 819 -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJXfPmjAAoJEFk+7QW8Pvb92c0P/03aQFNwITvTfFcIo9E8yXgK +rENX6irGa3At8YuUzhkkQosWuboSh/pjDnqxD5SwqoDO1d0PSiMGzcoMVqbP1Co x83nndVOPqMtrCyHJ5AyDlto7WqGsu6aohKvxfo19chszDh53FyqXk+9M6VywIPs 6a/s1iKQe/ar5hvCvjKNMl1TKWlbwvARxMaqys+o7hjl5wDPtaPWoQPEGumSKaBU /wnVgQGk5MkG+6yRMZhDyg/tL5DRSWvNLfdV3j6P2g0aNYtEYKiqbGo1hGrh8ye/ oD52HBSGQi8S5mgoK+M/LyhOs2d+3QQlclDFihi3Xw7Rq+fWz3mcdvJjywzlAxQE 8AD1o1Ya33deOFbfh2tiIIKf0gNduKdioRZjnba/GImvqByXx3WckPr5a5GG55nU rIk9re16WXBgx0v1LHWup9eugPZLZUED+zzxM2uRrku4V3w/xxaQ/uRro9OsZ4g+ uWpOhS18cl8rNoMCgYfeijbtrO/VwpIJn3i+15H5oIB4sIzBKbA0RKPNdMFvIfn8 bmBm1n5femQus3TEZwiXPbwgSlXDBVWYa/rGvXEeUfqfiWpNlUX6lm4KgN9fFxvE Vx1Thvl+4WwT7R9gNQmahDy2Gof96MvsIgHw93fIibADjjaK0rIhE3Hgckk8QttZ fkVcV0HyJcX4V75oQnhz =5Dfp -----END PGP SIGNATURE----- --JoGO19ow0JrglvFOhL6ofwWvOkcB81bLE--