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 ?

And isn't there a away to tell GCC that '__builtin_trap()' is recoverable in our case ?


(and there should be many more and more serious warnings here).

#define BUG_ON(x) do {                                          \
        if (__builtin_constant_p(x)) {                          \
                if (x)                                          \
                        BUG();                                  \
        } else {                                                \
                if (x)                                          \
                        __builtin_trap();                       \
                BUG_ENTRY("", 0);                             \
        }                                                       \
} while (0)

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:

void test_bug1(unsigned long long a)
{
        BUG_ON(a);
}

00000090 <test_bug1>:
  90:   7c 63 23 78     or      r3,r3,r4
  94:   0f 03 00 00     twnei   r3,0
  98:   4e 80 00 20     blr

RELOCATION RECORDS FOR [__bug_table]:
OFFSET   TYPE              VALUE
00000084 R_PPC_ADDR32      .text+0x00000090

As you see, the relocation in __bug_table points to the 'or' and not to the 'twnei'.


or even just

#define BUG_ON(x) do {                                          \
        BUG_ENTRY("", 0);                                     \
        if (x)                                                  \
                __builtin_trap();                               \
        }                                                       \
} while (0)

if BUG_ENTRY can work for the trap insn *after* it.

Can you put the bug table asm *before* the __builtin_trap maybe?  That
should make it all work fine...  If you somehow can tell what machine
instruction is that trap, anyway.

And how can I tell that ?

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__)

Christophe

Reply via email to