Message ID | 1504294850.3182.66.camel@cavium.com |
---|---|
State | New |
Headers | show |
Series | [aarch64] Fix glibc.tune.cpu tunable handling | expand |
On Saturday 02 September 2017 01:10 AM, Steve Ellcey wrote: > While working on a gcc patch I realized that the glibc tunable > 'glibc.tune.cpu' was not working on aarch64. I tracked it down to > get_midr_from_mcpu and at first I thought I should remove the '== 0' > from the if statement because tunable_is_name returns true/false, not > -1/0/1 like strcmp. But then I realized we shouldn't be using > tunable_is_name at all because we are comparing two null terminated > strings and tunable_is_name is trying to compare a null terminated > string with a '=' terminated string which is not what we have. > > I tested this by running a program that calls memcpy and checking what > memcpy gets run after setting glibc.tune.cpu to generic, thunderx, > and falkor. Right, tunable_is_name is the wrong function to call but using strcmp there may not be safe. Please verify that it does not go through a PLT, i.e. it calls the ld.so version at all times. Siddhesh
On Mon, 2017-09-04 at 11:07 +0530, Siddhesh Poyarekar wrote: > > Right, tunable_is_name is the wrong function to call but using strcmp > there may not be safe. Please verify that it does not go through a PLT, > i.e. it calls the ld.so version at all times. > > Siddhesh I am not sure I know how to do this. If I look at csu/libc-start.o, where, after inlining, this code shows up I see a reference to "strcmp" and not to "__GI_strcmp" that I see in some other places. Does that mean I am going through the PLT? If so, what do I do about it? I also see a new undefined reference to strcmp from elf/dl-sysdep.os, but that file was already referencing "strlen", so maybe it doesn't matter there. Steve Ellcey sellcey@cavium.com
On Tuesday 05 September 2017 09:23 PM, Steve Ellcey wrote: > I am not sure I know how to do this. If I look at csu/libc-start.o, > where, after inlining, this code shows up I see a reference to "strcmp" > and not to "__GI_strcmp" that I see in some other places. Does that > mean I am going through the PLT? If so, what do I do about it? I also > see a new undefined reference to strcmp from elf/dl-sysdep.os, but that > file was already referencing "strlen", so maybe it doesn't matter > there. When you disassemble ld.so you should see (1) a definition of strcmp and (2) the call site should have a call to strcmp and not to strcmp@plt. Siddhesh
On Wed, 2017-09-06 at 11:53 +0530, Siddhesh Poyarekar wrote: > On Tuesday 05 September 2017 09:23 PM, Steve Ellcey wrote: > > > > I am not sure I know how to do this. If I look at csu/libc-start.o, > > where, after inlining, this code shows up I see a reference to "strcmp" > > and not to "__GI_strcmp" that I see in some other places. Does that > > mean I am going through the PLT? If so, what do I do about it? I also > > see a new undefined reference to strcmp from elf/dl-sysdep.os, but that > > file was already referencing "strlen", so maybe it doesn't matter > > there. > When you disassemble ld.so you should see (1) a definition of strcmp and > (2) the call site should have a call to strcmp and not to strcmp@plt. > > Siddhesh OK, I disassembled ld.so with 'objdump -d' and the calls are to 'strcmp' and not to 'strcmp@plt'. I see quite a few of them so it looks like we were already using strcmp in other places. I also see the definition of strcmp in ld.so. Steve Ellcey sellcey@cavium.com
On Wednesday 06 September 2017 08:58 PM, Steve Ellcey wrote: > OK, I disassembled ld.so with 'objdump -d' and the calls are to > 'strcmp' and not to 'strcmp@plt'. I see quite a few of them so > it looks like we were already using strcmp in other places. > > I also see the definition of strcmp in ld.so. Looks good then. Thanks, Siddhesh
diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/ linux/aarch64/cpu-features.c index 18f5e60..0c7e13f 100644 --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c @@ -37,7 +37,7 @@ static uint64_t get_midr_from_mcpu (const char *mcpu) { for (int i = 0; i < sizeof (cpu_list) / sizeof (struct cpu_list); i++) - if (tunable_is_name (mcpu, cpu_list[i].name) == 0) + if (strcmp (mcpu, cpu_list[i].name) == 0) return cpu_list[i].midr; return UINT64_MAX;