DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] buildtools: fix invalid symbols
@ 2024-06-27 10:11 Mingjin Ye
  2024-06-27 10:50 ` Bruce Richardson
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Mingjin Ye @ 2024-06-27 10:11 UTC (permalink / raw)
  To: dev; +Cc: Mingjin Ye, stable, Dmitry Kozlyuk

ELF files generated by higher version compilers wrap multiple
symbols prefixed with "this_pmd_name".

This patch fixes the issue by filtering invalid symbols.

Bugzilla ID: 1466
Fixes: 6c4bf8f42432 ("buildtools: add Python pmdinfogen")
Cc: stable@dpdk.org

Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
---
 buildtools/pmdinfogen.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/buildtools/pmdinfogen.py b/buildtools/pmdinfogen.py
index 2a44f17bda..6ea97caec7 100755
--- a/buildtools/pmdinfogen.py
+++ b/buildtools/pmdinfogen.py
@@ -200,7 +200,8 @@ def dump(self, file):
 def load_drivers(image):
     drivers = []
     for symbol in image.find_by_prefix("this_pmd_name"):
-        drivers.append(Driver.load(image, symbol))
+        if len(symbol.string_value) != 0:
+            drivers.append(Driver.load(image, symbol))
     return drivers
 
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] buildtools: fix invalid symbols
  2024-06-27 10:11 [PATCH] buildtools: fix invalid symbols Mingjin Ye
@ 2024-06-27 10:50 ` Bruce Richardson
  2024-06-27 12:39   ` David Marchand
  2024-06-27 11:30 ` David Marchand
  2024-07-01 10:33 ` [PATCH v2] " Mingjin Ye
  2 siblings, 1 reply; 11+ messages in thread
From: Bruce Richardson @ 2024-06-27 10:50 UTC (permalink / raw)
  To: Mingjin Ye; +Cc: dev, stable, Dmitry Kozlyuk

On Thu, Jun 27, 2024 at 10:11:44AM +0000, Mingjin Ye wrote:
> ELF files generated by higher version compilers wrap multiple
> symbols prefixed with "this_pmd_name".
> 
> This patch fixes the issue by filtering invalid symbols.
> 
> Bugzilla ID: 1466
> Fixes: 6c4bf8f42432 ("buildtools: add Python pmdinfogen")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---
>  buildtools/pmdinfogen.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/buildtools/pmdinfogen.py b/buildtools/pmdinfogen.py
> index 2a44f17bda..6ea97caec7 100755
> --- a/buildtools/pmdinfogen.py
> +++ b/buildtools/pmdinfogen.py
> @@ -200,7 +200,8 @@ def dump(self, file):
>  def load_drivers(image):
>      drivers = []
>      for symbol in image.find_by_prefix("this_pmd_name"):
> -        drivers.append(Driver.load(image, symbol))
> +        if len(symbol.string_value) != 0:
> +            drivers.append(Driver.load(image, symbol))

One small suggestion. Empty strings evaluate to boolean false, so the
condition can just be simplified to:

	if symbol.string_value:
	    drivers.append(....)

/Bruce


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] buildtools: fix invalid symbols
  2024-06-27 10:11 [PATCH] buildtools: fix invalid symbols Mingjin Ye
  2024-06-27 10:50 ` Bruce Richardson
@ 2024-06-27 11:30 ` David Marchand
  2024-07-01 10:33 ` [PATCH v2] " Mingjin Ye
  2 siblings, 0 replies; 11+ messages in thread
From: David Marchand @ 2024-06-27 11:30 UTC (permalink / raw)
  To: Mingjin Ye; +Cc: dev, stable, Dmitry Kozlyuk

On Thu, Jun 27, 2024 at 12:36 PM Mingjin Ye <mingjinx.ye@intel.com> wrote:
>
> ELF files generated by higher version compilers wrap multiple
> symbols prefixed with "this_pmd_name".
>
> This patch fixes the issue by filtering invalid symbols.
>
> Bugzilla ID: 1466
> Fixes: 6c4bf8f42432 ("buildtools: add Python pmdinfogen")
> Cc: stable@dpdk.org
>
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>

