[libvirt PATCH v2 0/7] Enable sanitizers

Tim Wiederhake posted 7 patches 2 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210506150838.30129-1-twiederh@redhat.com
Test syntax-check failed
.gitlab-ci.yml            | 35 +++++++++++++++++++++++++++++++++++
build-aux/syntax-check.mk |  2 +-
meson.build               | 14 ++++++++++----
src/libvirt_openvz.syms   |  2 ++
src/security/meson.build  |  1 -
tests/meson.build         |  2 +-
tests/virfilemock.c       | 20 ++++++++++++--------
7 files changed, 61 insertions(+), 15 deletions(-)
[libvirt PATCH v2 0/7] Enable sanitizers
Posted by Tim Wiederhake 2 years, 11 months ago
This series enables and adds AddressSanitizer and UndefinedBehaviorSanitizer
builds to the CI.

See:
https://clang.llvm.org/docs/AddressSanitizer.html and
https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html

These sanitizers already found some issues in libvirt, e.g.
4eb7c621985dad4de911ec394ac628bd1a5b29ab,
1294de209cee6643511265c7e2d4283c047cf652,
8b8c91f487592c6c067847ca59dde405ca17573f, or
1c34211c22de28127a509edbf2cf2f44cb0d891e.

There exist two more relevant sanitizers, ThreadSanitizer and MemorySanitizer.
Unfortunately, those two require an instrumented build of all dependencies,
including libc, to work correctly.

Note that clang and gcc have different implementations of these sanitizers,
hence the introduction of two new jobs to the CI. The latter one issues a
warning about the use of LD_PRELOAD in `virTestMain`, which in this
particular case can be safely ignored by setting `ASAN_OPTIONS` to
verify_asan_link_order=0` for the gcc build.

Changes since V1:

Incorporated changes suggested by Pavel, except for #6 (now #7): The statement
in https://listman.redhat.com/archives/libvir-list/2021-May/msg00070.html on
the sanitizers working with Fedora 33 is wrong, I was fooled by caching. The
bug described there is present in Fedora 33, 34, and Rawhide.

Cheers,
Tim

Tim Wiederhake (7):
  meson: Allow larger stack frames when instrumenting
  meson: Allow undefined symbols when sanitizers are enabled
  tests: virfilemock: realpath: Allow non-null second parameter
  openvz: Add missing symbols to libvirt_openvz.syms
  tests: openvzutilstest: Remove duplicate linking with libvirt_openvz.a
  virt-aa-helper: Remove duplicate linking with src/datatypes.o
  ci: Enable address and undefined behavior sanitizers

 .gitlab-ci.yml            | 35 +++++++++++++++++++++++++++++++++++
 build-aux/syntax-check.mk |  2 +-
 meson.build               | 14 ++++++++++----
 src/libvirt_openvz.syms   |  2 ++
 src/security/meson.build  |  1 -
 tests/meson.build         |  2 +-
 tests/virfilemock.c       | 20 ++++++++++++--------
 7 files changed, 61 insertions(+), 15 deletions(-)

-- 
2.26.3


Re: [libvirt PATCH v2 0/7] Enable sanitizers
Posted by Tim Wiederhake 2 years, 11 months ago
Ping.

On Thu, 2021-05-06 at 17:08 +0200, Tim Wiederhake wrote:
> This series enables and adds AddressSanitizer and
> UndefinedBehaviorSanitizer
> builds to the CI.
> 
> See:
> https://clang.llvm.org/docs/AddressSanitizer.html and
> https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html
> 
> These sanitizers already found some issues in libvirt, e.g.
> 4eb7c621985dad4de911ec394ac628bd1a5b29ab,
> 1294de209cee6643511265c7e2d4283c047cf652,
> 8b8c91f487592c6c067847ca59dde405ca17573f, or
> 1c34211c22de28127a509edbf2cf2f44cb0d891e.
> 
> There exist two more relevant sanitizers, ThreadSanitizer and
> MemorySanitizer.
> Unfortunately, those two require an instrumented build of all
> dependencies,
> including libc, to work correctly.
> 
> Note that clang and gcc have different implementations of these
> sanitizers,
> hence the introduction of two new jobs to the CI. The latter one
> issues a
> warning about the use of LD_PRELOAD in `virTestMain`, which in this
> particular case can be safely ignored by setting `ASAN_OPTIONS` to
> verify_asan_link_order=0` for the gcc build.
> 
> Changes since V1:
> 
> Incorporated changes suggested by Pavel, except for #6 (now #7): The
> statement
> in 
> https://listman.redhat.com/archives/libvir-list/2021-May/msg00070.html
> on
> the sanitizers working with Fedora 33 is wrong, I was fooled by
> caching. The
> bug described there is present in Fedora 33, 34, and Rawhide.
> 
> Cheers,
> Tim
> 
> Tim Wiederhake (7):
>   meson: Allow larger stack frames when instrumenting
>   meson: Allow undefined symbols when sanitizers are enabled
>   tests: virfilemock: realpath: Allow non-null second parameter
>   openvz: Add missing symbols to libvirt_openvz.syms
>   tests: openvzutilstest: Remove duplicate linking with
> libvirt_openvz.a
>   virt-aa-helper: Remove duplicate linking with src/datatypes.o
>   ci: Enable address and undefined behavior sanitizers
> 
>  .gitlab-ci.yml            | 35 +++++++++++++++++++++++++++++++++++
>  build-aux/syntax-check.mk |  2 +-
>  meson.build               | 14 ++++++++++----
>  src/libvirt_openvz.syms   |  2 ++
>  src/security/meson.build  |  1 -
>  tests/meson.build         |  2 +-
>  tests/virfilemock.c       | 20 ++++++++++++--------
>  7 files changed, 61 insertions(+), 15 deletions(-)
> 
> -- 
> 2.26.3
> 
> 

