Re: [PATCH v2 2/6] elf: Avoid unnecessary slowdown from profiling with audit (BZ#15533)

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 20/07/2021 05:31, Alexander Monakov wrote:
> On Mon, 19 Jul 2021, Adhemerval Zanella via Libc-alpha wrote:
> 
>> The rtld-audit interfaces introduces a slowdown due to enabling profiling
> 
> "interface" (my original message had "using the audit interfaces introduces").
> 
>> instrumentation (as if LD_AUDIT implied LD_PROFILE).  However, instrumenting
>> is only necessary if one of audit libraries provides PLT (la_plt{enter,exit}
>> symbols).  Otherwise, the slowdown can be avoided.
>>
>> The following patch adjusts the logic that enables profiling to iterate
>> over all audit modules and check if any of those provides a PLT hook.
>>
>> Co-authored-by: Alexander Monakov <amonakov@ispras.dot.ru>
> 
> Please use my correct email here and in the Cc list (it's ispras.ru, not
> ispras.dot.ru).

Ack, I used the one on previous submission. I will update with the correct
one

> 
>> ---
>>  NEWS                 |  4 ++++
>>  elf/Makefile         |  6 +++++-
>>  elf/dl-reloc.c       | 12 +++++++++++-
>>  elf/rtld.c           |  8 +-------
>>  elf/tst-audit18.c    | 39 +++++++++++++++++++++++++++++++++++++++
>>  elf/tst-auditmod18.c | 24 ++++++++++++++++++++++++
>>  include/link.h       |  2 ++
>>  7 files changed, 86 insertions(+), 9 deletions(-)
>>  create mode 100644 elf/tst-audit18.c
>>  create mode 100644 elf/tst-auditmod18.c
>>
>> diff --git a/NEWS b/NEWS
>> index 13ffe627da..8fde312ec6 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -76,6 +76,10 @@ Major new features:
>>    equal to a giver integer.  This function is a GNU extension, although
>>    Solaris also provides a similar function.
>>  
>> +* The audit libraries will avoid unnecessary slowdown if it is not required
> 
> "required for" or "... by"?
> 
>> +  either PLT tracking or symbol binding profiling (enabled with LA_FLG_BINDFROM
>> +  or LA_FLG_BINDTO from la_objopen() callback).
> 
> It seems the part about binding needs to be updated, because the commit message
> and the patch itself does not change anything for binding hooks? My
> understanding always was that no change is necessary w.r.t la_symbind
> callbacks: the loader invokes them when doing symbol resolution, and presence
> of PLT does not matter.

The NEWS entry needs update indeed.  For 'la_symbind', I was trying to handle a
different issue which in turn made this very change ineffective (issue la_symbind
for bind-now).  My plan is to address it in a different patch.