Is it the same issue as:
https://patchwork.dpdk.org/project/dpdk/patch/20240320155814.617220-1-alialnu@nvidia.com/
?


-- 
David Marchand


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] buildtools: fix invalid symbols
  2024-06-27 10:50 ` Bruce Richardson
@ 2024-06-27 12:39   ` David Marchand
  0 siblings, 0 replies; 11+ messages in thread
From: David Marchand @ 2024-06-27 12:39 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: Mingjin Ye, dev, stable, Dmitry Kozlyuk

On Thu, Jun 27, 2024 at 12:57 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Thu, Jun 27, 2024 at 10:11:44AM +0000, Mingjin Ye wrote:
> > ELF files generated by higher version compilers wrap multiple
> > symbols prefixed with "this_pmd_name".
> >
> > This patch fixes the issue by filtering invalid symbols.
> >
> > Bugzilla ID: 1466
> > Fixes: 6c4bf8f42432 ("buildtools: add Python pmdinfogen")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> > ---
> >  buildtools/pmdinfogen.py | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/buildtools/pmdinfogen.py b/buildtools/pmdinfogen.py
> > index 2a44f17bda..6ea97caec7 100755
> > --- a/buildtools/pmdinfogen.py
> > +++ b/buildtools/pmdinfogen.py
> > @@ -200,7 +200,8 @@ def dump(self, file):
> >  def load_drivers(image):
> >      drivers = []
> >      for symbol in image.find_by_prefix("this_pmd_name"):
> > -        drivers.append(Driver.load(image, symbol))
> > +        if len(symbol.string_value) != 0:
> > +            drivers.append(Driver.load(image, symbol))
>
> One small suggestion. Empty strings evaluate to boolean false, so the
> condition can just be simplified to:
>
>         if symbol.string_value:
>             drivers.append(....)

I have the same comment than what Ali tried with:
https://patchwork.dpdk.org/project/dpdk/patch/20240320155814.617220-1-alialnu@nvidia.com/

I would prefer we don't rely on the content of symbols (that we don't
know anything about) when we can filter on the symbol names exactly
which is something DPDK controls.

My suggestion is to filter symbol *names* with regex ^this_pmd_name[0-9]+$.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] buildtools: fix invalid symbols
  2024-06-27 10:11 [PATCH] buildtools: fix invalid symbols Mingjin Ye
  2024-06-27 10:50 ` Bruce Richardson
  2024-06-27 11:30 ` David Marchand
@ 2024-07-01 10:33 ` Mingjin Ye
  2024-07-03  1:30   ` Jiale, SongX
                     ` (2 more replies)
  2 siblings, 3 replies; 11+ messages in thread
From: Mingjin Ye @ 2024-07-01 10:33 UTC (permalink / raw)
  To: dev; +Cc: Mingjin Ye, stable, Dmitry Kozlyuk

Elf files generated by higher version compilers wrap multiple
symbols prefixed with "this_pmd_name".

The patch uses the regex "^this_pmd_name[0-9]+$" to match the
symbol name.

Bugzilla ID: 1466
Fixes: 6c4bf8f42432 ("buildtools: add Python pmdinfogen")
Cc: stable@dpdk.org

Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
---
v2: Use regex ^this_pmd_name[0-9]+$ to filter symbols *names*
---
 buildtools/pmdinfogen.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/buildtools/pmdinfogen.py b/buildtools/pmdinfogen.py
index 2a44f17bda..0fbcc697ed 100755
--- a/buildtools/pmdinfogen.py
+++ b/buildtools/pmdinfogen.py
@@ -6,6 +6,7 @@
 import argparse
 import ctypes
 import json
+import re
 import sys
 import tempfile
 
@@ -70,7 +71,7 @@ def find_by_prefix(self, prefix):
         prefix = prefix.encode("utf-8") if self._legacy_elftools else prefix
         for i in range(self._symtab.num_symbols()):
             symbol = self._symtab.get_symbol(i)
-            if symbol.name.startswith(prefix):
+            if re.match(prefix, symbol.name):
                 yield ELFSymbol(self._image, symbol)
 
 
@@ -199,7 +200,7 @@ def dump(self, file):
 
 def load_drivers(image):
     drivers = []
-    for symbol in image.find_by_prefix("this_pmd_name"):
+    for symbol in image.find_by_prefix("^this_pmd_name[0-9]+$"):
         drivers.append(Driver.load(image, symbol))
     return drivers
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v2] buildtools: fix invalid symbols
  2024-07-01 10:33 ` [PATCH v2] " Mingjin Ye
