From mboxrd@z Thu Jan  1 00:00:00 1970
Content-Type: multipart/mixed; boundary="===============5857512717270410103=="
MIME-Version: 1.0
From: Roland McGrath <roland@hack.frob.com>
To: elfutils-devel@lists.fedorahosted.org
Subject: Re: [PATCH] Support 1-sized reads in read_ubyte_unaligned_inc and
 read_sbyte_unaligned_inc
Date: Thu, 11 Sep 2014 15:16:19 -0700
Message-ID: <20140911221619.A9AC62C3972@topped-with-meat.com>
In-Reply-To: m21trj3upp.fsf@redhat.com

--===============5857512717270410103==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable

> Five, four of them with constant size and one with a 64bit?8:4 sort of
> expression.  The reads are done through a macro that checks bounds,
> There's one macro for all the widths, mostly because I didn't like to
> have four macros with unknown cut'n'paste errors.  I expect that the
> compiler will be able to see through and just inline a check and an
> access for the right width directly, but I didn't actually check.

The existing uses are in practice 64bit?8:4 cases.  They just happen
to allow a nonsensical 2 case rather than diagnosing it (i.e. a
crazy FDE encoding) early.  readelf.c's encoded_ptr_size has exactly
this one caller, so it could just be rolled in there and have a
diagnostic and early bail for anything that's not a 4- or 8-wide
encoding.  Then we could have a memory-access.h macro that is
specifically only for "ptr_size" (4 or 8), and using the single-size
named macros for the constant-size cases.

For your macro used for constant-size cases, you could make it a
single macro that takes the size literal as a macro argument and
uses read_##size##ubyte_unaligned_inc.

> Admittedly this is all somewhat moot.  I don't check bounds with LEB's
> anyway, and most of libdw just checks post fact that the pointers are
> still in bounds.  Maybe I should simply do the same.

We should be consistent throughout the codebase, one way or another.
If the bounds-checking matters for one case, it matters for the
others; if we don't care for umpteen cases, we shouldn't care any
more for one more case.  I've never been happy with the lossy bounds
checking, and tend to think we should fix it up all throughout.  But
that is quite unrelated to your current work.  It probably makes
sense to finish the new thing you're doing now just by following the
existing models, i.e. doing no better on bounds-checking but
matching the idioms we see elsewhere in the code.  Then later (right
away if you care to take it on, in the fullness of time if not) we
can figure out how to revamp the whole memory-access.h interface and
all its users to be fully robust.


Thanks,
Roland

--===============5857512717270410103==--