diff mbox series

automation: enable UBSAN for debug tests

Message ID alpine.DEB.2.22.394.2502051756210.619090@ubuntu-linux-20-04-desktop (mailing list archive)
State New
Headers show
Series automation: enable UBSAN for debug tests | expand

Commit Message

Stefano Stabellini Feb. 6, 2025, 2:37 a.m. UTC
automation: enable UBSAN for debug tests

Enable CONFIG_UBSAN and CONFIG_UBSAN_FATAL for the ARM64 and x86_64
build jobs, with debug enabled, which are later used for Xen tests on
QEMU and/or real hardware.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
---
successful pipeline: https://gitlab.com/xen-project/people/sstabellini/xen/-/pipelines/1657961377

Comments

Michal Orzel Feb. 6, 2025, 7:46 a.m. UTC | #1
On 06/02/2025 03:37, Stefano Stabellini wrote:
> 
> 
> automation: enable UBSAN for debug tests
> 
> Enable CONFIG_UBSAN and CONFIG_UBSAN_FATAL for the ARM64 and x86_64
> build jobs, with debug enabled, which are later used for Xen tests on
> QEMU and/or real hardware.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

However, I do remember Julien being opposed to this approach in the past, mostly because he did not like
the idea of failing on first UB that can possibly hide next UBs (I don't see this as a problem because other
UBs will simply be found on the next pipeline or locally when testing the fix).

~Michal
Stefano Stabellini Feb. 6, 2025, 11:03 p.m. UTC | #2
On Thu, 6 Feb 2025, Orzel, Michal wrote:
> On 06/02/2025 03:37, Stefano Stabellini wrote:
> > 
> > 
> > automation: enable UBSAN for debug tests
> > 
> > Enable CONFIG_UBSAN and CONFIG_UBSAN_FATAL for the ARM64 and x86_64
> > build jobs, with debug enabled, which are later used for Xen tests on
> > QEMU and/or real hardware.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>

Thanks!


> However, I do remember Julien being opposed to this approach in the past, mostly because he did not like
> the idea of failing on first UB that can possibly hide next UBs (I don't see this as a problem because other
> UBs will simply be found on the next pipeline or locally when testing the fix).

That may have been a problem in the past, but it is no longer an issue
now that the pipeline is fully operational with UBSAN enabled on both
ARM and x86.

Andrew also mentioned in chat that he supports enabling UBSAN in the
pipeline as soon as possible.

Since the pipeline remains green with UBSAN enabled and is not expected
to suddenly go red before the release, I am requesting a release
ack from Oleksii.

Cheers,

Stefano
Oleksii Kurochko Feb. 7, 2025, 8:28 a.m. UTC | #3
On 2/7/25 12:03 AM, Stefano Stabellini wrote:
> On Thu, 6 Feb 2025, Orzel, Michal wrote:
>> On 06/02/2025 03:37, Stefano Stabellini wrote:
>>>
>>> automation: enable UBSAN for debug tests
>>>
>>> Enable CONFIG_UBSAN and CONFIG_UBSAN_FATAL for the ARM64 and x86_64
>>> build jobs, with debug enabled, which are later used for Xen tests on
>>> QEMU and/or real hardware.
>>>
>>> Signed-off-by: Stefano Stabellini<stefano.stabellini@amd.com>
>> Reviewed-by: Michal Orzel<michal.orzel@amd.com>
> Thanks!
>
>
>> However, I do remember Julien being opposed to this approach in the past, mostly because he did not like
>> the idea of failing on first UB that can possibly hide next UBs (I don't see this as a problem because other
>> UBs will simply be found on the next pipeline or locally when testing the fix).
> That may have been a problem in the past, but it is no longer an issue
> now that the pipeline is fully operational with UBSAN enabled on both
> ARM and x86.
>
> Andrew also mentioned in chat that he supports enabling UBSAN in the
> pipeline as soon as possible.
>
> Since the pipeline remains green with UBSAN enabled and is not expected
> to suddenly go red before the release, I am requesting a release
> ack from Oleksii.

