Re: [PATCH v10 1/4] elf: Add la_activity during application exit

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

On 18/01/2022 08:29, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> diff --git a/elf/dl-fini.c b/elf/dl-fini.c
>> index de8eb1b3c9..2705a15c88 100644
>> --- a/elf/dl-fini.c
>> +++ b/elf/dl-fini.c
>> @@ -64,6 +64,11 @@ _dl_fini (void)
>>  	__rtld_lock_unlock_recursive (GL(dl_load_lock));
>>        else
>>  	{
>> +#ifdef SHARED
>> +	  /* Auditing checkpoint: we will start deleting objects.  */
>> +	  _dl_audit_activity_nsid (ns, LA_ACT_DELETE);
>> +#endif
>> +
>>  	  /* Now we can allocate an array to hold all the pointers and
>>  	     copy the pointers in.  */
>>  	  struct link_map *maps[nloaded];
>> @@ -153,6 +158,11 @@ _dl_fini (void)
>>  	      /* Correct the previous increment.  */
>>  	      --l->l_direct_opencount;
>>  	    }
>> +
>> +#ifdef SHARED
>> +	  /* Auditing checkpoint: we will start deleting objects.  */
>> +	  _dl_audit_activity_nsid (ns, LA_ACT_CONSISTENT);
>> +#endif
>>  	}
>>      }
> 
> The tense in the second comment seems wrong.

Ack, in fact I think both comments do not add much (it is pretty obvious
from the function call).  I will remove them.

> 
>>  
>> diff --git a/elf/tst-audit23.c b/elf/tst-audit23.c
>> new file mode 100644
>> index 0000000000..9a7e69c1e1
>> --- /dev/null
>> +++ b/elf/tst-audit23.c
> 
>> +  FILE *out = fmemopen (result.err.buffer, result.err.length, "r");
>> +  TEST_VERIFY (out != NULL);
>> +  char *buffer = NULL;
>> +  size_t buffer_length = 0;
>> +  while (xgetline (&buffer, &buffer_length, out))
>> +    {
>> +      if (startswith (buffer, "la_activity: "))
> 
> It may be simpler (or easier to maintain going forward) to have the
> expected output (without varying pointers) and compare against that
> verbatim.  This is what I implemented for some of the DNS tests.

The main issue is to handle the vDSO (is_vdso) and match the cookies
and address with the subsequent la_activities/la_objclose.  So it
would require to keep track with some internal state anyway.

And checking on resolve/tst-ns_name.c I don't think it would be
simpler to use such scheme (specially the required handling it would 
require to parse a possible input to handle line with multiple options
and different outputs).