From owner-freebsd-hackers@FreeBSD.ORG Fri Mar 29 12:32:08 2013 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id A6B5783D; Fri, 29 Mar 2013 12:32:08 +0000 (UTC) (envelope-from to.my.trociny@gmail.com) Received: from mail-ee0-f51.google.com (mail-ee0-f51.google.com [74.125.83.51]) by mx1.freebsd.org (Postfix) with ESMTP id 3D3D4F84; Fri, 29 Mar 2013 12:32:07 +0000 (UTC) Received: by mail-ee0-f51.google.com with SMTP id c4so203860eek.10 for ; Fri, 29 Mar 2013 05:32:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=FpJ0+MsgWnQ+HeV4MEpNTXHsW81Nv+zxGs76zz4EUOg=; b=k1LOhBdn40R5U/OX29k4sQS59iMnt8ERYqFWoGFmgxye3SpnjVOP8i780VCGkWS8sS bu2HVhDiQ3cdOqpXMCxy83joRPfNCVnG9yssRBozdbTymbcSaQADiAvBL2uYbGW+7ia2 GH+xemG4ArFo0zQ5EuitlGE/fg1a+mG4SQl77ZUMt0wTV/hm1c/5FbEpkog2nZ32CuAw uVZeRnVeAumvXp1E4ODZ12pmP4I9a+U1F3YwOL2IzhMiSYSnkFCYr6AWJdNpyDO75pR7 4VD0TkVmifWo/2mCGQw84tYeU8w5PKpkvLcrY6LXLmQuM5bIdy0qWV4zasEOLrbcdnXY 3v0A== X-Received: by 10.14.5.6 with SMTP id 6mr7321801eek.42.1364560320088; Fri, 29 Mar 2013 05:32:00 -0700 (PDT) Received: from localhost ([188.230.122.226]) by mx.google.com with ESMTPS id 3sm3805903eej.6.2013.03.29.05.31.58 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 29 Mar 2013 05:31:59 -0700 (PDT) Date: Fri, 29 Mar 2013 14:31:57 +0200 From: Mikolaj Golub To: Konstantin Belousov Subject: Re: libprocstat(3): retrieve process command line args and environment Message-ID: <20130329123155.GA94024@gmail.com> References: <20130220195801.GA8679@gmail.com> <20130316180915.GA91146@gmail.com> <20130316191605.GJ3794@kib.kiev.ua> <20130316223339.GA3534@gmail.com> <20130317063033.GL3794@kib.kiev.ua> <20130317091930.GA2833@gmail.com> <20130324155426.GA87022@gmail.com> <20130328105134.GO3794@kib.kiev.ua> <20130328211820.GA6657@gmail.com> <20130329092245.GU3794@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130329092245.GU3794@kib.kiev.ua> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Stanislav Sedov , Attilio Rao , freebsd-hackers@freebsd.org, Mikolaj Golub , "Robert N. M. Watson" X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Mar 2013 12:32:08 -0000 On Fri, Mar 29, 2013 at 11:22:45AM +0200, Konstantin Belousov wrote: > On Thu, Mar 28, 2013 at 11:18:21PM +0200, Mikolaj Golub wrote: > > On Thu, Mar 28, 2013 at 12:51:34PM +0200, Konstantin Belousov wrote: > > > > > In the generic Elf 64bit draft specification I have, the notes sections > > > are specified to consists of entries, each of which is an array of 8-byte > > > words. I think we are right using the 8-byte alignment. > > > > I have impression many implementations use 4-byte alignment. E.g. in > > NetBSD: > > > > sys/kern/core_elf32.c: > > > > #define ELFROUNDSIZE 4 /* XXX Should it be sizeof(Elf_Word)? */ > > #define elfround(x) roundup((x), ELFROUNDSIZE) > Note that this is core_elf32. I am concerned with the 64-bit cores. core_elf64.c: #define ELFSIZE 64 #include "core_elf32.c" > > > > Also, we have inconsistency with imgactl_elf.c/parse_notes(), which > > uses 4-byte alignment: > > > > note = (const Elf_Note *)((const char *)(note + 1) + > > roundup2(note->n_namesz, sizeof(Elf32_Addr)) + > > roundup2(note->n_descsz, sizeof(Elf32_Addr))); > > > > I suppose there were no issues before, because accidentally the sizes > > of all notes we had were 8 bytes aligned. > Indeed, both ABI and NOINIT notes have size which is multiple of 8. > > > > > Now, when I add new notes it will break things. I don't have strong > > opinion, it will be ok for me to leave 8-byte alignment and fix > > issues, just want to have strong support here :-) > Well, while the issue is discussed and decided, you could just make > your new notes size be multiple of 8 too. I thought about this too. Then I need to be more caerful when extracting stats from notes, because the length returned by procstat_core_get() can be more than a real payload. Ok, I will try this way. I could add length to the note header, which is currently contains only structsize, so it would became something like: struct { int structsize; int lenght; } But not sure it is worth doing, especially if the forced 8-bit alignment is a temporary mesure. > > > > BTW, looking at NetBSD code I see they set p_align in the note > > segement to ELFROUNDSIZE: > > > > /* Write out the PT_NOTE header. */ > > ws.psections->p_type = PT_NOTE; > > ws.psections->p_offset = notestart; > > ws.psections->p_vaddr = 0; > > ws.psections->p_paddr = 0; > > ws.psections->p_filesz = notesize; > > ws.psections->p_memsz = 0; > > ws.psections->p_flags = PF_R; > > ws.psections->p_align = ELFROUNDSIZE; > > > > while we set to 0: > > > > /* The note segement. */ > > phdr->p_type = PT_NOTE; > > phdr->p_offset = hdrsize; > > phdr->p_vaddr = 0; > > phdr->p_paddr = 0; > > phdr->p_filesz = notesz; > > phdr->p_memsz = 0; > > phdr->p_flags = 0; > > phdr->p_align = 0; > You mean, for the core dumps ? yes > > > > Shouldn't we set it to alignment size too? Note also, they set > > "Segment is readable" flag. > I think both changes are fine. > > I skimmed over the usermode parts of the patch. One thing I did not liked > is the mis-handling of the read() return values. If there is short read, > the errno value is meaningless, but warn() would still append it to > the message. I suggest to explicitely distinguish -1 and >= 0 returns > from reads. ok. Thanks. -- Mikolaj Golub