[dpdk-dev] [PATCH 0/3] Experiment ASAN in GHA

[dpdk-dev] [PATCH 0/3] Experiment ASAN in GHA

[David Marchand]

Following fixes for ASAN, I added ASAN in GHA to see where we stand.
Here are two fixes that can be merged.

The last patch enables ASAN, but as we can see, there are still some
unit tests who fails because of ASAN:
- multiprocess is broken, we could flag the ut requiring/testing mp,
- hash tests have a lot of issues, I suspect bugs in the hash library,


-- 
David Marchand

David Marchand (3):
  bus/vmbus: fix leak on device scan
  test/latencystats: fix incorrect loop boundary
  ci: run unit tests with ASAN

 .ci/linux-build.sh                  | 9 ++++++++-
 app/test/test_latencystats.c        | 2 +-
 drivers/bus/vmbus/linux/vmbus_bus.c | 5 ++++-
 drivers/bus/vmbus/rte_bus_vmbus.h   | 1 +
 4 files changed, 14 insertions(+), 3 deletions(-)

-- 
2.23.0

[dpdk-dev] [PATCH 1/3] bus/vmbus: fix leak on device scan

[David Marchand]

Caught running ASAN.

The device name is leaked on scan.
rte_device name field being a const, use the private vmbus struct to
store the device name and point at it.

Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/bus/vmbus/linux/vmbus_bus.c | 5 ++++-
 drivers/bus/vmbus/rte_bus_vmbus.h   | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c b/drivers/bus/vmbus/linux/vmbus_bus.c
index 3c924eee14..d8eb07d398 100644
--- a/drivers/bus/vmbus/linux/vmbus_bus.c
+++ b/drivers/bus/vmbus/linux/vmbus_bus.c
@@ -242,7 +242,7 @@ vmbus_scan_one(const char *name)
 		return -1;
 
 	dev->device.bus = &rte_vmbus_bus.bus;
-	dev->device.name = strdup(name);
+	dev->device.name = dev->name = strdup(name);
 	if (!dev->device.name)
 		goto error;
 
@@ -261,6 +261,7 @@ vmbus_scan_one(const char *name)
 
 	/* skip non-network devices */
 	if (rte_uuid_compare(dev->class_id, vmbus_nic_uuid) != 0) {
+		free(dev->name);
 		free(dev);
 		return 0;
 	}
@@ -312,6 +313,7 @@ vmbus_scan_one(const char *name)
 		} else { /* already registered */
 			VMBUS_LOG(NOTICE,
 				"%s already registered", name);
+			free(dev->name);
 			free(dev);
 		}
 		return 0;
@@ -322,6 +324,7 @@ vmbus_scan_one(const char *name)
 error:
 	VMBUS_LOG(DEBUG, "failed");
 
+	free(dev->name);
 	free(dev);
 	return -1;
 }
