meson.build | 1 + 1 file changed, 1 insertion(+)
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')
add_global_arguments(qemu_common_flags, native: false, language: all_languages)
add_global_link_arguments(qemu_ldflags, native: false, language: all_languages)
---
base-commit: 93b799fafd9170da3a79a533ea6f73a18de82e22
change-id: 20240714-function-7d32c723abbc
Best regards,
--
Akihiko Odaki <akihiko.odaki@daynix.com>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?
Thomas
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.
Regards,
Akihiko Odaki
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...?
Thomas
On 8/16/24 17: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...?
AFAICS, -fsanitize=function is enabled by -fsanitize=undefined.
Anyway, see also
https://lore.kernel.org/qemu-devel/20240813095216.306555-1-richard.henderson@linaro.org/
r~
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.
Regards,
Akihiko Odaki
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.
Thomas
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.
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
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.
On 16/08/2024 10.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.
I think I tend to disagree here. If users enabled saitize=undefined it with
--extra-cflags, they also have to disable sanitize=function with
--extra-cflags, so listing this unconditionally in our meson.build is a red
herring when people are looking at the file.
Thomas
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~
On 2024/08/16 17:46, Richard Henderson wrote:
> 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.
That is a good point. We should add -fno-sanitize=function immediately
after -fsanitize=undefined; I will submit v4 with that change.
Regards,
Akihiko Odaki
© 2016 - 2024 Red Hat, Inc.