On Thu, Oct 7, 2021 at 9:48 PM Denys Vlasenko <[email protected]> wrote:
>
> On Thu, Oct 7, 2021 at 3:25 PM 余生与君 <[email protected]> wrote:
> > > where p is a dummy, unused variable
> > No. p here shadows the global variable with the same name local
> > variable so the following context (scope) will use this local variable
> > instead of the global one.
>
> Aha...
> The problem here is that even though later uses of "p" in this block
> where we use ASSIGN_CONST_PTR() macro will use the local "p" pointer,
> when we exit the block, the following uses will refer to the global one.
> Nothing prevents them to still use incorrect value.
> The fix depends only on the hope that there won't be such uses.
> But they already exist:
>
> inetd.c:
> INIT_G();
> real_uid = getuid();
>
> ftpgetput.c:
> INIT_G();
> /* Set default values */
> user = "anonymous";
> password = "busybox";
Yes, I noticed that, and that's why I removed do-while in the INIT_G
in this patch.
> Can you try something more? E.g. (in current git):
>
> #define ASSIGN_CONST_PTR(p, v) do { \
> *(void**)not_const_pp(&p) = (void*)(v); \
> /* At least gcc 3.4.6 on mipsel needs optimization barrier */ \
> barrier(); \
> + sleep(0); \
> } while (0)
Cool! Sleep(0) does the magic!
ADRP X21, #ash_ptr_to_globals_misc_ptr@PAGE
LDR X21, [X21,#ash_ptr_to_globals_misc_ptr@PAGEOFF]
MOV X8, X21
STR X0, [X8]
MOV W0, WZR
BL sleep
LDR X21, [X21]
And further investigation shows that a dummy function can also do this trick!
hack.c:
void clang_barrier() {
}
libbb.h:
void clang_barrier(); // invisible in this file
#define ASSIGN_CONST_PTR(p, v) do { \
*(void**)not_const_pp(&p) = (void*)(v); \
/* At least gcc 3.4.6 on mipsel needs optimization barrier */ \
- barrier(); \
+ clang_barrier(); \
} while (0)
Result:
ADRP X21, #ash_ptr_to_globals_misc_ptr@PAGE
LDR X21, [X21,#ash_ptr_to_globals_misc_ptr@PAGEOFF]
MOV X8, X21
STR X0, [X8]
BL clang_barrier
LDR X21, [X21]
I will submit a patch about this.
> or even
>
> - barrier(); \
> + pselect(0, NULL, NULL, NULL, {0,0}, NULL); \
>
> Basically, we should convince clang to not load "p" too early,
> by starving it of available registers to store the fetched result.
>
> > > Also, why ({...;})
> > That is to avoid conflict between the global variable and the local
> > variable. When the ({...}) part is evaluating, it uses the global
> > variable p and stores it into the local variable, and then returns the
> > variable to initialize the local variable.
>
> I'm not even sure other compilers would interpret it like that -
> they might think that "p" inside ({}) is the newly defined one.
That's why I only apply it to clang to avoid other compilers having
different behavior.
But in c++14, this is UB. The `p` is the newly defined one. But we can
avoid this to two statements with a tmp variable. But the name can be
conflicted.
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