diff --git a/drivers/bus/vmbus/rte_bus_vmbus.h b/drivers/bus/vmbus/rte_bus_vmbus.h
index 4cf73ce815..438eb481fc 100644
--- a/drivers/bus/vmbus/rte_bus_vmbus.h
+++ b/drivers/bus/vmbus/rte_bus_vmbus.h
@@ -65,6 +65,7 @@ struct rte_vmbus_device {
 	TAILQ_ENTRY(rte_vmbus_device) next;    /**< Next probed VMBUS device */
 	const struct rte_vmbus_driver *driver; /**< Associated driver */
 	struct rte_device device;              /**< Inherit core device */
+	char *name;                            /**< Device name */
 	rte_uuid_t device_id;		       /**< VMBUS device id */
 	rte_uuid_t class_id;		       /**< VMBUS device type */
 	uint32_t relid;			       /**< id for primary */
-- 
2.23.0

[dpdk-dev] [PATCH 2/3] test/latencystats: fix incorrect loop boundary

[David Marchand]

Caught running ASAN.

lat_stats_strings[] is an array containing NUM_STATS strings.

Fixes: 1e3676a06e4c ("test/latency: add unit tests for latencystats library")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/test_latencystats.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/test_latencystats.c b/app/test/test_latencystats.c
index 427339904d..724acbc315 100644
--- a/app/test/test_latencystats.c
+++ b/app/test/test_latencystats.c
@@ -80,7 +80,7 @@ static int test_latencystats_get_names(void)
 	/* Success Test: Valid names and size */
 	size = NUM_STATS;
 	ret = rte_latencystats_get_names(names, size);
-	for (i = 0; i <= NUM_STATS; i++) {
+	for (i = 0; i < NUM_STATS; i++) {
 		if (strcmp(lat_stats_strings[i].name, names[i].name) == 0)
 			printf(" %s\n", names[i].name);
 		else
-- 
2.23.0

[dpdk-dev] [PATCH 3/3] ci: run unit tests with ASAN

[David Marchand]

Enable ASAN for clang jobs.
This can greatly help identify leaks and buffer overflows.
This patch is more a fyi, as some unit tests stil have issues.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 .ci/linux-build.sh | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index 91e43a975b..a961d9b92d 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -79,7 +79,14 @@ fi
 
 OPTS="$OPTS -Dmachine=default"
 OPTS="$OPTS --default-library=$DEF_LIB"
-OPTS="$OPTS --buildtype=debugoptimized"
+
+if [ "$CC" != "${CC%%clang}" ] && [ "$RUN_TESTS" = 'true' ]; then
+    # Let's run tests with ASAN
+    OPTS="$OPTS -Db_sanitize=address -Db_lundef=false --buildtype=debug"
+else
+    OPTS="$OPTS --buildtype=debugoptimized"
+fi
+
 OPTS="$OPTS -Dcheck_includes=true"
 meson build --werror $OPTS
 ninja -C build
-- 
2.23.0

Re: [dpdk-dev] [PATCH 2/3] test/latencystats: fix incorrect loop boundary

[Pattan, Reshma]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Caught running ASAN.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Acked-by:  Reshma Pattan <reshma.pattan@intel.com>

Re: [dpdk-dev] [PATCH 1/3] bus/vmbus: fix leak on device scan

[Long Li]

> Subject: [PATCH 1/3] bus/vmbus: fix leak on device scan
> 
> Caught running ASAN.
> 
> The device name is leaked on scan.
> rte_device name field being a const, use the private vmbus struct to store the
> device name and point at it.
> 
> Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/bus/vmbus/linux/vmbus_bus.c | 5 ++++-
>  drivers/bus/vmbus/rte_bus_vmbus.h   | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c
> b/drivers/bus/vmbus/linux/vmbus_bus.c
> index 3c924eee14..d8eb07d398 100644
> --- a/drivers/bus/vmbus/linux/vmbus_bus.c
> +++ b/drivers/bus/vmbus/linux/vmbus_bus.c
> @@ -242,7 +242,7 @@ vmbus_scan_one(const char *name)
>  		return -1;
> 
>  	dev->device.bus = &rte_vmbus_bus.bus;
> -	dev->device.name = strdup(name);
> +	dev->device.name = dev->name = strdup(name);


I suggest not to add a "name" in struct rte_vmbus_device. How about defining a local variable in this function, like:
dev->device.name = dev_name = strdup(name);

If failed, just free(dev_name).

>  	if (!dev->device.name)
>  		goto error;
> 
> @@ -261,6 +261,7 @@ vmbus_scan_one(const char *name)
> 
>  	/* skip non-network devices */
>  	if (rte_uuid_compare(dev->class_id, vmbus_nic_uuid) != 0) {
> +		free(dev->name);
>  		free(dev);
>  		return 0;
>  	}
> @@ -312,6 +313,7 @@ vmbus_scan_one(const char *name)
>  		} else { /* already registered */
>  			VMBUS_LOG(NOTICE,
>  				"%s already registered", name);
> +			free(dev->name);
>  			free(dev);
>  		}
>  		return 0;
> @@ -322,6 +324,7 @@ vmbus_scan_one(const char *name)
>  error:
>  	VMBUS_LOG(DEBUG, "failed");
> 
> +	free(dev->name);
>  	free(dev);
>  	return -1;
>  }
> diff --git a/drivers/bus/vmbus/rte_bus_vmbus.h
> b/drivers/bus/vmbus/rte_bus_vmbus.h
> index 4cf73ce815..438eb481fc 100644
> --- a/drivers/bus/vmbus/rte_bus_vmbus.h
> +++ b/drivers/bus/vmbus/rte_bus_vmbus.h
> @@ -65,6 +65,7 @@ struct rte_vmbus_device {
>  	TAILQ_ENTRY(rte_vmbus_device) next;    /**< Next probed VMBUS
> device */
>  	const struct rte_vmbus_driver *driver; /**< Associated driver */
>  	struct rte_device device;              /**< Inherit core device */
> +	char *name;                            /**< Device name */
>  	rte_uuid_t device_id;		       /**< VMBUS device id */
>  	rte_uuid_t class_id;		       /**< VMBUS device type */
>  	uint32_t relid;			       /**< id for primary */
> --
> 2.23.0

Re: [dpdk-dev] [PATCH 1/3] bus/vmbus: fix leak on device scan

[David Marchand]

On Wed, Sep 29, 2021 at 10:57 PM Long Li <longli@microsoft.com> wrote:
>
> > Subject: [PATCH 1/3] bus/vmbus: fix leak on device scan
> >
> > Caught running ASAN.
> >
> > The device name is leaked on scan.
> > rte_device name field being a const, use the private vmbus struct to store the
> > device name and point at it.
> >
> > Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  drivers/bus/vmbus/linux/vmbus_bus.c | 5 ++++-
> >  drivers/bus/vmbus/rte_bus_vmbus.h   | 1 +
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c
> > b/drivers/bus/vmbus/linux/vmbus_bus.c
> > index 3c924eee14..d8eb07d398 100644
> > --- a/drivers/bus/vmbus/linux/vmbus_bus.c
> > +++ b/drivers/bus/vmbus/linux/vmbus_bus.c
> > @@ -242,7 +242,7 @@ vmbus_scan_one(const char *name)
> >               return -1;
> >
> >       dev->device.bus = &rte_vmbus_bus.bus;
> > -     dev->device.name = strdup(name);
> > +     dev->device.name = dev->name = strdup(name);
>
>
> I suggest not to add a "name" in struct rte_vmbus_device. How about defining a local variable in this function, like:
> dev->device.name = dev_name = strdup(name);

What is your concern for storing the name?


rte_device name only points at some location where the name is stored.
In general this storage is in the bus object or (in some buses) the
devarg that resulted in the rte_device object creation.

If we won't store the name in the bus object, then we lose the ability
to release the name later.
This is probably fine as long as we never release rte_vmbus_device
objects which is the case atm.
But I don't understand why vmbus should be an exception when comparing
to other buses.


-- 
David Marchand

Re: [dpdk-dev] [PATCH 1/3] bus/vmbus: fix leak on device scan

[Long Li]

> Subject: Re: [PATCH 1/3] bus/vmbus: fix leak on device scan
> 
> On Wed, Sep 29, 2021 at 10:57 PM Long Li <longli@microsoft.com> wrote:
> >
> > > Subject: [PATCH 1/3] bus/vmbus: fix leak on device scan
> > >
> > > Caught running ASAN.
> > >
> > > The device name is leaked on scan.
> > > rte_device name field being a const, use the private vmbus struct to
> > > store the device name and point at it.
> > >
> > > Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> > >  drivers/bus/vmbus/linux/vmbus_bus.c | 5 ++++-
> > >  drivers/bus/vmbus/rte_bus_vmbus.h   | 1 +
> > >  2 files changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c
> > > b/drivers/bus/vmbus/linux/vmbus_bus.c
> > > index 3c924eee14..d8eb07d398 100644
> > > --- a/drivers/bus/vmbus/linux/vmbus_bus.c
> > > +++ b/drivers/bus/vmbus/linux/vmbus_bus.c
> > > @@ -242,7 +242,7 @@ vmbus_scan_one(const char *name)
> > >               return -1;
> > >
> > >       dev->device.bus = &rte_vmbus_bus.bus;
> > > -     dev->device.name = strdup(name);
> > > +     dev->device.name = dev->name = strdup(name);
> >
> >
> > I suggest not to add a "name" in struct rte_vmbus_device. How about
> defining a local variable in this function, like:
> > dev->device.name = dev_name = strdup(name);
> 
> What is your concern for storing the name?

This name is only used in this function. I see little value of putting it in struct rte_vmbus_device. Am I missing another patch that uses it from struct rte_vmbus_device from another place?

> 
> 
> rte_device name only points at some location where the name is stored.
> In general this storage is in the bus object or (in some buses) the devarg that
> resulted in the rte_device object creation.
> 
> If we won't store the name in the bus object, then we lose the ability to
> release the name later.
> This is probably fine as long as we never release rte_vmbus_device objects
> which is the case atm.
> But I don't understand why vmbus should be an exception when comparing
> to other buses.

I don’t understand why you want to put a name there if it's never used outside vmbus_scan_one().

Long

Re: [dpdk-dev] [PATCH 1/3] bus/vmbus: fix leak on device scan

[David Marchand]

On Thu, Sep 30, 2021 at 9:14 PM Long Li <longli@microsoft.com> wrote:
> > rte_device name only points at some location where the name is stored.
> > In general this storage is in the bus object or (in some buses) the devarg that
> > resulted in the rte_device object creation.
> >
> > If we won't store the name in the bus object, then we lose the ability to
> > release the name later.
> > This is probably fine as long as we never release rte_vmbus_device objects
> > which is the case atm.
> > But I don't understand why vmbus should be an exception when comparing
> > to other buses.
>
> I don’t understand why you want to put a name there if it's never used outside vmbus_scan_one().

Since you don't care about releasing rte_vmbus_device objects, I'll
rework as you suggest.


-- 
David Marchand

Re: [dpdk-dev] [PATCH 1/3] bus/vmbus: fix leak on device scan

[Long Li]

> Subject: Re: [PATCH 1/3] bus/vmbus: fix leak on device scan
> 
> On Thu, Sep 30, 2021 at 9:14 PM Long Li <longli@microsoft.com> wrote:
> > > rte_device name only points at some location where the name is stored.
> > > In general this storage is in the bus object or (in some buses) the
> > > devarg that resulted in the rte_device object creation.
> > >
> > > If we won't store the name in the bus object, then we lose the
> > > ability to release the name later.
> > > This is probably fine as long as we never release rte_vmbus_device
> > > objects which is the case atm.
> > > But I don't understand why vmbus should be an exception when
> > > comparing to other buses.
> >
> > I don’t understand why you want to put a name there if it's never used outside
> vmbus_scan_one().
> 
> Since you don't care about releasing rte_vmbus_device objects, I'll rework as
> you suggest.

I don't have any objection defining new data fields in the device struct, as long as it's used during the life cycle of the device.

The rest of the patch looks good, thank you.

Long

[dpdk-dev] [PATCH v2 0/3] Experiment ASAN in GHA

[David Marchand]

Following fixes for ASAN, I added ASAN in GHA to see where we stand.
Here are two fixes that can be merged.

The last patch enables ASAN, but as we can see, there are still some
unit tests who fails because of ASAN:
- multiprocess is broken, we could flag the ut requiring/testing mp,
- hash tests have a lot of issues, I suspect bugs in the hash library,
  bugs have been opened,

-- 
David Marchand

David Marchand (3):
  bus/vmbus: fix leak on device scan
  test/latencystats: fix incorrect loop boundary
  ci: run unit tests with ASAN

 .ci/linux-build.sh                  | 9 ++++++++-
 app/test/test_latencystats.c        | 2 +-
 drivers/bus/vmbus/linux/vmbus_bus.c | 6 +++++-
 3 files changed, 14 insertions(+), 3 deletions(-)

-- 
2.23.0

[dpdk-dev] [PATCH v2 1/3] bus/vmbus: fix leak on device scan

[David Marchand]

Caught running ASAN.
The device name was leaked on scan.
rte_device name field being a const, use a local pointer and release
in error path.

Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since v1:
- left rte_vmbus_device untouched,

---
 drivers/bus/vmbus/linux/vmbus_bus.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c b/drivers/bus/vmbus/linux/vmbus_bus.c
index 3c924eee14..68f6cc5742 100644
--- a/drivers/bus/vmbus/linux/vmbus_bus.c
+++ b/drivers/bus/vmbus/linux/vmbus_bus.c
@@ -236,13 +236,14 @@ vmbus_scan_one(const char *name)
 	char filename[PATH_MAX];
 	char dirname[PATH_MAX];
 	unsigned long tmp;
+	char *dev_name;
 
 	dev = calloc(1, sizeof(*dev));
 	if (dev == NULL)
 		return -1;
 
 	dev->device.bus = &rte_vmbus_bus.bus;
-	dev->device.name = strdup(name);
+	dev->device.name = dev_name = strdup(name);
 	if (!dev->device.name)
 		goto error;
 
@@ -261,6 +262,7 @@ vmbus_scan_one(const char *name)
 
 	/* skip non-network devices */
 	if (rte_uuid_compare(dev->class_id, vmbus_nic_uuid) != 0) {
+		free(dev_name);
 		free(dev);
 		return 0;
 	}
@@ -312,6 +314,7 @@ vmbus_scan_one(const char *name)
 		} else { /* already registered */
 			VMBUS_LOG(NOTICE,
 				"%s already registered", name);
+			free(dev_name);
 			free(dev);
 		}
 		return 0;
@@ -322,6 +325,7 @@ vmbus_scan_one(const char *name)
 error:
 	VMBUS_LOG(DEBUG, "failed");
 
+	free(dev_name);
 	free(dev);
 	return -1;
 }
