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

[Thomas Huth <thuth@redhat.com>]

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 ?

  Thomas