diff mbox series

gitlab-ci: Use -fno-sanitize=function in the clang-user job

Message ID 20240723232543.18093-1-richard.henderson@linaro.org (mailing list archive)
State New, archived
Headers show
Series gitlab-ci: Use -fno-sanitize=function in the clang-user job | expand

Commit Message

Richard Henderson July 23, 2024, 11:25 p.m. UTC
With -fsanitize=undefined, which implies -fsanitize=function,
clang will add a "type signature" before functions.
It accesses funcptr-8 and funcptr-4 to do so.

The generated TCG prologue is directly on a page boundary,
so these accesses segfault.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---

Does anyone know why we're using --extra-cflags for the clang-user
and clang-system jobs, as opposed to --enable-sanitizers?  It
certainly seems like regular users who use the normal configure
flag are going to run into this as well.

Anyway, this is why the clang-user job is failing at the momemnt.
I can only assume that changes to our docker file, or upstream
distro updates have pulled in a new compiler version, because this
wasn't failing in this way last week.


r~

---
 .gitlab-ci.d/buildtest.yml | 1 +
 1 file changed, 1 insertion(+)

Comments

Thomas Huth July 24, 2024, 6:08 a.m. UTC | #1
On 24/07/2024 01.25, Richard Henderson wrote:
> With -fsanitize=undefined, which implies -fsanitize=function,
> clang will add a "type signature" before functions.
> It accesses funcptr-8 and funcptr-4 to do so.
> 
> The generated TCG prologue is directly on a page boundary,
> so these accesses segfault.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

What happend to Akihiko Odaki's more generic patch:

 
https://lore.kernel.org/qemu-devel/20240714-function-v1-1-cc2acb4171ba@daynix.com/

?

Paolo, you mentioned that you'd queue it, did this patch fell through the 
cracks?

> Does anyone know why we're using --extra-cflags for the clang-user
> and clang-system jobs, as opposed to --enable-sanitizers?

I don't remember, but it was likely the -fno-sanitize-recover=undefined I guess.

>  It
> certainly seems like regular users who use the normal configure
> flag are going to run into this as well.

Yes, we should merge Akihiko Odaki's patch for this reason.

  Thomas
Daniel P. Berrangé July 24, 2024, 10:52 a.m. UTC | #2
On Wed, Jul 24, 2024 at 09:25:42AM +1000, Richard Henderson wrote:
> With -fsanitize=undefined, which implies -fsanitize=function,
> clang will add a "type signature" before functions.
> It accesses funcptr-8 and funcptr-4 to do so.
> 
> The generated TCG prologue is directly on a page boundary,
> so these accesses segfault.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> 
> Does anyone know why we're using --extra-cflags for the clang-user
> and clang-system jobs, as opposed to --enable-sanitizers?  It
> certainly seems like regular users who use the normal configure
> flag are going to run into this as well.
> 
> Anyway, this is why the clang-user job is failing at the momemnt.
> I can only assume that changes to our docker file, or upstream
> distro updates have pulled in a new compiler version, because this
> wasn't failing in this way last week.

Logs show the clang version didn't change, but it is possible the
libubsan.so package changed, but we can't see package versions.

I've sent a series that will make it easier to compare pacakge
versions between new & historical jobs in future situations like
this:

  https://lists.nongnu.org/archive/html/qemu-devel/2024-07/msg05749.html


With regards,
Daniel
Richard Henderson July 24, 2024, 10:03 p.m. UTC | #3
On 7/24/24 20:52, Daniel P. Berrangé wrote:
> On Wed, Jul 24, 2024 at 09:25:42AM +1000, Richard Henderson wrote:
>> With -fsanitize=undefined, which implies -fsanitize=function,
>> clang will add a "type signature" before functions.
>> It accesses funcptr-8 and funcptr-4 to do so.
>>
>> The generated TCG prologue is directly on a page boundary,
>> so these accesses segfault.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>
>> Does anyone know why we're using --extra-cflags for the clang-user
>> and clang-system jobs, as opposed to --enable-sanitizers?  It
>> certainly seems like regular users who use the normal configure
>> flag are going to run into this as well.
>>
>> Anyway, this is why the clang-user job is failing at the momemnt.
>> I can only assume that changes to our docker file, or upstream
>> distro updates have pulled in a new compiler version, because this
>> wasn't failing in this way last week.
> 
> Logs show the clang version didn't change, but it is possible the
> libubsan.so package changed, but we can't see package versions.

But the code generation definitely did -- the segv is right at the indirect function call 
to the (generated) tcg prologue.  The library is not involved at all.


> I've sent a series that will make it easier to compare pacakge
> versions between new & historical jobs in future situations like
> this:
> 
>    https://lists.nongnu.org/archive/html/qemu-devel/2024-07/msg05749.html

Nice.


r~
Richard Henderson July 25, 2024, 11:33 p.m. UTC | #4
On 7/24/24 16:08, Thomas Huth wrote:
> On 24/07/2024 01.25, Richard Henderson wrote:
>> With -fsanitize=undefined, which implies -fsanitize=function,
>> clang will add a "type signature" before functions.
>> It accesses funcptr-8 and funcptr-4 to do so.
>>
>> The generated TCG prologue is directly on a page boundary,
>> so these accesses segfault.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> 
> What happend to Akihiko Odaki's more generic patch:
> 
> 
> https://lore.kernel.org/qemu-devel/20240714-function-v1-1-cc2acb4171ba@daynix.com/
> 
> ?

This patch does not work:

https://gitlab.com/qemu-project/qemu/-/jobs/7432239478/viewer#L4956

I presume this is an argument ordering issue vs --extra-cflags.


r~
Thomas Huth July 26, 2024, 9:09 a.m. UTC | #5
On 26/07/2024 01.33, Richard Henderson wrote:
> On 7/24/24 16:08, Thomas Huth wrote:
>> On 24/07/2024 01.25, Richard Henderson wrote:
>>> With -fsanitize=undefined, which implies -fsanitize=function,
>>> clang will add a "type signature" before functions.
>>> It accesses funcptr-8 and funcptr-4 to do so.
>>>
>>> The generated TCG prologue is directly on a page boundary,
>>> so these accesses segfault.
>>>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>
>> What happend to Akihiko Odaki's more generic patch:
>>
>>
>> https://lore.kernel.org/qemu-devel/20240714-function-v1-1-cc2acb4171ba@daynix.com/
>>
>> ?
> 
> This patch does not work:
> 
> https://gitlab.com/qemu-project/qemu/-/jobs/7432239478/viewer#L4956
> 
> I presume this is an argument ordering issue vs --extra-cflags.

Ok, then we should definitely go with your patch to fix the job now. ... and 
I just saw that you already applied it 
diff mbox series

Patch

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index e3a0758bd9..aa32782405 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -444,6 +444,7 @@  clang-user:
     CONFIGURE_ARGS: --cc=clang --cxx=clang++ --disable-system
       --target-list-exclude=alpha-linux-user,microblazeel-linux-user,aarch64_be-linux-user,i386-linux-user,m68k-linux-user,mipsn32el-linux-user,xtensaeb-linux-user
       --extra-cflags=-fsanitize=undefined --extra-cflags=-fno-sanitize-recover=undefined
+      --extra-cflags=-fno-sanitize=function
     MAKE_CHECK_ARGS: check-unit check-tcg
 
 # Set LD_JOBS=1 because this requires LTO and ld consumes a large amount of memory.