-- 
2.23.0

[dpdk-dev] [PATCH v2 2/3] test/latencystats: fix incorrect loop boundary

[David Marchand]

Caught running ASAN.

lat_stats_strings[] is an array containing NUM_STATS strings.

Fixes: 1e3676a06e4c ("test/latency: add unit tests for latencystats library")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by:  Reshma Pattan <reshma.pattan@intel.com>
---
 app/test/test_latencystats.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/test_latencystats.c b/app/test/test_latencystats.c
index 427339904d..724acbc315 100644
--- a/app/test/test_latencystats.c
+++ b/app/test/test_latencystats.c
@@ -80,7 +80,7 @@ static int test_latencystats_get_names(void)
 	/* Success Test: Valid names and size */
 	size = NUM_STATS;
 	ret = rte_latencystats_get_names(names, size);
-	for (i = 0; i <= NUM_STATS; i++) {
+	for (i = 0; i < NUM_STATS; i++) {
 		if (strcmp(lat_stats_strings[i].name, names[i].name) == 0)
 			printf(" %s\n", names[i].name);
 		else
-- 
2.23.0

[dpdk-dev] [PATCH v2 3/3] ci: run unit tests with ASAN

[David Marchand]

Enable ASAN for clang jobs.
This can greatly help identify leaks and buffer overflows.
This patch is more a fyi, as some unit tests stil have issues.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 .ci/linux-build.sh | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index 91e43a975b..a961d9b92d 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -79,7 +79,14 @@ fi
 
 OPTS="$OPTS -Dmachine=default"
 OPTS="$OPTS --default-library=$DEF_LIB"
-OPTS="$OPTS --buildtype=debugoptimized"
+
+if [ "$CC" != "${CC%%clang}" ] && [ "$RUN_TESTS" = 'true' ]; then
+    # Let's run tests with ASAN
+    OPTS="$OPTS -Db_sanitize=address -Db_lundef=false --buildtype=debug"
+else
+    OPTS="$OPTS --buildtype=debugoptimized"
+fi
+
 OPTS="$OPTS -Dcheck_includes=true"
 meson build --werror $OPTS
 ninja -C build
-- 
2.23.0

Re: [dpdk-dev] [PATCH v2 1/3] bus/vmbus: fix leak on device scan

[Long Li]

> Subject: [PATCH v2 1/3] bus/vmbus: fix leak on device scan
> 
> Caught running ASAN.
> The device name was leaked on scan.
> rte_device name field being a const, use a local pointer and release in error path.
> 
> Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Thank you.

Acked-by: Long Li <longli@microsoft.com>

> ---
> Changes since v1:
> - left rte_vmbus_device untouched,
> 
> ---
>  drivers/bus/vmbus/linux/vmbus_bus.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c
> b/drivers/bus/vmbus/linux/vmbus_bus.c
> index 3c924eee14..68f6cc5742 100644
> --- a/drivers/bus/vmbus/linux/vmbus_bus.c
> +++ b/drivers/bus/vmbus/linux/vmbus_bus.c
> @@ -236,13 +236,14 @@ vmbus_scan_one(const char *name)
>  	char filename[PATH_MAX];
>  	char dirname[PATH_MAX];
>  	unsigned long tmp;
> +	char *dev_name;
> 
>  	dev = calloc(1, sizeof(*dev));
>  	if (dev == NULL)
>  		return -1;
> 
>  	dev->device.bus = &rte_vmbus_bus.bus;
> -	dev->device.name = strdup(name);
> +	dev->device.name = dev_name = strdup(name);
>  	if (!dev->device.name)
>  		goto error;
> 
> @@ -261,6 +262,7 @@ vmbus_scan_one(const char *name)
> 
>  	/* skip non-network devices */
>  	if (rte_uuid_compare(dev->class_id, vmbus_nic_uuid) != 0) {
> +		free(dev_name);
>  		free(dev);
>  		return 0;
>  	}
> @@ -312,6 +314,7 @@ vmbus_scan_one(const char *name)
>  		} else { /* already registered */
>  			VMBUS_LOG(NOTICE,
>  				"%s already registered", name);
> +			free(dev_name);
>  			free(dev);
>  		}
>  		return 0;
> @@ -322,6 +325,7 @@ vmbus_scan_one(const char *name)
>  error:
>  	VMBUS_LOG(DEBUG, "failed");
> 
> +	free(dev_name);
>  	free(dev);
>  	return -1;
>  }
> --
> 2.23.0

