Messages in this thread | | | Subject | Re: GCC bug ? Re: [PATCH v2 10/10] powerpc/32s: Implement Kernel Userspace Access Protection | From | Christophe Leroy <> | Date | Wed, 22 Jan 2020 15:45:52 +0100 |
| |
Le 22/01/2020 à 14:36, Segher Boessenkool a écrit : > On Wed, Jan 22, 2020 at 07:52:02AM +0100, Christophe Leroy wrote: >> Le 21/01/2020 à 20:55, Segher Boessenkool a écrit : >>> On Tue, Jan 21, 2020 at 05:22:32PM +0000, Christophe Leroy wrote: >>>> g1() should return 3, not 5. >>> >>> What makes you say that? >> >> What makes me say that is that NULL is obviously a constant pointer and >> I think we are all expecting gcc to see it as a constant during kernel >> build, ie at -O2 > > But apparently at the point where the builtin was checked it did not > yet know it is passed a null pointer. > > Please make a self-contained test case if we need further investigation?
The test in my original mail is self-contained:
#define NULL (void*)0
static inline int f1(void *to) { if (__builtin_constant_p(to) && to == NULL) return 3; return 5; }
int g1(void) { return f1(NULL); }
Build the above with -O2 then objdump:
00000000 <g1>: 0: 38 60 00 05 li r3,5 4: 4e 80 00 20 blr
It returns 5 so that shows __builtin_constant_p(to) was evaluated as false.
> >>> "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." >>> >>> (And the rules it uses for this are *not* the same as C "constant >>> expressions" or C "integer constant expression" or C "arithmetic >>> constant expression" or anything like that -- which should be already >>> obvious from that it changes with different -Ox). >>> >>> You can use builtin_constant_p to have the compiler do something better >>> if the compiler feels like it, but not anything more. Often people >>> want stronger guarantees, but when they see how much less often it then >>> returns "true", they do not want that either. > >> If GCC doesn't see NULL as a constant, then the above doesn't work as >> expected. > > That's not the question. Of course GCC sees it as a null pointer > constant, because it is one. But this builtin does its work very > early, during preprocessing already. Its concept of "constant" is > very different. > > Does it work if you write just "0" instead of "NULL", btw? "0" is > also a null pointer constant eventually (here, that is).
No it doesn't.
It works if you change the 'void *to' to 'unsigned long to'
> > The question is why (and if, it still needs verification after all) > builtin_constant_p didn't return true.
I sent a patch to overcome the problem. See https://patchwork.ozlabs.org/patch/1227249/
Christophe
| |