Agree, enabling UBSAN support in the pipeline is a good idea, so:
   R-Acked-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>

~ Oleksii

>
> Cheers,
>
> Stefano
Andrew Cooper Feb. 7, 2025, 9:27 a.m. UTC | #4
On 06/02/2025 2:37 am, Stefano Stabellini wrote:
> automation: enable UBSAN for debug tests
>
> Enable CONFIG_UBSAN and CONFIG_UBSAN_FATAL for the ARM64 and x86_64
> build jobs, with debug enabled, which are later used for Xen tests on
> QEMU and/or real hardware.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, but aren't you
missing two builds?
Stefano Stabellini Feb. 7, 2025, 11:05 p.m. UTC | #5
On Fri, 7 Feb 2025, Andrew Cooper wrote:
> On 06/02/2025 2:37 am, Stefano Stabellini wrote:
> > automation: enable UBSAN for debug tests
> >
> > Enable CONFIG_UBSAN and CONFIG_UBSAN_FATAL for the ARM64 and x86_64
> > build jobs, with debug enabled, which are later used for Xen tests on
> > QEMU and/or real hardware.
> >
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, but aren't you
> missing two builds?

Thanks Andrew.

Looking at test.yaml in details, this is the list of debug Xen builds
for ARM64 and x86_64 without UBSAN enabled:

alpine-3.18-gcc-debug-arm64-staticmem
alpine-3.18-gcc-debug-arm64-static-shared-mem
alpine-3.18-gcc-debug-arm64-earlyprintk
debian-12-x86_64-gcc-debug
debian-12-x86_64-clang-debug

Do you think we should enable UBSAN in all of them? I am fine with that.
So far, I have only targeted the two that are used more widely.
Andrew Cooper Feb. 7, 2025, 11:06 p.m. UTC | #6
On 07/02/2025 11:05 pm, Stefano Stabellini wrote:
> On Fri, 7 Feb 2025, Andrew Cooper wrote:
>> On 06/02/2025 2:37 am, Stefano Stabellini wrote:
>>> automation: enable UBSAN for debug tests
>>>
>>> Enable CONFIG_UBSAN and CONFIG_UBSAN_FATAL for the ARM64 and x86_64
>>> build jobs, with debug enabled, which are later used for Xen tests on
>>> QEMU and/or real hardware.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, but aren't you
>> missing two builds?
> Thanks Andrew.
>
> Looking at test.yaml in details, this is the list of debug Xen builds
> for ARM64 and x86_64 without UBSAN enabled:
>
> alpine-3.18-gcc-debug-arm64-staticmem
> alpine-3.18-gcc-debug-arm64-static-shared-mem
> alpine-3.18-gcc-debug-arm64-earlyprintk
> debian-12-x86_64-gcc-debug
> debian-12-x86_64-clang-debug
>
> Do you think we should enable UBSAN in all of them? I am fine with that.
> So far, I have only targeted the two that are used more widely.

I was referring to RISC-V and PPC.

I've done a series enabling UBSAN in RISC-V.   I'm currently fixing bugs
in ARM, and PPC is unknown because the console is broken for some reason
I can't fathom.

~Andrew
diff mbox series

Patch

diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
index bc4a8a5ad2..fb55d4ce55 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -333,6 +333,8 @@  alpine-3.18-gcc-debug:
       CONFIG_EXPERT=y
       CONFIG_UNSUPPORTED=y
       CONFIG_ARGO=y
+      CONFIG_UBSAN=y
+      CONFIG_UBSAN_FATAL=y
 
 debian-12-x86_64-gcc-debug:
   extends: .gcc-x86-64-build-debug
@@ -419,6 +421,9 @@  alpine-3.18-gcc-debug-arm64:
   extends: .gcc-arm64-build-debug
   variables:
     CONTAINER: alpine:3.18-arm64v8
+    EXTRA_XEN_CONFIG: |
+      CONFIG_UBSAN=y
+      CONFIG_UBSAN_FATAL=y
 
 alpine-3.18-gcc-arm64-randconfig:
   extends: .gcc-arm64-build