Re: Invalid use of const pointer?
On Sat, 2022-01-08 at 22:34 +0100, Henrik Carlqvist wrote: > But what about the case marked with <--X above? To me it seems as if > the function returns after having modified const char *name bu using > userend. You're right, that's wrong. It turns out to be innocuous because none of the callers care that the value of the input string is modified if we return a different string, but it's still wrong and should be fixed. Thanks for noticing!
Re: Invalid use of const pointer?
On Sat, 08 Jan 2022 15:36:46 -0500
Paul Smith wrote:
> On Sat, 2022-01-08 at 19:47 +0100, Henrik Carlqvist wrote:
> > But now, with both userend and pwent set it seems as if the calling
> > function will have its const string modified. If this final case were
> > fixed at least no calling function would suffer from a modified const
> > string.
> I'm not sure what you mean here. It is never the case that the
> incoming string (name) is ever modified under any circumstances, as far
> as the calling function is concerned.
I mean this code, where name is a const char * which was an input variable to
the function:
# if !defined(_AMIGA) && !defined(WINDOWS32)
else
{
struct passwd *pwent;
char *userend = strchr (name + 1, '/');
if (userend != 0)
*userend = '\0'; <--- ** Here userend modifies the content
pwent = getpwnam (name + 1);
if (pwent != 0)
{
if (userend == 0)
return xstrdup (pwent->pw_dir);
else
return xstrdup (concat (3, pwent->pw_dir, "/", userend + 1)); <--X
}
else if (userend != 0)
*userend = '/'; <--- ** Here userend restores the content
}
# endif /* !AMIGA && !WINDOWS32 */
> If the incoming string needs to expanded then a new string is allocated
> and returned from this function containing the expansion. If the
> incoming string is not expanded, then no new string is allocated and 0
> (null pointer) is returned.
But what about the case marked with <--X above? To me it seems as if the
function returns after having modified const char *name bu using userend.
Best regards Henrik
Re: Segafult while running make(1) from /lib/init/rc with -j
On Sat, 2022-01-08 at 21:37 +0100, Alejandro Colomar (man-pages) wrote: > Hi Dmitry, > On 1/7/22 17:48, Dmitry Goncharov wrote: > > On Thu, Jan 6, 2022 at 2:13 PM Alejandro Colomar (man-pages) > > wrote: > > >I could try to write a simpler Makefile > > That would be good. We need to be able to reproduce the crash. > > I couldn't reproduce it with a simple Makefile with a few includes > and a few sleeps. > > Would you mind if I send you the script with which I generated the > Makefiles, and you run it in a virtual machine? If it's not possible to reproduce the crash outside of your environment then better would be for you to build GNU make with debugging enabled: make CFLAGS=-g then run it so it crashes, then investigate the generated core file with a debugger (gdb) and generate a stack trace to see where things are crashing.
Re: Segafult while running make(1) from /lib/init/rc with -j
On 1/8/22 21:40, Alejandro Colomar (man-pages) wrote: On 1/8/22 21:37, Alejandro Colomar (man-pages) wrote: Hi Dmitry, On 1/7/22 17:48, Dmitry Goncharov wrote: On Thu, Jan 6, 2022 at 2:13 PM Alejandro Colomar (man-pages) wrote: I could try to write a simpler Makefile That would be good. We need to be able to reproduce the crash. I couldn't reproduce it with a simple Makefile with a few includes and a few sleeps. Would you mind if I send you the script with which I generated the Makefiles, and you run it in a virtual machine? I run it on a Devuan install with sysvinit. The steps are: $ sudo mkdir /etc/rc.mk.d/ $ sudo ./script_insserv.sh $ sudo cp ./Makefile /etc/rc.mk.d/Makefile $ sudo sed -i 's/startup stop$/echo foo/' $ sudo sed -i 's,startup [^ ]$,make -C "/etc/rc.mk.d/" "$runlevel" -j 2' I forgot the file name for the sed calls. Append '/lib/init/rc' to those lines. If you try on a different distro, use whatever patch the main rc D'oh! s/patch/path/ script lives in, but you may need to edit it differently if it is different. I also forgot the last comma ',' in the sed s,,, command. Regards, Alex -- Alejandro Colomar Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ http://www.alejandro-colomar.es/
Re: Segafult while running make(1) from /lib/init/rc with -j
On 1/8/22 21:37, Alejandro Colomar (man-pages) wrote: Hi Dmitry, On 1/7/22 17:48, Dmitry Goncharov wrote: On Thu, Jan 6, 2022 at 2:13 PM Alejandro Colomar (man-pages) wrote: I could try to write a simpler Makefile That would be good. We need to be able to reproduce the crash. I couldn't reproduce it with a simple Makefile with a few includes and a few sleeps. Would you mind if I send you the script with which I generated the Makefiles, and you run it in a virtual machine? I run it on a Devuan install with sysvinit. The steps are: $ sudo mkdir /etc/rc.mk.d/ $ sudo ./script_insserv.sh $ sudo cp ./Makefile /etc/rc.mk.d/Makefile $ sudo sed -i 's/startup stop$/echo foo/' $ sudo sed -i 's,startup [^ ]$,make -C "/etc/rc.mk.d/" "$runlevel" -j 2' I forgot the file name for the sed calls. Append '/lib/init/rc' to those lines. If you try on a different distro, use whatever patch the main rc script lives in, but you may need to edit it differently if it is different. I also forgot the last comma ',' in the sed s,,, command. Regards, Alex -- Alejandro Colomar Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ http://www.alejandro-colomar.es/
Re: Segafult while running make(1) from /lib/init/rc with -j
Hi Dmitry,
On 1/7/22 17:48, Dmitry Goncharov wrote:
On Thu, Jan 6, 2022 at 2:13 PM Alejandro Colomar (man-pages)
wrote:
I could try to write a simpler Makefile
That would be good. We need to be able to reproduce the crash.
I couldn't reproduce it with a simple Makefile with a few includes and a
few sleeps.
Would you mind if I send you the script with which I generated the
Makefiles, and you run it in a virtual machine?
I run it on a Devuan install with sysvinit.
The steps are:
$ sudo mkdir /etc/rc.mk.d/
$ sudo ./script_insserv.sh
$ sudo cp ./Makefile /etc/rc.mk.d/Makefile
$ sudo sed -i 's/startup stop$/echo foo/'
$ sudo sed -i 's,startup [^ ]$,make -C "/etc/rc.mk.d/" "$runlevel" -j 2'
It's easier if you have 2 systems on the same virtual machine to be able
to boot from the other ones.
I'll copy the script and the Makefile here, but maybe the mailer breaks
it. If so, I can send it through git-send-email(1).
Thanks,
Alex
---
$ cat script_insserv.sh
#!/bin/bash
tmpd="$(mktemp -d)";
# Add a prefix or suffix "$1" to each Word (in the vi sense of a Word)
function prefix_Words()
{
sed 's/^ *//' \
|sed 's/ *$//' \
|sed "s/ */ $1/g" \
|sed "s/^\(.\)/$1\1/";
}
function suffix_Words()
{
sed 's/^ *//' \
|sed 's/ *$//' \
|sed "s/ */$1 /g" \
|sed "s/\(.\)$/\1$1/";
}
# Copy LSB info into a temp dir. Slightly reformat it regarding whitespace.
find /etc/init.d/ -maxdepth 1 -not -type d \
|while read F; do
grep '^### BEGIN INIT INFO *$' <$F >/dev/null \
&& grep '^### END INIT INFO *$' <$F >/dev/null \
|| continue;
f="$tmpd/$(basename $F)";
# It will be useful to have 1 trailing whitespace
<$F \
sed -n '/^### BEGIN INIT INFO *$/,/^### END INIT INFO *$/p' \
|sed 's/ *$/ /' \
>$f;
done;
function test_service_exists()
{
find $tmpd -not -type d \
|xargs cat \
|grep "# Provides: $1 " >/dev/null;
}
function resolv_insservconf()
{
test '$all' = "$1" \
&& echo '$all' \
&& return 0;
find /etc/insserv.conf /etc/insserv.conf.d/ -not -type d \
|xargs grep "^$1 " \
|sed "s/^$1//" \
|xargs -x -n1 echo \
|while read val; do
echo $val \
|grep '^\$' >/dev/null \
&& resolv_insservconf $val \
|| echo $val;
done \
|xargs -x -n1 echo \
|while read val;do
echo $val \
|grep '^+' >/dev/null \
&& ! test_service_exists $(echo $val | sed 's/^+//') \
|| echo $val;
done \
|xargs echo;
return 0;
}
# Resolve variable dependencies (except for $all, which is special).
find $tmpd -not -type d \
|xargs cat \
|grep -o '\$[^ ]*' \
|sort \
|uniq \
|while read x; do \
val="$(resolv_insservconf $x)";
find $tmpd -not -type d \
|xargs sed -i "s/ $x/ $val/g";
done;
# Remove optional deps that aren't found. Otherwise, make(1) would fail.
find $tmpd -not -type d \
|xargs cat \
|grep -e '# Should-Start:' -e '# Should-Stop:' \
|sed 's/.*://' \
|xargs -n1 echo \
|sort \
|uniq \
|while read x; do \
test '$all' = "$x" \
|| test_service_exists $x \
&& continue;
find $tmpd -not -type d \
|xargs sed -i "/# Should-Start:.* $x .*/s/ $x / /";
find $tmpd -not -type d \
|xargs sed -i "/# Should-Stop:.* $x .*/s/ $x / /";
done;
# XXX We can't use that info with current make(1). Discard it.
# Maybe someone knows how to fix this.
find $tmpd -not -type d \
|while read f; do \
sed -i '/# X-Interactive:/d' $f;
done;
# And finally, write the to-be-included makefile.
find $tmpd -not -type d \
|while read f; do \
p="$(<$f grep '# Provides:' | sed 's/.*://')";
kp="$(echo $p | prefix_Words 'k')";
sp="$(echo $p | prefix_Words 's')";
Kf="K$(basename $f)";
Sf="S$(basename $f)";
allS="$(<$f grep '# Required-Start:.* $all ' >/dev/null && echo all)";
allK="$(<$f grep '# Required-Stop:.* $all ' >/dev/null && echo all)";
sed -i 's/ $all//' $f;
mk="/etc/rc.mk.d/$(basename $f).mk";
echo ".PHONY: $kp $sp $Kf $Sf" >$mk;
<$f \
grep -e '# Required-Start:' -e '# Should-Start:' \
|sed 's/.*://' \
|xargs echo \
|prefix_Words 's' \
|while read sdeps; do \
echo "$Sf: $sdeps";
done \
>>$mk;
<$f \
grep '# X-Start-Before:' \
|sed 's/.*://' \
|prefix_Words 's' \
|while read srdeps; do \
echo "$srdeps: $sp";
done \
>>$mk;
<$f \
grep -e '# Required-Stop:' -e '# Should-Stop:' \
|sed 's/.*://' \
|xargs echo \
|prefix_Words 'k' \
|while read krdeps; do \
echo "$krdeps: $kp";
Re: Invalid use of const pointer?
On Sat, 2022-01-08 at 19:47 +0100, Henrik Carlqvist wrote: > On Sat, 08 Jan 2022 10:37:17 -0500 > Paul Smith wrote: > > The const-correct way to write this code would be to allocate a new > > string and modify that > > Another correct way to do this would be to not declare the input > variable *name as const, but that would need to spread up to calling > functions. Indeed. We explicitly don't want to do that. > There are cases in tilde_expand when *userend is restored restored to > '/' after having being altered to '\0'. In those cases at least no > permanent changes has been made to the const string seen from the > calling functions point of view. Correct. > But now, with both userend and pwent set it seems as if the calling > function will have its const string modified. If this final case were > fixed at least no calling function would suffer from a modified const > string. I'm not sure what you mean here. It is never the case that the incoming string (name) is ever modified under any circumstances, as far as the calling function is concerned. If the incoming string needs to expanded then a new string is allocated and returned from this function containing the expansion. If the incoming string is not expanded, then no new string is allocated and 0 (null pointer) is returned.
Re: Invalid use of const pointer?
On Sat, 08 Jan 2022 10:37:17 -0500 Paul Smith wrote: > The const-correct way to write this code would be to allocate a new > string and modify that (or, rework the entire program to use the > equivalent of C++'s std::string_view), but the author of this code > (might be me, might be someone else: I didn't investigate) decided that > the quick and dirty way had enough benefits to outweigh the gross-ness. Another correct way to do this would be to not declare the input variable *name as const, but that would need to spread up to calling functions. There are cases in tilde_expand when *userend is restored restored to '/' after having being altered to '\0'. In those cases at least no permanent changes has been made to the const string seen from the calling functions point of view. But now, with both userend and pwent set it seems as if the calling function will have its const string modified. If this final case were fixed at least no calling function would suffer from a modified const string. Best regards Henrik
Re: Invalid use of const pointer?
On Fri, 2022-01-07 at 18:28 -0500, Joe Filion wrote: > Line 3094 of read.c is: > > char *userend = strchr (name + 1, '/'); > > The name parameter is a const pointer, so the overloaded version of > strchr that takes a const pointer as the first parameter should also > return a const pointer. You might be thinking of C++. GNU make is written in C, not C++, and there's no such thing as an "overloaded" function in C. There's only one strchr() and it has this signature (according to the C standard): char *strchr(const char *s, int c); > But userend is not a const pointer and is used to modify the string > later in the code. I’m a bit surprised the compiler allows this and I > realize this could just be a misunderstanding of something on my > part. Does anyone else find this construct suspicious? It is kind of gross, yes. It relies on some internal knowledge that, in fact, the string passed in is never truly constant (that we never pass a string constant to this function, that might have been stored in read-only memory). The const-correct way to write this code would be to allocate a new string and modify that (or, rework the entire program to use the equivalent of C++'s std::string_view), but the author of this code (might be me, might be someone else: I didn't investigate) decided that the quick and dirty way had enough benefits to outweigh the gross-ness.