@ 2024-07-03  1:30   ` Jiale, SongX
  2024-07-03 16:13   ` David Marchand
  2024-07-05  8:25   ` [PATCH v3] " Mingjin Ye
  2 siblings, 0 replies; 11+ messages in thread
From: Jiale, SongX @ 2024-07-03  1:30 UTC (permalink / raw)
  To: dev; +Cc: Ye, MingjinX

> -----Original Message-----
> From: Mingjin Ye <mingjinx.ye@intel.com>
> Sent: Monday, July 1, 2024 6:33 PM
> To: dev@dpdk.org
> Cc: Ye, MingjinX <mingjinx.ye@intel.com>; stable@dpdk.org; Dmitry Kozlyuk
> <dmitry.kozliuk@gmail.com>
> Subject: [PATCH v2] buildtools: fix invalid symbols
> 
> Elf files generated by higher version compilers wrap multiple symbols prefixed
> with "this_pmd_name".
> 
> The patch uses the regex "^this_pmd_name[0-9]+$" to match the symbol
> name.
> 
> Bugzilla ID: 1466
> Fixes: 6c4bf8f42432 ("buildtools: add Python pmdinfogen")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---
> v2: Use regex ^this_pmd_name[0-9]+$ to filter symbols *names*
> ---
Tested-by: Song Jiale <songx.jiale@intel.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] buildtools: fix invalid symbols
  2024-07-01 10:33 ` [PATCH v2] " Mingjin Ye
  2024-07-03  1:30   ` Jiale, SongX
@ 2024-07-03 16:13   ` David Marchand
  2024-07-05  8:25   ` [PATCH v3] " Mingjin Ye
  2 siblings, 0 replies; 11+ messages in thread
From: David Marchand @ 2024-07-03 16:13 UTC (permalink / raw)
  To: Mingjin Ye; +Cc: dev, stable, Dmitry Kozlyuk, Ali Alnubani

Adding Ali in the loop, as he was working on a similar patch.

On Mon, Jul 1, 2024 at 12:56 PM Mingjin Ye <mingjinx.ye@intel.com> wrote:
>
> Elf files generated by higher version compilers wrap multiple
> symbols prefixed with "this_pmd_name".
>
> The patch uses the regex "^this_pmd_name[0-9]+$" to match the
> symbol name.
>
> Bugzilla ID: 1466
> Fixes: 6c4bf8f42432 ("buildtools: add Python pmdinfogen")
> Cc: stable@dpdk.org
>
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---
> v2: Use regex ^this_pmd_name[0-9]+$ to filter symbols *names*
> ---
>  buildtools/pmdinfogen.py | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/buildtools/pmdinfogen.py b/buildtools/pmdinfogen.py
> index 2a44f17bda..0fbcc697ed 100755
> --- a/buildtools/pmdinfogen.py
> +++ b/buildtools/pmdinfogen.py
> @@ -6,6 +6,7 @@
>  import argparse
>  import ctypes
>  import json
> +import re
>  import sys
>  import tempfile
>
> @@ -70,7 +71,7 @@ def find_by_prefix(self, prefix):

This function does not find symbols with a prefix anymore...
Please rename.


>          prefix = prefix.encode("utf-8") if self._legacy_elftools else prefix
>          for i in range(self._symtab.num_symbols()):
>              symbol = self._symtab.get_symbol(i)
> -            if symbol.name.startswith(prefix):
> +            if re.match(prefix, symbol.name):
>                  yield ELFSymbol(self._image, symbol)
>
>
> @@ -199,7 +200,7 @@ def dump(self, file):
>
>  def load_drivers(image):
>      drivers = []
> -    for symbol in image.find_by_prefix("this_pmd_name"):
> +    for symbol in image.find_by_prefix("^this_pmd_name[0-9]+$"):
>          drivers.append(Driver.load(image, symbol))
>      return drivers

The COFF implementation is probably broken by this change as it is
still filtering symbols with a prefix.
Please update.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3] buildtools: fix invalid symbols
  2024-07-01 10:33 ` [PATCH v2] " Mingjin Ye
  2024-07-03  1:30   ` Jiale, SongX
  2024-07-03 16:13   ` David Marchand
