check-experimental-syms.sh: prevent symbol matches on substrings

Message ID 20181010142928.11274-1-nhorman@tuxdriver.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series check-experimental-syms.sh: prevent symbol matches on substrings |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Neil Horman Oct. 10, 2018, 2:29 p.m. UTC
Thomas attempted to submit this:
https://patches.dpdk.org/patch/46311/

The other day, because the other patches being submitted with it were
breaking on a false positive from the check-experimental-syms check.

The problem was that the experimental symbol check script matched on the
regexs "\.text.*$SYM" and "\.text\.experimental.*$SYM" which allows for
substring matches, and librte_ethdev recently introduced symbols that
are leading substrings of one another (e.g. symbol foo is a substring of
symbol foobar), and so we would match on symbols when we shouldn't

Instead of dropping the check, fix this properly by matching
additionally on the end of line so that symbols are an exact match.

Confirmed to build properly on Thomas' submitted patch set with the
experimental check patch reverted (so that the checking actually
happens)

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Thomas Monjalon <thomas@monjalon.net>
CC: Ferruh Yigit <ferruh.yigit@intel.com>
---
 buildtools/check-experimental-syms.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Thomas Monjalon Oct. 10, 2018, 2:43 p.m. UTC | #1
10/10/2018 16:29, Neil Horman:
> Thomas attempted to submit this:
> https://patches.dpdk.org/patch/46311/
> 
> The other day, because the other patches being submitted with it were
> breaking on a false positive from the check-experimental-syms check.
> 
> The problem was that the experimental symbol check script matched on the
> regexs "\.text.*$SYM" and "\.text\.experimental.*$SYM" which allows for
> substring matches, and librte_ethdev recently introduced symbols that
> are leading substrings of one another (e.g. symbol foo is a substring of
> symbol foobar), and so we would match on symbols when we shouldn't
> 
> Instead of dropping the check, fix this properly by matching
> additionally on the end of line so that symbols are an exact match.
> 
> Confirmed to build properly on Thomas' submitted patch set with the
> experimental check patch reverted (so that the checking actually
> happens)

That's great Neil!
I would like to push it now.
May I suggest to remove the details of how I (baldly) reported it?

I suggest this text:
"
The experimental symbol check script matched on the regexes
"\.text.*$SYM" and "\.text\.experimental.*$SYM" which allows for
substring matches.
If a symbol is leading substring of another one (e.g. symbol foo
is a substring of symbol foobar), it would match on symbols
when it shouldn't.

It is fixed by matching additionally on the end of line
so that symbols are an exact match.
"
  
Neil Horman Oct. 10, 2018, 8:19 p.m. UTC | #2
On Wed, Oct 10, 2018 at 04:43:14PM +0200, Thomas Monjalon wrote:
> 10/10/2018 16:29, Neil Horman:
> > Thomas attempted to submit this:
> > https://patches.dpdk.org/patch/46311/
> > 
> > The other day, because the other patches being submitted with it were
> > breaking on a false positive from the check-experimental-syms check.
> > 
> > The problem was that the experimental symbol check script matched on the
> > regexs "\.text.*$SYM" and "\.text\.experimental.*$SYM" which allows for
> > substring matches, and librte_ethdev recently introduced symbols that
> > are leading substrings of one another (e.g. symbol foo is a substring of
> > symbol foobar), and so we would match on symbols when we shouldn't
> > 
> > Instead of dropping the check, fix this properly by matching
> > additionally on the end of line so that symbols are an exact match.
> > 
> > Confirmed to build properly on Thomas' submitted patch set with the
> > experimental check patch reverted (so that the checking actually
> > happens)
> 
> That's great Neil!
> I would like to push it now.
> May I suggest to remove the details of how I (baldly) reported it?
> 
> I suggest this text:
> "
> The experimental symbol check script matched on the regexes
> "\.text.*$SYM" and "\.text\.experimental.*$SYM" which allows for
> substring matches.
> If a symbol is leading substring of another one (e.g. symbol foo
> is a substring of symbol foobar), it would match on symbols
> when it shouldn't.
> 
> It is fixed by matching additionally on the end of line
> so that symbols are an exact match.
> "
Sure, if you want to make that change on commit, I'm ok with it.
Neil

> 
> 
> 
>
  
Thomas Monjalon Oct. 11, 2018, 11:59 a.m. UTC | #3
10/10/2018 22:19, Neil Horman:
> On Wed, Oct 10, 2018 at 04:43:14PM +0200, Thomas Monjalon wrote:
> > 10/10/2018 16:29, Neil Horman:
> > > Thomas attempted to submit this:
> > > https://patches.dpdk.org/patch/46311/
> > > 
> > > The other day, because the other patches being submitted with it were
> > > breaking on a false positive from the check-experimental-syms check.
> > > 
> > > The problem was that the experimental symbol check script matched on the
> > > regexs "\.text.*$SYM" and "\.text\.experimental.*$SYM" which allows for
> > > substring matches, and librte_ethdev recently introduced symbols that
> > > are leading substrings of one another (e.g. symbol foo is a substring of
> > > symbol foobar), and so we would match on symbols when we shouldn't
> > > 
> > > Instead of dropping the check, fix this properly by matching
> > > additionally on the end of line so that symbols are an exact match.
> > > 
> > > Confirmed to build properly on Thomas' submitted patch set with the
> > > experimental check patch reverted (so that the checking actually
> > > happens)
> > 
> > That's great Neil!
> > I would like to push it now.
> > May I suggest to remove the details of how I (baldly) reported it?
> > 
> > I suggest this text:
> > "
> > The experimental symbol check script matched on the regexes
> > "\.text.*$SYM" and "\.text\.experimental.*$SYM" which allows for
> > substring matches.
> > If a symbol is leading substring of another one (e.g. symbol foo
> > is a substring of symbol foobar), it would match on symbols
> > when it shouldn't.
> > 
> > It is fixed by matching additionally on the end of line
> > so that symbols are an exact match.
> > "
> Sure, if you want to make that change on commit, I'm ok with it.

Applied, thanks
  

Patch

diff --git a/buildtools/check-experimental-syms.sh b/buildtools/check-experimental-syms.sh
index 5bc8cda17..d0915102d 100755
--- a/buildtools/check-experimental-syms.sh
+++ b/buildtools/check-experimental-syms.sh
@@ -16,9 +16,9 @@  for i in `awk 'BEGIN {found=0}
 		/.*;/ {if (found == 1) print $1}' $MAPFILE`
 do
 	SYM=`echo $i | sed -e"s/;//"`
-	objdump -t $OBJFILE | grep -q "\.text.*$SYM"
+	objdump -t $OBJFILE | grep -q "\.text.*$SYM$"
 	IN_TEXT=$?
-	objdump -t $OBJFILE | grep -q "\.text\.experimental.*$SYM"
+	objdump -t $OBJFILE | grep -q "\.text\.experimental.*$SYM$"
 	IN_EXP=$?
 	if [ $IN_TEXT -eq 0 -a $IN_EXP -ne 0 ]
 	then