| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <6931c0d8-8aa8-8039-fc7f-8e2026b94036@c-s.fr>
Date: Fri, 23 Aug 2019 15:35:31 +0000
From: Christophe Leroy <christophe.leroy@....fr>
To: Segher Boessenkool <segher@...nel.crashing.org>
Cc: Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Paul Mackerras <paulus@...ba.org>,
Michael Ellerman <mpe@...erman.id.au>,
linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH 3/3] powerpc: use __builtin_trap() in BUG/WARN macros.
On 08/19/2019 03:45 PM, Segher Boessenkool wrote:
> On Mon, Aug 19, 2019 at 05:05:46PM +0200, Christophe Leroy wrote:
>> Le 19/08/2019 à 16:37, Segher Boessenkool a écrit :
>>> On Mon, Aug 19, 2019 at 04:08:43PM +0200, Christophe Leroy wrote:
>>>> Le 19/08/2019 à 15:23, Segher Boessenkool a écrit :
>>>>> On Mon, Aug 19, 2019 at 01:06:31PM +0000, Christophe Leroy wrote:
>>>>>> Note that we keep using an assembly text using "twi 31, 0, 0" for
>>>>>> inconditional traps because GCC drops all code after
>>>>>> __builtin_trap() when the condition is always true at build time.
>>>>>
>>>>> As I said, it can also do this for conditional traps, if it can prove
>>>>> the condition is always true.
>>>>
>>>> But we have another branch for 'always true' and 'always false' using
>>>> __builtin_constant_p(), which don't use __builtin_trap(). Is there
>>>> anything wrong with that ?:
>>>
>>> The compiler might not realise it is constant when it evaluates the
>>> __builtin_constant_p, but only realises it later. As the documentation
>>> for the builtin says:
>>> A return of 0 does not indicate that the
>>> value is _not_ a constant, but merely that GCC cannot prove it is a
>>> constant with the specified value of the '-O' option.
>>
>> So you mean GCC would not be able to prove that
>> __builtin_constant_p(cond) is always true but it would be able to prove
>> that if (cond) is always true ?
>
> Not sure what you mean, sorry.
>
>> And isn't there a away to tell GCC that '__builtin_trap()' is
>> recoverable in our case ?
>
> No, GCC knows that a trap will never fall through.
>
>>> I think it may work if you do
>>>
>>> #define BUG_ON(x) do { \
>>> if (__builtin_constant_p(x)) { \
>>> if (x) \
>>> BUG(); \
>>> } else { \
>>> BUG_ENTRY("", 0); \
>>> if (x) \
>>> __builtin_trap(); \
>>> } \
>>> } while (0)
>>
>> It doesn't work:
>
> You need to make a BUG_ENTRY so that it refers to the *following* trap
> instruction, if you go this way.
>
>>> I don't know how BUG_ENTRY works exactly.
>>
>> It's basic, maybe too basic: it adds an inline asm with a label, and
>> adds a .long in the __bug_table section with the address of that label.
>>
>> When putting it after the __builtin_trap(), I changed it to using the
>> address before the one of the label which is always the twxx instruction
>> as far as I can see.
>>
>> #define BUG_ENTRY(insn, flags, ...) \
>> __asm__ __volatile__( \
>> "1: " insn "\n" \
>> ".section __bug_table,\"aw\"\n" \
>> "2:\t" PPC_LONG "1b, %0\n" \
>> "\t.short %1, %2\n" \
>> ".org 2b+%3\n" \
>> ".previous\n" \
>> : : "i" (__FILE__), "i" (__LINE__), \
>> "i" (flags), \
>> "i" (sizeof(struct bug_entry)), \
>> ##__VA_ARGS__)
>
> #define MY_BUG_ENTRY(lab, flags) \
> __asm__ __volatile__( \
> ".section __bug_table,\"aw\"\n" \
> "2:\t" PPC_LONG "%4, %0\n" \
> "\t.short %1, %2\n" \
> ".org 2b+%3\n" \
> ".previous\n" \
> : : "i" (__FILE__), "i" (__LINE__), \
> "i" (flags), \
> "i" (sizeof(struct bug_entry)), \
> "i" (lab))
>
> called as
>
> #define BUG_ON(x) do { \
> MY_BUG_ENTRY(&&lab, 0); \
> lab: if (x) \
> __builtin_trap(); \
> } while (0)
>
> not sure how reliable that works -- *if* it works, I just typed that in
> without testing or anything -- but hopefully you get the idea.
>
I've not been able to make it work. GCC puts the label (.L2 and .L6)
outside of the function, so the instruction preceding the label is blr,
not the trap.
#define _EMIT_BUG_ENTRY \
".section __bug_table,\"aw\"\n" \
"2:\t" PPC_LONG "%4, %0\n" \
"\t.short %1, %2\n" \
".org 2b+%3\n" \
".previous\n"
#define BUG_ENTRY(flags, label) \
__asm__ __volatile__( \
_EMIT_BUG_ENTRY \
: : "i" (__FILE__), "i" (__LINE__), \
"i" (flags), \
"i" (sizeof(struct bug_entry)), \
"i" (label - 4))
#define __recoverable_trap() asm volatile ("twi 31, 0, 0;");
#define __WARN_FLAGS(flags) do { \
__label__ label; \
BUG_ENTRY(BUGFLAG_WARNING | (flags), &&label); \
__recoverable_trap(); \
label: ; \
} while (0)
#define WARN_ON(x) ({ \
int __ret_warn_on = !!(x); \
if (__builtin_constant_p(__ret_warn_on)) { \
if (__ret_warn_on) \
__WARN_TAINT(TAINT_WARN); \
} else { \
__label__ label; \
BUG_ENTRY(BUGFLAG_WARNING | BUGFLAG_TAINT(TAINT_WARN), &&label); \
if (__ret_warn_on) \
__builtin_trap(); \
label: ; \
} \
unlikely(__ret_warn_on); \
})
void test_warn1(unsigned long long a)
{
WARN_ON(a);
}
void test_warn2(unsigned long a)
{
WARN_ON(a);
}
00000000 <test_warn1>:
0: 7c 63 23 78 or r3,r3,r4
4: 0f 03 00 00 twnei r3,0
8: 4e 80 00 20 blr
0000000c <test_warn2>:
c: 0f 03 00 00 twnei r3,0
10: 4e 80 00 20 blr
RELOCATION RECORDS FOR [__bug_table]:
OFFSET TYPE VALUE
00000000 R_PPC_ADDR32 .text+0x00000008
00000004 R_PPC_ADDR32 .rodata.str1.4
0000000c R_PPC_ADDR32 .text+0x00000010
00000010 R_PPC_ADDR32 .rodata.str1.4
.file "test.c"
.section ".text"
.Ltext0:
.align 2
.globl test_warn1
.type test_warn1, @function
test_warn1:
.LFB598:
.file 1 "arch/powerpc/mm/test.c"
.loc 1 34 0
.LBB2:
.LBB3:
.loc 1 35 0
#APP
# 35 "arch/powerpc/mm/test.c" 1
.section __bug_table,"aw"
2: .long .L2-4, .LC0
.short 35, 2305
.org 2b+12
.previous
# 0 "" 2
#NO_APP
or 3,3,4
twnei 3,0
blr
.L3:
.L2:
.LBE3:
.LBE2:
.LFE598:
.size test_warn1, .-test_warn1
.align 2
.globl test_warn2
.type test_warn2, @function
test_warn2:
.LFB599:
.loc 1 39 0
.LBB4:
.LBB5:
.loc 1 40 0
#APP
# 40 "arch/powerpc/mm/test.c" 1
.section __bug_table,"aw"
2: .long .L6-4, .LC0
.short 40, 2305
.org 2b+12
.previous
# 0 "" 2
#NO_APP
twnei 3,0
blr
.L7:
.L6:
.LBE5:
.LBE4:
.LFE599:
Any idea ?
Christophe
Powered by blists - more mailing lists