Bug 241258 - Error building 12.1-RC1 from 11.3, "jevents" program getting a sigsegv in /usr/src/lib/libpmc/pmu-events
Summary: Error building 12.1-RC1 from 11.3, "jevents" program getting a sigsegv in /us...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 12.1-RELEASE
Hardware: amd64 Any
: --- Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords: patch
Depends on:
Blocks: 240700
  Show dependency treegraph
 
Reported: 2019-10-15 07:08 UTC by sigsys
Modified: 2019-11-07 19:09 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description sigsys 2019-10-15 07:08:06 UTC
buildworld failed like that while trying to build 12.1-RC1 (as of r353451) on a 11.3-RELEASE-p1 system:

--- libpmc_events.c ---
./pmu-events/jevents "x86" /usr/src/lib/libpmc/pmu-events/arch libpmc_events.c
Segmentation fault
*** [libpmc_events.c] Error code 139

make[5]: stopped in /usr/src/lib/libpmc
1 error

With this change it doesn't fail anymore:

Index: lib/libpmc/pmu-events/jevents.c
===================================================================
--- lib/libpmc/pmu-events/jevents.c     (revision 353534)
+++ lib/libpmc/pmu-events/jevents.c     (working copy)
@@ -119,8 +119,7 @@
        char *e = s + strlen(s);

        /* Remove trailing dots that look ugly in perf list */
-       --e;
-       while (e >= s && isspace(*e))
+       while (e > s && isspace(e[-1]))
                --e;
        if (*e == '.')
                *e = 0;

Dunno if the problem happens because of something specific to this system but this loop is wrong nevertheless (it could access before the string if it was empty or all spaces).
Comment 1 sigsys 2019-10-15 08:13:16 UTC
Oops. I made a mistake. Comparing the file it was generating on another 12-STABLE system, I noticed that it was stripping the string wrong. Rewrote the whole function, it was confusing.


Index: lib/libpmc/pmu-events/jevents.c
===================================================================
--- lib/libpmc/pmu-events/jevents.c     (revision 353500)
+++ lib/libpmc/pmu-events/jevents.c     (working copy)
@@ -117,13 +117,14 @@
 static void fixdesc(char *s)
 {
        char *e = s + strlen(s);
-
        /* Remove trailing dots that look ugly in perf list */
-       --e;
-       while (e >= s && isspace(*e))
-               --e;
-       if (*e == '.')
-               *e = 0;
+       while (e-- > s) {
+               if (isspace(*e))
+                       continue;
+               if ('.' == *e)
+                       *e = '\0';
+               break;
+       }
 }

 /* Add escapes for '\' so they are proper C strings. */


Testing by doing this:

cd /usr/src/lib/libpmc/pmu-events
make
cd /usr/obj/usr/src/amd64.amd64/lib/libpmc
cp libpmc_events.c libpmc_events.c.saved
./pmu-events/jevents "x86" /usr/src/lib/libpmc/pmu-events/arch libpmc_events.c
diff -u libpmc_events.c.saved libpmc_events.c
Comment 2 sigsys 2019-10-15 08:24:08 UTC
And here it is without the tabs messed up even.

Index: lib/libpmc/pmu-events/jevents.c
===================================================================
--- lib/libpmc/pmu-events/jevents.c	(revision 353534)
+++ lib/libpmc/pmu-events/jevents.c	(working copy)
@@ -117,13 +117,14 @@
 static void fixdesc(char *s)
 {
 	char *e = s + strlen(s);
-
 	/* Remove trailing dots that look ugly in perf list */
-	--e;
-	while (e >= s && isspace(*e))
-		--e;
-	if (*e == '.')
-		*e = 0;
+	while (e-- > s) {
+		if (isspace(*e))
+			continue;
+		if ('.' == *e)
+			*e = '\0';
+		break;
+	}
 }
 
 /* Add escapes for '\' so they are proper C strings. */
Comment 3 Ed Maste freebsd_committer freebsd_triage 2019-11-04 17:15:19 UTC
Would you submit this patch to https://github.com/andikleen/pmu-tools ?

I will look at addressing this in FreeBSD, but it is unfortunately too late for 12.1.
Comment 4 sigsys 2019-11-04 19:41:49 UTC
(In reply to Ed Maste from comment #3)
Alright, done. I submitted a simpler change. Rewriting the whole function wasn't necessary. So maybe there's more chance they'll merge it. The function had a small change in their repo but the bug is still there.

I still have no idea why the crash only happened on this computer. It's been upgraded for a while now and it doesn't exhibit the problem even without the patch anymore. It must have something to do with how the allocated memory ends up being laid out.
Comment 6 commit-hook freebsd_committer freebsd_triage 2019-11-04 22:52:38 UTC
A commit references this bug:

Author: emaste
Date: Mon Nov  4 22:52:03 UTC 2019
New revision: 354342
URL: https://svnweb.freebsd.org/changeset/base/354342

Log:
  libpmc: jevents: handle empty descriptoin

  PR:		241258
  Reported by:	sigsys @ gmail.com
  Obtained from:	github.com/andikleen/pmu-tools commit bb3c77ed61
  MFC after:	3 days

Changes:
  head/lib/libpmc/pmu-events/jevents.c
Comment 7 commit-hook freebsd_committer freebsd_triage 2019-11-07 18:15:48 UTC
A commit references this bug:

Author: emaste
Date: Thu Nov  7 18:14:58 UTC 2019
New revision: 354457
URL: https://svnweb.freebsd.org/changeset/base/354457

Log:
  MFC r354342: libpmc: jevents: handle empty description

  PR:		241258
  Reported by:	sigsys @ gmail.com
  Obtained from:	github.com/andikleen/pmu-tools commit bb3c77ed61

Changes:
_U  stable/12/
  stable/12/lib/libpmc/pmu-events/jevents.c