buildtools: fix build with clang 17

Message ID 20240320155814.617220-1-alialnu@nvidia.com (mailing list archive)
State Changes Requested
Delegated to: David Marchand
Headers
Series buildtools: fix build with clang 17 |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/github-robot: build fail github build: failed
ci/iol-compile-amd64-testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-unit-amd64-testing fail Testing issues
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS

Commit Message

Ali Alnubani March 20, 2024, 3:58 p.m. UTC
  On Fedora 39 with Clang 17.0.3 and ASan enabled,
RTE_PMD_EXPORT_NAME seems to be done twice for a single
lib, which results in load_drivers() returning a list
consisting of 2 drivers (e.g., ['mlx5_common_pci', '']).
image.find_by_prefix("this_pmd_name") returns 2 symbols in this case,
"mlx5_common_pci" and an empty string ''. This didn't reproduce
with clang version 16.0.6.

This patch ensures that a symbol with an empty string_value doesn't
cause an addition to the list of drivers.

Bugzilla ID: 1313

Cc: stable@dpdk.org

Suggested-by: Thomas Monjalon <thomas@monjalon.net>
Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 buildtools/pmdinfogen.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
  

Comments

David Marchand March 20, 2024, 4:30 p.m. UTC | #1
Hello Ali, Thomas,

On Wed, Mar 20, 2024 at 5:01 PM Ali Alnubani <alialnu@nvidia.com> wrote:
>
> On Fedora 39 with Clang 17.0.3 and ASan enabled,
> RTE_PMD_EXPORT_NAME seems to be done twice for a single
> lib, which results in load_drivers() returning a list
> consisting of 2 drivers (e.g., ['mlx5_common_pci', '']).
> image.find_by_prefix("this_pmd_name") returns 2 symbols in this case,
> "mlx5_common_pci" and an empty string ''. This didn't reproduce
> with clang version 16.0.6.
>
> This patch ensures that a symbol with an empty string_value doesn't
> cause an addition to the list of drivers.

I suppose this comes from ASan instrumenting the code:
# nm /root/dpdk/build-clang/drivers/libtmp_rte_common_mlx5.a | grep this_pmd
0000000000000000 r this_pmd_name3
0000000000000000 n this_pmd_name3.e5676185d74e2e1a9de646deebca963f
0000000000000000 r this_pmd_name3
0000000000000000 n this_pmd_name3.a2533baf7a46959f41383725087d4086

The name of the symbols this script should look for has a clear
format, which is this_pmd_name[0-9]+.
Filtering with this pattern, there would be no need to go and
interpret a symbol content.
  
David Marchand June 27, 2024, 12:36 p.m. UTC | #2
On Wed, Mar 20, 2024 at 5:30 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> Hello Ali, Thomas,
>
> On Wed, Mar 20, 2024 at 5:01 PM Ali Alnubani <alialnu@nvidia.com> wrote:
> >
> > On Fedora 39 with Clang 17.0.3 and ASan enabled,
> > RTE_PMD_EXPORT_NAME seems to be done twice for a single
> > lib, which results in load_drivers() returning a list
> > consisting of 2 drivers (e.g., ['mlx5_common_pci', '']).
> > image.find_by_prefix("this_pmd_name") returns 2 symbols in this case,
> > "mlx5_common_pci" and an empty string ''. This didn't reproduce
> > with clang version 16.0.6.
> >
> > This patch ensures that a symbol with an empty string_value doesn't
> > cause an addition to the list of drivers.
>
> I suppose this comes from ASan instrumenting the code:
> # nm /root/dpdk/build-clang/drivers/libtmp_rte_common_mlx5.a | grep this_pmd
> 0000000000000000 r this_pmd_name3
> 0000000000000000 n this_pmd_name3.e5676185d74e2e1a9de646deebca963f
> 0000000000000000 r this_pmd_name3
> 0000000000000000 n this_pmd_name3.a2533baf7a46959f41383725087d4086
>
> The name of the symbols this script should look for has a clear
> format, which is this_pmd_name[0-9]+.
> Filtering with this pattern, there would be no need to go and
> interpret a symbol content.

Ali, can you send a v2?
  
Ali Alnubani June 27, 2024, 4:50 p.m. UTC | #3
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, June 27, 2024 3:37 PM
> To: Ali Alnubani <alialnu@nvidia.com>
> Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon (EXTERNAL)
> <thomas@monjalon.net>; stable@dpdk.org; Bruce Richardson
> <bruce.richardson@intel.com>; Mingjin Ye <mingjinx.ye@intel.com>
> Subject: Re: [PATCH] buildtools: fix build with clang 17
> 
> On Wed, Mar 20, 2024 at 5:30 PM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > Hello Ali, Thomas,
> >
> > On Wed, Mar 20, 2024 at 5:01 PM Ali Alnubani <alialnu@nvidia.com> wrote:
> > >
> > > On Fedora 39 with Clang 17.0.3 and ASan enabled,
> > > RTE_PMD_EXPORT_NAME seems to be done twice for a single
> > > lib, which results in load_drivers() returning a list
> > > consisting of 2 drivers (e.g., ['mlx5_common_pci', '']).
> > > image.find_by_prefix("this_pmd_name") returns 2 symbols in this case,
> > > "mlx5_common_pci" and an empty string ''. This didn't reproduce
> > > with clang version 16.0.6.
> > >
> > > This patch ensures that a symbol with an empty string_value doesn't
> > > cause an addition to the list of drivers.
> >
> > I suppose this comes from ASan instrumenting the code:
> > # nm /root/dpdk/build-clang/drivers/libtmp_rte_common_mlx5.a | grep
> this_pmd
> > 0000000000000000 r this_pmd_name3
> > 0000000000000000 n this_pmd_name3.e5676185d74e2e1a9de646deebca963f
> > 0000000000000000 r this_pmd_name3
> > 0000000000000000 n this_pmd_name3.a2533baf7a46959f41383725087d4086
> >
> > The name of the symbols this script should look for has a clear
> > format, which is this_pmd_name[0-9]+.
> > Filtering with this pattern, there would be no need to go and
> > interpret a symbol content.
> 
> Ali, can you send a v2?
> 

Hi David,

Apologies for the delay, will continue working on this as soon as possible.

Regards,
Ali
  

Patch

diff --git a/buildtools/pmdinfogen.py b/buildtools/pmdinfogen.py
index 2a44f17bda..2b29872649 100755
--- a/buildtools/pmdinfogen.py
+++ b/buildtools/pmdinfogen.py
@@ -71,7 +71,9 @@  def find_by_prefix(self, prefix):
         for i in range(self._symtab.num_symbols()):
             symbol = self._symtab.get_symbol(i)
             if symbol.name.startswith(prefix):
-                yield ELFSymbol(self._image, symbol)
+                elf_symbol = ELFSymbol(self._image, symbol)
+                if elf_symbol.string_value:
+                    yield elf_symbol
 
 
 class COFFSymbol: