Re: [PATCH v5 02/10] x86: Compile-time asm code validation

Josh Poimboeuf
2015-06-10T18:58:58Z

On Wed, Jun 10, 2015 at 11:15:19AM -0700, Andy Lutomirski wrote:
> On Wed, Jun 10, 2015 at 10:53 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > On Wed, Jun 10, 2015 at 10:21:36AM -0700, Andy Lutomirski wrote:
> >> On Jun 10, 2015 5:07 AM, "Josh Poimboeuf" <jpoimboe@redhat.com> wrote:
> >> > 2. Each callable function must never leave its own bounds (i.e. with a
> >> > jump to outside the function) except when returning.
> >>
> >> Won't that break with sibling/tail calls?
> >
> > Yes, asmvalidate will flag a warning for tail calls.
> >
> >> GCC can generate those, and the ia32_ptregs_common label is an example
> >> of such a thing.
> >>
> >> I'd rather have the script understand tail calls and possibly require
> >> that ia32_ptregs_common have a dummy frame pointer save in front
> >> before the label if needed.
> >
> > Why do you prefer tail calls there? See patch 3 for how I handled that
> > for ia32_ptregs_common (I duplicated the code with macros).
> >
> > I think adding support for tail calls in the tooling would be tricky.
> > So I'm just trying to figure out if there's a good reason to keep them.
>
> To save code size by deduplicating common tails. The code currently
> does that, and it would be nice to avoid bloating the code to keep the
> validator happy.

Well, I wonder whether it's really worth sacrificing code readability
and consistency, and maybe some improved i-cache locality, to save a few
hundred bytes of code size.

> I imagine that an automatic CFI annotation adder would walk through
> functions one instruction at a time and keep track of the frame state.
> If so, then it could verify that common jump targets had identical
> state and continue walking through them and annotating. I think this
> would get this case right, and it might be necessary anyway to handle
> jumps within functions.

This would definitely add complexity to both asmvalidate and the CFI
generator. In fact it sounds like it would push the CFI generator out
of its current awk script territory and more into complex C code
territory.

--
Josh