lkml.org 
[lkml]   [2020]   [Mar]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 01/13] objtool: Remove CFI save/restore special case
On Thu, Mar 26, 2020 at 12:30:50PM +0100, Peter Zijlstra wrote:
> Subject: objtool: Remove CFI save/restore special case
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Wed Mar 25 12:58:16 CET 2020
>
> There is a special case in the UNWIND_HINT_RESTORE code. When, upon
> looking for the UNWIND_HINT_SAVE instruction to restore from, it finds
> the instruction hasn't been visited yet, it normally issues a WARN,
> except when this HINT_SAVE instruction is the first instruction of
> this branch.
>
> The reason for this special case comes apparent when we remove it;
> code like:
>
> if (cond) {
> UNWIND_HINT_SAVE
> // do stuff
> UNWIND_HINT_RESTORE
> }
> // more stuff
>
> will now trigger the warning. This is because UNWIND_HINT_RESTORE is
> just a label, and there is nothing keeping it inside the (extended)
> basic block covered by @cond. It will attach itself to the first
> instruction of 'more stuff' and we'll hit it outside of the @cond,
> confusing things.
>
> I don't much like this special case, it confuses things and will come
> apart horribly if/when the annotation needs to support nesting.
> Instead extend the affected code to at least form an extended basic
> block.

> @@ -727,6 +727,13 @@ static inline void sync_core(void)
> #else
> unsigned int tmp;
>
> + /*
> + * The trailing NOP is required to make this an extended basic block,
> + * such that we can argue about it locally. Specifically this is
> + * important for the UNWIND_HINTs, without this the UNWIND_HINT_RESTORE
> + * can fall outside our extended basic block and objtool gets
> + * (rightfully) confused.
> + */
> asm volatile (
> UNWIND_HINT_SAVE
> "mov %%ss, %0\n\t"
> @@ -739,7 +746,7 @@ static inline void sync_core(void)
> "pushq $1f\n\t"
> "iretq\n\t"
> UNWIND_HINT_RESTORE
> - "1:"
> + "1: nop\n\t"
> : "=&r" (tmp), ASM_CALL_CONSTRAINT : : "cc", "memory");

Note that the special case very much relies on the HINT_SAVE being the
first instruction of the (extended) basic block, which is only true in
this one usage anyway.

\
 
 \ /
  Last update: 2020-03-26 14:01    [W:0.099 / U:9.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site