GNU bug report logs - #33181
sed --debug: fix two problems

Previous Next

To reply to this bug, email your comments to 33181 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: bug-sed <at> gnu.org
Subject: sed --debug: fix two problems
Date: Sat, 27 Oct 2018 16:00:40 -0700

Hi Assaf,

I built and ran tests on a Fedora 29 beta system
and noticed a failure in the new debug test because
I'd set MALLOC_PERTURB_=13 in my environment.
That exposed what looked like a UMR bug.  I confirmed it by
running under valgrind.  Fixed by the first patch below.

Initially, I didn't even reach the offending code because
that same system lacked perl's Data::Dump module.
That is addressed by the second patch.

Thanks,
Jim

>From 2bfa7984c84caaaedcfb26a13d8c190f490d7cd0 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering <at> fb.com>
Date: Sat, 27 Oct 2018 15:47:41 -0700
Subject: [PATCH 1/2] sed: avoid UMR in --debug code path

* sed/debug.c (debug_print_function) [b, t, T]: For a b, t or T
command with no LABEL, do not access uninitialized memory.
I.e., print the label name only when there is one.
---
 sed/debug.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/sed/debug.c b/sed/debug.c
index 4eedd40..9ec37b6 100644
--- a/sed/debug.c
+++ b/sed/debug.c
@@ -302,9 +302,12 @@ debug_print_function (const struct vector *program, const struct sed_cmd *sc)
     case 't':
     case 'T':
       {
-        const char *label_name = program->v[sc->x.jump_index].x.label_name;
-        if (label_name)
-          printf (" %s", label_name);
+        if (sc->x.jump_index < program->v_length)
+          {
+            const char *label_name = program->v[sc->x.jump_index].x.label_name;
+            if (label_name)
+              printf (" %s", label_name);
+          }
       }
       break;

--
2.18.0


>From 1c0866b1d34c18cfba2cfd20cc915e70fa975cbd Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering <at> fb.com>
Date: Sat, 27 Oct 2018 15:53:03 -0700
Subject: [PATCH 2/2] tests: avoid unnecessary dependency on Data::Dump

* testsuite/debug.pl: Don't use Data::Dump.  Unnecessary.
That package, perl-Data-Dump, appear not to be default-installed
on at least a Fedora 29 beta system.
---
 testsuite/debug.pl | 1 -
 1 file changed, 1 deletion(-)

diff --git a/testsuite/debug.pl b/testsuite/debug.pl
index 2e4235a..661c5ed 100644
--- a/testsuite/debug.pl
+++ b/testsuite/debug.pl
@@ -18,7 +18,6 @@

 use strict;
 use File::stat;
-use Data::Dump qw(dump);

 (my $program_name = $0) =~ s|.*/||;

--
2.18.0

Message #8 received at 33181 <at> debbugs.gnu.org (full text, mbox):

From: Assaf Gordon <assafgordon <at> gmail.com>
To: Jim Meyering <jim <at> meyering.net>, 33181 <at> debbugs.gnu.org
Subject: Re: bug#33181: sed --debug: fix two problems
Date: Sat, 27 Oct 2018 23:56:34 -0600

On 2018-10-27 5:00 p.m., Jim Meyering wrote:
> That exposed what looked like a UMR bug.  I confirmed it by
> running under valgrind.  Fixed by the first patch below.
[...]
> Initially, I didn't even reach the offending code because
> that same system lacked perl's Data::Dump module.
> That is addressed by the second patch.

Good catch!
I should have used the new "make build-asan" to test the new debug
feature, I now see it does report similar failures.

Thanks for the quick fix, looks good.

-assaf

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.