Re: [dpdk-dev] [PATCH v2 3/3] ci: run unit tests with ASAN

[Aaron Conole]

David Marchand <david.marchand@redhat.com> writes:

> Enable ASAN for clang jobs.
> This can greatly help identify leaks and buffer overflows.
> This patch is more a fyi, as some unit tests stil have issues.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

I know this isn't yet ready to be merged due to failing unit tests, and
I still support it :)

Acked-by: Aaron Conole <aconole@redhat.com>

>  .ci/linux-build.sh | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index 91e43a975b..a961d9b92d 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -79,7 +79,14 @@ fi
>  
>  OPTS="$OPTS -Dmachine=default"
>  OPTS="$OPTS --default-library=$DEF_LIB"
> -OPTS="$OPTS --buildtype=debugoptimized"
> +
> +if [ "$CC" != "${CC%%clang}" ] && [ "$RUN_TESTS" = 'true' ]; then
> +    # Let's run tests with ASAN
> +    OPTS="$OPTS -Db_sanitize=address -Db_lundef=false --buildtype=debug"
> +else
> +    OPTS="$OPTS --buildtype=debugoptimized"
> +fi
> +
>  OPTS="$OPTS -Dcheck_includes=true"
>  meson build --werror $OPTS
>  ninja -C build

Re: [dpdk-dev] [PATCH v2 0/3] Experiment ASAN in GHA

[David Marchand]

On Sat, Oct 2, 2021 at 6:24 PM David Marchand <david.marchand@redhat.com> wrote:
>
> Following fixes for ASAN, I added ASAN in GHA to see where we stand.
> Here are two fixes that can be merged.
>
> The last patch enables ASAN, but as we can see, there are still some
> unit tests who fails because of ASAN:
> - multiprocess is broken, we could flag the ut requiring/testing mp,
> - hash tests have a lot of issues, I suspect bugs in the hash library,
>
> --
> David Marchand
>
> David Marchand (3):
>   bus/vmbus: fix leak on device scan
>   test/latencystats: fix incorrect loop boundary
>   ci: run unit tests with ASAN

Series^WFirst two patches applied.
Thanks.

-- 
David Marchand