Date: Fri, 29 Mar 2013 14:31:57 +0200 From: Mikolaj Golub <to.my.trociny@gmail.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Stanislav Sedov <stas@freebsd.org>, Attilio Rao <attilio@freebsd.org>, freebsd-hackers@freebsd.org, Mikolaj Golub <trociny@FreeBSD.org>, "Robert N. M. Watson" <rwatson@freebsd.org> Subject: Re: libprocstat(3): retrieve process command line args and environment Message-ID: <20130329123155.GA94024@gmail.com> In-Reply-To: <20130329092245.GU3794@kib.kiev.ua> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130329123155.GA94024>