@ 2024-07-05  8:25   ` Mingjin Ye
  2024-07-05 18:08     ` Ali Alnubani
                       ` (2 more replies)
  2 siblings, 3 replies; 11+ messages in thread
From: Mingjin Ye @ 2024-07-05  8:25 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, alialnu, Mingjin Ye, stable, Dmitry Kozlyuk

In scenarios where a higher clang compiler is used and ASAN is enabled,
the generated ELF file will additionally insert undefined debug symbols
with the same prefix. This causes duplicate C code to be generated.

This patch fixes this issue by skipping the unspecified symbol type.

Fixes: 6c4bf8f42432 ("buildtools: add Python pmdinfogen")
Cc: stable@dpdk.org

Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
---
 buildtools/pmdinfogen.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/buildtools/pmdinfogen.py b/buildtools/pmdinfogen.py
index 2a44f17bda..9896f107dc 100755
--- a/buildtools/pmdinfogen.py
+++ b/buildtools/pmdinfogen.py
@@ -70,6 +70,9 @@ def find_by_prefix(self, prefix):
         prefix = prefix.encode("utf-8") if self._legacy_elftools else prefix
         for i in range(self._symtab.num_symbols()):
             symbol = self._symtab.get_symbol(i)
+            # Skip unspecified symbol type
+            if symbol.entry.st_info['type'] == "STT_NOTYPE":
+                continue
             if symbol.name.startswith(prefix):
                 yield ELFSymbol(self._image, symbol)
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v3] buildtools: fix invalid symbols
  2024-07-05  8:25   ` [PATCH v3] " Mingjin Ye
