Roland McGrath <roland(a)hack.frob.com> writes:
> 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.
Right, that would be meaningful.
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.
Sure, I can do that, and add the corresponding _1ubyte/_1sbyte {,inc}
calls if you find that more acceptable.
> 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.
Bounds checking of course matters for all cases the same. But some have
bugs in them. That's no reason to add more buggy code, even if it
happens to be consistently buggy. To put it differently, I don't see
value--on side of maintainer or user--to have the guarantee that we
don't do any bounds checking.
But I realized something else. A patch that implements .debug_macro
support shouldn't happen to add bounds-checking primitives in a
by-the-way manner like mine does, and moreover keep them private. That
should be a separate patch adding this stuff in a properly global
manner, with a separate discussion and all that.
So yeah, let me rework it.
Thanks,
PM