Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

From: Jiri Slaby
Date: Fri May 12 2017 - 03:53:55 EST


On 04/26/2017, 03:42 AM, Josh Poimboeuf wrote:
>> @@ -323,7 +323,7 @@ ENTRY(resume_userspace)
>> movl %esp, %eax
>> call prepare_exit_to_usermode
>> jmp restore_all
>> -END(ret_from_exception)
>> +ENDPROC(ret_from_exception)
>
> What exactly is the motivation of this patch? It would be good to
> describe that in the commit message.
>
> Is the point to allow objtool to generate CFI for it? If so, I don't
> really see how that would work. Today, objtool considers ENDPROC to
> annotate a *callable* function which conforms to the C calling ABI and
> can be called by another function. The stack is in a known state at
> function entry, and so the CFI (or frame pointer info) can be reliably
> determined.

Ugh, I haven't checked this in 100 % of cases, but this looks pretty
fragile to me. From reading the code, the use of END or ENDPROC is
rather random -- depending on mood and who wrote the code.

> But entry code is different. In most cases, the global symbols aren't
> actually called, and they don't follow any conventions. The code is
> spaghetti-esque, with HW handlers and jumps everywhere. The state of
> the stack at symbol entry varies per "function". That's why objtool
> ignores these files.

Unfortunately, this is true.

> For special cases (like entry code), I was thinking we'd need manual CFI
> annotations, like we had before. Or maybe there's another way, like
> some new macros which tell objtool about the HW entry points and the
> state of the registers there.
>
> But I'm having trouble seeing how marking these code snippets with
> ENTRY/ENDPROC would help objtool make any sense of the code and where
> things are on the stack.

Ok, my intention was to have every line of assembly code in between of
FUNC_START/FUNC_END. That way, every rsp related push/pop/sub/add can be
annotated very easily. For the C-like functions this is all what needs
to be done.

Then there is the spaghetti code. And I was thinking about manual
annotations like:

# skip the frame pointer checking between START+END here
OBJTOOL(SKIP_CHECKING)

# this fn has unusual frame (like interrupts have),
and you can find return RIP stored at fp + 0x20
OBJTOOL(RIP_IS_AT, 0x20)

# put this raw CFI for this location into eh_frame
OBJTOOL(RAW_CFI, 0x00, 0x00, 0x00)


Similarly, I have OBJTOOL(START_FUNC) and OBJTOOL(END_FUNC) emitted with
each FUNC_START/FUNC_END. So far, when manually expanded for simplicity,
it looks like this:

#define OBJTOOL_START_FUNC \
.pushsection .discard.asmfunctions ASM_NL \
.long 0xfd111111 ASM_NL \
.long 1f - . ASM_NL \
.popsection ASM_NL \
1:

#define OBJTOOL_END_FUNC \
.pushsection .discard.asmfunctions ASM_NL \
.long 0xfe111111 ASM_NL \
.long 1f - . ASM_NL \
.popsection ASM_NL \
1:

0xfd111111, 0xfe111111 are "opcodes" for objtool meaning
START_FUNC/END_FUNC. Similar would be SKIP_CHECKING, RIP_IS_AT, and
RAW_CFI from the above.

So on the objtool side, it looks like:
switch (data->magic) {
case 0xfd111111:
pc_begin = rela->addend;
break;
case 0xfe111111:
ret = dwarf_annotate_func(dwarf, rela->sym->sec,
pc_begin, rela->addend - pc_begin);
if (ret < 0)
return -1;

break;

So this was my idea -- having all code marked as function and manually
annotate those which are different.

thanks,
--
js
suse labs