Re: [PATCH v2 2/4] Consolidate stdio-lock.h

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

On 27/04/2022 10:25, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> diff --git a/sysdeps/generic/stdio-lock.h b/sysdeps/generic/stdio-lock.h
>> index 14cf458bdd..fd61f0b5b7 100644
>> --- a/sysdeps/generic/stdio-lock.h
>> +++ b/sysdeps/generic/stdio-lock.h
>> @@ -45,20 +45,13 @@ __libc_lock_define_recursive (typedef, _IO_lock_t)
>>  #define _IO_cleanup_region_end(_doit) \
>>    __libc_cleanup_region_end (_doit)
>>  
>> -#if defined _LIBC && IS_IN (libc)
>> -
>> -# ifdef __EXCEPTIONS
>> -# define _IO_acquire_lock(_fp) \
>> +#define _IO_acquire_lock(_fp) \
>>    do {									      \
>> -    FILE *_IO_acquire_lock_file						      \
>> -	__attribute__((cleanup (_IO_acquire_lock_fct)))			      \
>> -	= (_fp);							      \
>> -    _IO_flockfile (_IO_acquire_lock_file);
>> -# else
>> -#  define _IO_acquire_lock(_fp) _IO_acquire_lock_needs_exceptions_enabled
>> -# endif
>> -# define _IO_release_lock(_fp) ; } while (0)
>> -
>> -#endif
>> +    _IO_cleanup_region_start((void (*) (void *)) &_IO_funlockfile, _fp);      \
>> +    _IO_flockfile (_fp);
>> +#define _IO_release_lock(_fp) \
>> +    _IO_funlockfile (_fp);						      \
>> +    _IO_cleanup_region_end (0);						      \
>> +  } while (0)
>>  
>>  #endif /* stdio-lock.h */
> 
> I think this change replaces unwind tables for -fexceptions builds.  If
> GCC can't turn the indirect call to the unlock function into a direct
> call, this will result in a loss of hardening due to the additional
> indirect function call.
> 
> This change may also lose C++ unwinding compatibility for some
> fopencookie use cases, I think.

This is an internal header where if __EXCEPTIONS is not defined we will
get a compiler error because of an undefined symbol
(_IO_acquire_lock_needs_exceptions_enabled).  So internally all
_IO_acquire_lock usage already requires __EXCEPTIONS, so the fallback
is just unused definitions.