@ 2024-07-05 18:08     ` Ali Alnubani
  2024-07-11  9:20     ` Jiale, SongX
  2024-07-11 11:39     ` David Marchand
  2 siblings, 0 replies; 11+ messages in thread
From: Ali Alnubani @ 2024-07-05 18:08 UTC (permalink / raw)
  To: Mingjin Ye, dev; +Cc: david.marchand, stable, Dmitry Kozlyuk

> -----Original Message-----
> From: Mingjin Ye <mingjinx.ye@intel.com>
> Sent: Friday, July 5, 2024 11:26 AM
> To: dev@dpdk.org
> Cc: david.marchand@redhat.com; Ali Alnubani <alialnu@nvidia.com>; Mingjin Ye
> <mingjinx.ye@intel.com>; stable@dpdk.org; Dmitry Kozlyuk
> <dmitry.kozliuk@gmail.com>
> Subject: [PATCH v3] buildtools: fix invalid symbols
> 
> In scenarios where a higher clang compiler is used and ASAN is enabled,
> the generated ELF file will additionally insert undefined debug symbols
> with the same prefix. This causes duplicate C code to be generated.
> 
> This patch fixes this issue by skipping the unspecified symbol type.
> 
> Fixes: 6c4bf8f42432 ("buildtools: add Python pmdinfogen")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---

Can confirm it fixes the build failure in my environment, thanks Mingjin.

Tested-by: Ali Alnubani <alialnu@nvidia.com>

Regards,
Ali

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v3] buildtools: fix invalid symbols
  2024-07-05  8:25   ` [PATCH v3] " Mingjin Ye
  2024-07-05 18:08     ` Ali Alnubani
@ 2024-07-11  9:20     ` Jiale, SongX
  2024-07-11 11:39     ` David Marchand
  2 siblings, 0 replies; 11+ messages in thread
From: Jiale, SongX @ 2024-07-11  9:20 UTC (permalink / raw)
  To: Ye, MingjinX, dev; +Cc: Ye, MingjinX

> -----Original Message-----
> From: Mingjin Ye <mingjinx.ye@intel.com>
> Sent: Friday, July 5, 2024 4:26 PM
> To: dev@dpdk.org
> Cc: Marchand, David <david.marchand@redhat.com>; alialnu@nvidia.com;
> Ye, MingjinX <mingjinx.ye@intel.com>; stable@dpdk.org; Dmitry Kozlyuk
> <dmitry.kozliuk@gmail.com>
> Subject: [PATCH v3] buildtools: fix invalid symbols
> 
> In scenarios where a higher clang compiler is used and ASAN is enabled, the
> generated ELF file will additionally insert undefined debug symbols with the
> same prefix. This causes duplicate C code to be generated.
> 
> This patch fixes this issue by skipping the unspecified symbol type.
> 
> Fixes: 6c4bf8f42432 ("buildtools: add Python pmdinfogen")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---
Tested-by: Jiale Song <songx.jiale@intel.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3] buildtools: fix invalid symbols
  2024-07-05  8:25   ` [PATCH v3] " Mingjin Ye
  2024-07-05 18:08     ` Ali Alnubani
  2024-07-11  9:20     ` Jiale, SongX
@ 2024-07-11 11:39     ` David Marchand
  2 siblings, 0 replies; 11+ messages in thread
From: David Marchand @ 2024-07-11 11:39 UTC (permalink / raw)
  To: Mingjin Ye; +Cc: dev, alialnu, stable, Dmitry Kozlyuk

On Fri, Jul 5, 2024 at 10:49 AM Mingjin Ye <mingjinx.ye@intel.com> wrote:
>
> In scenarios where a higher clang compiler is used and ASAN is enabled,
> the generated ELF file will additionally insert undefined debug symbols
> with the same prefix. This causes duplicate C code to be generated.
>
> This patch fixes this issue by skipping the unspecified symbol type.

You did not reply to my comments on v3 and here we have a new hack.
This hack is ugly and not future proof (I can imagine some other
symbols may appear in the future. If those symbols are not of type
"STT_NOTYPE" we will have to find another filter at this point...).

Please have a try with:
https://patchwork.dpdk.org/project/dpdk/patch/20240711113851.975368-1-david.marchand@redhat.com/


Thanks.

-- 
David Marchand


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-07-11 11:39 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-27 10:11 [PATCH] buildtools: fix invalid symbols Mingjin Ye
2024-06-27 10:50 ` Bruce Richardson
2024-06-27 12:39   ` David Marchand
2024-06-27 11:30 ` David Marchand
2024-07-01 10:33 ` [PATCH v2] " Mingjin Ye
2024-07-03  1:30   ` Jiale, SongX
2024-07-03 16:13   ` David Marchand
2024-07-05  8:25   ` [PATCH v3] " Mingjin Ye
2024-07-05 18:08     ` Ali Alnubani
2024-07-11  9:20     ` Jiale, SongX
2024-07-11 11:39     ` David Marchand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).