Re: [PATCH v3] meson: Use -fno-sanitize=function when available

[Richard Henderson <richard.henderson@linaro.org>]

On 8/16/24 18:27, Akihiko Odaki wrote:
> On 2024/08/16 17:24, Thomas Huth wrote:
>> On 16/08/2024 10.21, Akihiko Odaki wrote:
>>> On 2024/08/16 17:03, Thomas Huth wrote:
>>>> On 16/08/2024 09.30, Akihiko Odaki wrote:
>>>>> On 2024/08/16 16:27, Thomas Huth wrote:
>>>>>> On 16/08/2024 09.12, Akihiko Odaki wrote:
>>>>>>> On 2024/08/16 16:03, Thomas Huth wrote:
>>>>>>>> On 16/08/2024 08.22, Akihiko Odaki wrote:
>>>>>>>>> Commit 23ef50ae2d0c (".gitlab-ci.d/buildtest.yml: Use
>>>>>>>>> -fno-sanitize=function in the clang-system job") adds
>>>>>>>>> -fno-sanitize=function for the CI but doesn't add the flag in the
>>>>>>>>> other context. Add it to meson.build for such. It is not removed from
>>>>>>>>> .gitlab-ci.d/buildtest.yml because -fno-sanitize=function in meson.build
>>>>>>>>> does not affect --extra-cflags due to argument ordering.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>>>>>>>> ---
>>>>>>>>> Changes in v3:
>>>>>>>>> - I was not properly dropping the change of .gitlab-ci.d/buildtest.yml
>>>>>>>>>    but only updated the message. v3 fixes this. (Thomas Huth)
>>>>>>>>> - Link to v2: https://lore.kernel.org/r/20240729-function- 
>>>>>>>>> v2-1-2401ab18b30b@daynix.com
>>>>>>>>>
>>>>>>>>> Changes in v2:
>>>>>>>>> - Dropped the change of: .gitlab-ci.d/buildtest.yml
>>>>>>>>> - Link to v1: https://lore.kernel.org/r/20240714-function-v1-1- 
>>>>>>>>> cc2acb4171ba@daynix.com
>>>>>>>>> ---
>>>>>>>>>   meson.build | 1 +
>>>>>>>>>   1 file changed, 1 insertion(+)
>>>>>>>>>
>>>>>>>>> diff --git a/meson.build b/meson.build
>>>>>>>>> index 5613b62a4f42..a4169c572ba9 100644
>>>>>>>>> --- a/meson.build
>>>>>>>>> +++ b/meson.build
>>>>>>>>> @@ -609,6 +609,7 @@ if host_os != 'openbsd' and \
>>>>>>>>>   endif
>>>>>>>>>   qemu_common_flags += cc.get_supported_arguments(hardening_flags)
>>>>>>>>> +qemu_common_flags += cc.get_supported_arguments('-fno-sanitize=function')
>>>>>>>>
>>>>>>>> As I mentioned in my last mail: I think it would make sense to move this at the 
>>>>>>>> end of the "if get_option('tsan')" block in meson.build, since this apparently 
>>>>>>>> only fixes the use of "--enable-sanitizers", and cannot fix the "--extra-cflags" 
>>>>>>>> that a user might have specified?
>>>>>>>
>>>>>>> Sorry, I missed it. It cannot fix --extra-cflags, but it should be able to fix 
>>>>>>> compiler flags specified by compiler distributor.
>>>>>>
>>>>>> Oh, you mean that there are distros that enable -fsanitize=function by default? Can 
>>>>>> you name one? If so, I think that information should go into the patch description...?
>>>>>
>>>>> No, it is just a precaution.
>>>>
>>>> Ok. I don't think any normal distro will enable this by default since this impacts 
>>>> performance of the programs, so it's either the user specifying --enable-sanitizers or 
>>>> the user specifying --extra-cflags="-fsanitize=...". In the latter case, your patch 
>>>> does not help. In the former case, I think this setting should go into the same code 
>>>> block as where we set -fsanitize=undefined in our meson.build file, so that it is 
>>>> clear where it belongs to.
>>>
>>> It does not look like -fno-sanitize=function belongs to the code block to me. Putting - 
>>> fno-sanitize=function in the code block will make it seem to say that we should disable 
>>> function sanitizer because the user requests to enable sanitizers, which makes little 
>>> sense.
>>
>> As far as I understood, -fsanitize=undefine turns on -fsanitize=function, too, or did I 
>> get that wrong?
>> If not, how did you run into this problem? How did you enable the function sanitizer if 
>> not using --enable-sanitizers ?
> 
> The point is we don't care who enables sanitizers, and unconditonally setting -fno- 
> sanitize=function will clarify that.
> 

Argument ordering is important.  You cannot just drop this in the middle of meson.build 
and expect anything reasonable to happen.


r~