Hi Zhen,
On Sat, Dec 17, 2022 at 8:32 AM Leizhen (ThunderTown)
<thunder.leizhen@xxxxxxxxxx> wrote:
On 2022/12/17 3:27, David Laight wrote:
From: Steven Rostedt
Sent: 16 December 2022 17:38
On Fri, 16 Dec 2022 12:19:47 -0500
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
I assumed that "memory" was for memory unrelated to the input constraints.
Well, it looks like you do need a "memory" barrier.
https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html
"memory"
The "memory" clobber tells the compiler that the assembly code
performs memory reads or writes to items other than those listed in
the input and output operands (for example, accessing the memory
pointed to by one of the input parameters). To ensure memory contains
correct values, GCC may need to flush specific register values to
memory before executing the asm. Further, the compiler does not
assume that any values read from memory before an asm remain
unchanged after that asm; it reloads them as needed. Using the
"memory" clobber effectively forms a read/write memory barrier for
the compiler.
As the "(for example, accessing the memory pointed to by one of the input
parameters)" is exactly this case.
Without the memory clobber code like:
int f(const char *s)
{
char c[4] = "abc";
return strcmp(s, c);
}
is very like to get optimised so that c[] is never written.
However, in this case, the strings have all existed for ages.
So that won't be the problem.
It might be obvious what is wrong from the asm output.
Although the binary-chop lookup is suspect I'd also check
that the sorted index is plausible - just tracing the first
20 entries might be enough.
No point peering at the search code if the setup is wrong.
6.47.2.1 Volatile
GCC’s optimizers sometimes discard asm statements if they determine there is no need for
the output variables. Also, the optimizers may move code out of loops if they believe that
the code will always return the same result (i.e. none of its input values change between
calls). Using the volatile qualifier disables these optimizations.
So it's quite possible (I didn't disassemble vmlinux, because I didn't learn m68k):
You don't have to disassemble, "make kernel/kallsyms.s" creates
annotated assembler.
//binary search
while (low <= high) {
...
ret = compare_symbol_name(name, namebuf); ----> (1)
if (!ret)
break;
}
low = mid;
while (low) {
...
if (compare_symbol_name(name, namebuf)) ----> (2)
break;
low--;
}
The pointer 'name' and 'namebuf' of (1) and (2) are the same,
so the 'if' statement of (2) maybe omitted by compiler.
And that is exactly what is happening: there are 3 calls to strcmp()
with the exact same parameters, and gcc omits two of them, because it
assumes it can predict the outcome, as the parameters haven't changed.
Now, why have we never noticed this before? I guess because it is very
uncommon for a function calling strcmp() multiple times with the exact
same pointer parameters. Common users change the pointers before
every call, instead of keeping the pointer, but changing the buffers'
contents the pointers point to.
By the way, I tried no volatile but with
+ : : "memory");
It also works well.
Indeed, gcc version 9.4.0 (Ubuntu 9.4.0-1ubuntu1~20.04) generates the
same code for either adding the volatile or the memory clobber.
Note that strcmp() is the only function in arch/m68k/include/asm/string.h
using inline asm without the volatile keyword. I guess we would
see similar issues with strnlen() (which also doesn't modify memory)
when dropping the volatile.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
