GNU bug report logs - #78762
[PATCH] Fix segfault in profiler-cpu-log and profiler-memory-log

Previous Next

To reply to this bug, email your comments to 78762 AT debbugs.gnu.org.
There is no need to reopen the bug first.

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

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

From: Zach Shaftel <zach <at> shaf.tel>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Fix segfault in profiler-cpu-log and profiler-memory-log
Date: Wed, 11 Jun 2025 16:03:12 -0400

[Message part 1 (text/plain, inline)]

Tags: patch

a simple NULL dereference fix. the profiler log would be null if
profiler-*-log is called without a profiler-*-start before it, or if a
previous profiler-*-log call had flushed the log and the profiler wasn't
restarted since then. then export_log would try to read the NULL log and
segfault.

to reproduce the bug:
emacs --batch -f profiler-cpu-log

this patch causes profiler-cpu-log and profiler-memory-log to just
return nil if the log is null.


In GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.49, cairo version 1.18.4) of 2025-06-04 built on bigbox
Repository revision: 680fa61b5989b84c0e19ac568be012afd8345f0c
Repository branch: master
System Description: Arch Linux

Configured using:
 'configure --with-modules --without-xwidgets --with-native-compilation
 --with-tree-sitter --without-gsettings --without-gconf --without-gpm
 --with-pgtk --without-compress-install 'CFLAGS=-mtune=native
 -march=native -O2 -g -fuse-ld=mold''

[0001-Fix-segfault-in-profiler-cpu-log-and-profiler-memory.patch (text/patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Zach Shaftel <zach <at> shaf.tel>
Cc: 78762 <at> debbugs.gnu.org
Subject: Re: bug#78762: [PATCH] Fix segfault in profiler-cpu-log and
 profiler-memory-log
Date: Thu, 12 Jun 2025 08:03:10 +0300

> Date: Wed, 11 Jun 2025 16:03:12 -0400
> From:  Zach Shaftel via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> a simple NULL dereference fix. the profiler log would be null if
> profiler-*-log is called without a profiler-*-start before it, or if a
> previous profiler-*-log call had flushed the log and the profiler wasn't
> restarted since then. then export_log would try to read the NULL log and
> segfault.
> 
> to reproduce the bug:
> emacs --batch -f profiler-cpu-log
> 
> this patch causes profiler-cpu-log and profiler-memory-log to just
> return nil if the log is null.

Thanks, please see a few minor comments below.

> >From 2c1b3bc8b020acbef496dd991bd08bc0f5505528 Mon Sep 17 00:00:00 2001
> From: Zach Shaftel <zach <at> shaf.tel>
> Date: Wed, 11 Jun 2025 15:37:31 -0400
> Subject: [PATCH] Fix segfault in profiler-cpu-log and profiler-memory-log
> 
> * src/profiler.c (export_log): Check if a log has been allocated first,
> and return nil if it hasn't.
> (Fprofiler_cpu_log, Fprofiler_memory_log): Mention the possibly nil
> result.

The last sentence should say "...in the doc string."  Or just "Doc
fix" should be enough, since the nature of the change is clear from
the diffs.

Also, in the next revisions of the patch mention the bug number in the
commit log message, since you now know its number.

> @@ -534,7 +534,11 @@ DEFUN ("profiler-cpu-log", Fprofiler_cpu_log, Sprofiler_cpu_log,
>  The log is a hash-table mapping backtraces to counters which represent
>  the amount of time spent at those points.  Every backtrace is a vector
>  of functions, where the last few elements may be nil.
> -Before returning, a new log is allocated for future samples.  */)
> +
> +If the profiler has not run since the last invocation of
> +`profiler-cpu-log' (or was never run at all), nil is returned.  If the
> +profiler is currently running, a new log is allocated for future samples
> +before returning.  */)

Please avoid passive tense as much as possible.  Here, "return nil"
and "allocate a new log" is shorter and more clear.

> +  if (plog->log == NULL)

We usually prefer the C style:

     if (!plog->log)

> +    /* We haven't collected any new data, so return nil.  */
> +    return Qnil;

No need to add comments that just describe what the code already says.
Comments which explain what the code does should tell something that
isn't clear by just looking at the code.

> @@ -639,7 +646,11 @@ DEFUN ("profiler-memory-log",
>  The log is a hash-table mapping backtraces to counters which represent
>  the amount of memory allocated at those points.  Every backtrace is a vector
>  of functions, where the last few elements may be nil.
> -Before returning, a new log is allocated for future samples.  */)
> +
> +If the profiler has not run since the last invocation of
> +`profiler-memory-log' (or was never run at all), nil is returned.  If
> +the profiler is currently running, a new log is allocated for future
> +samples before returning.  */)

Same comment as above for the other doc string.

Also, can you add a test for this?

Finally, the same problem exists on the emacs-30 release branch, so
could you please post your next revision of the patch relative to the
release branch?

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

From: zach shaftel <zach <at> shaf.tel>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 78762 <at> debbugs.gnu.org
Subject: Re: bug#78762: [PATCH] Fix segfault in profiler-cpu-log and
 profiler-memory-log
Date: Thu, 12 Jun 2025 22:11:36 -0400

[Message part 1 (text/plain, inline)]

thank you for the feedback, a new patch is attached. let me know if i
missed anything.

- zach

[0001-Fix-segfault-in-profiler-cpu-log-and-profiler-memory.patch (text/x-patch, attachment)]

Message #18 received at 78762-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: zach shaftel <zach <at> shaf.tel>
Cc: 78762-done <at> debbugs.gnu.org
Subject: Re: bug#78762: [PATCH] Fix segfault in profiler-cpu-log and
 profiler-memory-log
Date: Sat, 14 Jun 2025 17:56:58 +0300

> From: zach shaftel <zach <at> shaf.tel>
> Cc: 78762 <at> debbugs.gnu.org
> Date: Thu, 12 Jun 2025 22:11:36 -0400
> 
> thank you for the feedback, a new patch is attached. let me know if i
> missed anything.

Thanks, installed on master, and closing the bug.

Previous Next

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