Re: [libvirt PATCH v2 0/7] Enable sanitizers
Posted by Tim Wiederhake 2 years, 10 months ago
Ping

On Tue, 2021-05-18 at 10:41 +0200, Tim Wiederhake wrote:
> Ping.
> 
> On Thu, 2021-05-06 at 17:08 +0200, Tim Wiederhake wrote:
> > This series enables and adds AddressSanitizer and
> > UndefinedBehaviorSanitizer
> > builds to the CI.
> > 
> > See:
> > https://clang.llvm.org/docs/AddressSanitizer.html and
> > https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html
> > 
> > These sanitizers already found some issues in libvirt, e.g.
> > 4eb7c621985dad4de911ec394ac628bd1a5b29ab,
> > 1294de209cee6643511265c7e2d4283c047cf652,
> > 8b8c91f487592c6c067847ca59dde405ca17573f, or
> > 1c34211c22de28127a509edbf2cf2f44cb0d891e.
> > 
> > There exist two more relevant sanitizers, ThreadSanitizer and
> > MemorySanitizer.
> > Unfortunately, those two require an instrumented build of all
> > dependencies,
> > including libc, to work correctly.
> > 
> > Note that clang and gcc have different implementations of these
> > sanitizers,
> > hence the introduction of two new jobs to the CI. The latter one
> > issues a
> > warning about the use of LD_PRELOAD in `virTestMain`, which in this
> > particular case can be safely ignored by setting `ASAN_OPTIONS` to
> > verify_asan_link_order=0` for the gcc build.
> > 
> > Changes since V1:
> > 
> > Incorporated changes suggested by Pavel, except for #6 (now #7):
> > The
> > statement
> > in 
> > https://listman.redhat.com/archives/libvir-list/2021-May/msg00070.html
> > on
> > the sanitizers working with Fedora 33 is wrong, I was fooled by
> > caching. The
> > bug described there is present in Fedora 33, 34, and Rawhide.
> > 
> > Cheers,
> > Tim
> > 
> > Tim Wiederhake (7):
> >   meson: Allow larger stack frames when instrumenting
> >   meson: Allow undefined symbols when sanitizers are enabled
> >   tests: virfilemock: realpath: Allow non-null second parameter
> >   openvz: Add missing symbols to libvirt_openvz.syms
> >   tests: openvzutilstest: Remove duplicate linking with
> > libvirt_openvz.a
> >   virt-aa-helper: Remove duplicate linking with src/datatypes.o
> >   ci: Enable address and undefined behavior sanitizers
> > 
> >  .gitlab-ci.yml            | 35 +++++++++++++++++++++++++++++++++++
> >  build-aux/syntax-check.mk |  2 +-
> >  meson.build               | 14 ++++++++++----
> >  src/libvirt_openvz.syms   |  2 ++
> >  src/security/meson.build  |  1 -
> >  tests/meson.build         |  2 +-
> >  tests/virfilemock.c       | 20 ++++++++++++--------
> >  7 files changed, 61 insertions(+), 15 deletions(-)
> > 
> > -- 
> > 2.26.3
> > 
> > 


Re: [libvirt PATCH v2 0/7] Enable sanitizers
Posted by Michal Prívozník 2 years, 10 months ago
On 5/6/21 5:08 PM, Tim Wiederhake wrote:
> This series enables and adds AddressSanitizer and UndefinedBehaviorSanitizer
> builds to the CI.
> 
> See:
> https://clang.llvm.org/docs/AddressSanitizer.html and
> https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html
> 
> These sanitizers already found some issues in libvirt, e.g.
> 4eb7c621985dad4de911ec394ac628bd1a5b29ab,
> 1294de209cee6643511265c7e2d4283c047cf652,
> 8b8c91f487592c6c067847ca59dde405ca17573f, or
> 1c34211c22de28127a509edbf2cf2f44cb0d891e.
> 
> There exist two more relevant sanitizers, ThreadSanitizer and MemorySanitizer.
> Unfortunately, those two require an instrumented build of all dependencies,
> including libc, to work correctly.
> 
> Note that clang and gcc have different implementations of these sanitizers,
> hence the introduction of two new jobs to the CI. The latter one issues a
> warning about the use of LD_PRELOAD in `virTestMain`, which in this
> particular case can be safely ignored by setting `ASAN_OPTIONS` to
> verify_asan_link_order=0` for the gcc build.
> 
> Changes since V1:
> 
> Incorporated changes suggested by Pavel, except for #6 (now #7): The statement
> in https://listman.redhat.com/archives/libvir-list/2021-May/msg00070.html on
> the sanitizers working with Fedora 33 is wrong, I was fooled by caching. The
> bug described there is present in Fedora 33, 34, and Rawhide.
> 
> Cheers,
> Tim
> 
> Tim Wiederhake (7):
>   meson: Allow larger stack frames when instrumenting
>   meson: Allow undefined symbols when sanitizers are enabled
>   tests: virfilemock: realpath: Allow non-null second parameter
>   openvz: Add missing symbols to libvirt_openvz.syms
>   tests: openvzutilstest: Remove duplicate linking with libvirt_openvz.a
>   virt-aa-helper: Remove duplicate linking with src/datatypes.o
>   ci: Enable address and undefined behavior sanitizers
> 
>  .gitlab-ci.yml            | 35 +++++++++++++++++++++++++++++++++++
>  build-aux/syntax-check.mk |  2 +-
>  meson.build               | 14 ++++++++++----
>  src/libvirt_openvz.syms   |  2 ++
>  src/security/meson.build  |  1 -
>  tests/meson.build         |  2 +-
>  tests/virfilemock.c       | 20 ++++++++++++--------
>  7 files changed, 61 insertions(+), 15 deletions(-)
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and pushed.

Michal