x86-64/memset: Mark the debugger symbol as hidden

Message ID 20180505154639.GA23073@intel.com
State New, archived
Headers

Commit Message

Lu, Hongjiu May 5, 2018, 3:46 p.m. UTC
  When MEMSET_SYMBOL (__memset, erms) is provided for debugger, mark it
as hidden so that it will be local to the library.

Any comments?

H.J.
---
	* sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
	(MEMSET_SYMBOL (__memset, erms)): Mark the debugger symbol as
	hidden.
---
 sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

H.J. Lu May 7, 2018, 5:59 p.m. UTC | #1
On Sat, May 5, 2018 at 8:46 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> When MEMSET_SYMBOL (__memset, erms) is provided for debugger, mark it
> as hidden so that it will be local to the library.
>
> Any comments?
>
> H.J.
> ---
>         * sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
>         (MEMSET_SYMBOL (__memset, erms)): Mark the debugger symbol as
>         hidden.
> ---
>  sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S b/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
> index d0fe79c9f1..a0560cd5c8 100644
> --- a/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
> +++ b/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
> @@ -131,7 +131,8 @@ END (__memset_chk_erms)
>  /* Only used to measure performance of REP STOSB.  */
>  ENTRY (__memset_erms)
>  # else
> -/* Provide a symbol to debugger.  */
> +/* Provide a hidden symbol to debugger.  */
> +       .hidden MEMSET_SYMBOL (__memset, erms)
>  ENTRY (MEMSET_SYMBOL (__memset, erms))
>  # endif
>  L(stosb):
> --
> 2.17.0
>

I am checking it in.
  
Adhemerval Zanella May 7, 2018, 6:22 p.m. UTC | #2
On 07/05/2018 14:59, H.J. Lu wrote:
> On Sat, May 5, 2018 at 8:46 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>> When MEMSET_SYMBOL (__memset, erms) is provided for debugger, mark it
>> as hidden so that it will be local to the library.
>>
>> Any comments?
>>
>> H.J.
>> ---
>>         * sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
>>         (MEMSET_SYMBOL (__memset, erms)): Mark the debugger symbol as
>>         hidden.
>> ---
>>  sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S b/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
>> index d0fe79c9f1..a0560cd5c8 100644
>> --- a/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
>> +++ b/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
>> @@ -131,7 +131,8 @@ END (__memset_chk_erms)
>>  /* Only used to measure performance of REP STOSB.  */
>>  ENTRY (__memset_erms)
>>  # else
>> -/* Provide a symbol to debugger.  */
>> +/* Provide a hidden symbol to debugger.  */
>> +       .hidden MEMSET_SYMBOL (__memset, erms)
>>  ENTRY (MEMSET_SYMBOL (__memset, erms))
>>  # endif
>>  L(stosb):
>> --
>> 2.17.0
>>
> 
> I am checking it in.
> 

Why is this need only for this symbol in particular and not for all other
ifunc variations? What debugger is missing and does it potentially might
affect other architectures as well?
  
H.J. Lu May 7, 2018, 6:31 p.m. UTC | #3
On Mon, May 7, 2018 at 11:22 AM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
> On 07/05/2018 14:59, H.J. Lu wrote:
>> On Sat, May 5, 2018 at 8:46 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>>> When MEMSET_SYMBOL (__memset, erms) is provided for debugger, mark it
>>> as hidden so that it will be local to the library.
>>>
>>> Any comments?
>>>
>>> H.J.
>>> ---
>>>         * sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
>>>         (MEMSET_SYMBOL (__memset, erms)): Mark the debugger symbol as
>>>         hidden.
>>> ---
>>>  sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S b/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
>>> index d0fe79c9f1..a0560cd5c8 100644
>>> --- a/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
>>> +++ b/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
>>> @@ -131,7 +131,8 @@ END (__memset_chk_erms)
>>>  /* Only used to measure performance of REP STOSB.  */
>>>  ENTRY (__memset_erms)
>>>  # else
>>> -/* Provide a symbol to debugger.  */
>>> +/* Provide a hidden symbol to debugger.  */
>>> +       .hidden MEMSET_SYMBOL (__memset, erms)
>>>  ENTRY (MEMSET_SYMBOL (__memset, erms))
>>>  # endif
>>>  L(stosb):
>>> --
>>> 2.17.0
>>>
>>
>> I am checking it in.
>>
>
> Why is this need only for this symbol in particular and not for all other
> ifunc variations? What debugger is missing and does it potentially might
> affect other architectures as well?

This is specific to memset-vec-unaligned-erms.S.   The code looks like

function 1 entry:
   ..
   ret

local label:
  ...
  ret

function 2 entry:
  ...
  jump to local label

If "local label" isn't in the symbol table, gdb will think you jump to
"function 1"
when single-stepping over "jump to local label".
  
Adhemerval Zanella May 7, 2018, 6:56 p.m. UTC | #4
On 07/05/2018 15:31, H.J. Lu wrote:
> On Mon, May 7, 2018 at 11:22 AM, Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>> On 07/05/2018 14:59, H.J. Lu wrote:
>>> On Sat, May 5, 2018 at 8:46 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>>>> When MEMSET_SYMBOL (__memset, erms) is provided for debugger, mark it
>>>> as hidden so that it will be local to the library.
>>>>
>>>> Any comments?
>>>>
>>>> H.J.
>>>> ---
>>>>         * sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
>>>>         (MEMSET_SYMBOL (__memset, erms)): Mark the debugger symbol as
>>>>         hidden.
>>>> ---
>>>>  sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S b/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
>>>> index d0fe79c9f1..a0560cd5c8 100644
>>>> --- a/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
>>>> +++ b/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
>>>> @@ -131,7 +131,8 @@ END (__memset_chk_erms)
>>>>  /* Only used to measure performance of REP STOSB.  */
>>>>  ENTRY (__memset_erms)
>>>>  # else
>>>> -/* Provide a symbol to debugger.  */
>>>> +/* Provide a hidden symbol to debugger.  */
>>>> +       .hidden MEMSET_SYMBOL (__memset, erms)
>>>>  ENTRY (MEMSET_SYMBOL (__memset, erms))
>>>>  # endif
>>>>  L(stosb):
>>>> --
>>>> 2.17.0
>>>>
>>>
>>> I am checking it in.
>>>
>>
>> Why is this need only for this symbol in particular and not for all other
>> ifunc variations? What debugger is missing and does it potentially might
>> affect other architectures as well?
> 
> This is specific to memset-vec-unaligned-erms.S.   The code looks like
> 
> function 1 entry:
>    ..
>    ret
> 
> local label:
>   ...
>   ret
> 
> function 2 entry:
>   ...
>   jump to local label
> 
> If "local label" isn't in the symbol table, gdb will think you jump to
> "function 1"
> when single-stepping over "jump to local label".
> 

Right, thanks for the explanation.
  

Patch

diff --git a/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S b/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
index d0fe79c9f1..a0560cd5c8 100644
--- a/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
+++ b/sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S
@@ -131,7 +131,8 @@  END (__memset_chk_erms)
 /* Only used to measure performance of REP STOSB.  */
 ENTRY (__memset_erms)
 # else
-/* Provide a symbol to debugger.  */
+/* Provide a hidden symbol to debugger.  */
+	.hidden	MEMSET_SYMBOL (__memset, erms)
 ENTRY (MEMSET_SYMBOL (__memset, erms))
 # endif
 L(stosb):