Re: 4.3.90 release candidate segfaults on linux and solaris

[Paul Smith]

On Sat, 2022-09-24 at 13:06 -0400, Paul Smith wrote:
> > The crashes in solaris and linux are so similar, and go away also
> > so similarly, that i would primarily think about size of types.
> > Under cygwin and MacOS all sizes (pointers, long int, size_t,
> > SIZE_MAX, time_t) are 8, while under (this) solaris and (this)
> > linux all sizes are 4. Only size of int is 4 on all.
> 
> Aha!  You're using a 32bit Linux.  That is indeed helpful
> information, thanks.  I will check it.

I installed a 32bit docker container and tried with that but still no
failure.  I will keep poking at it; maybe I can find a much older
container.

Re: GNU make 4.3.90 release candidate available

[Paul Smith]

On Sat, 2022-09-24 at 16:41 -0400, Paul Smith wrote:
> makeint.h already has HAVE_MAKEINT_H

Doh, I meant HAVE_INTTYPES_H of course.

Re: GNU make 4.3.90 release candidate available

[Paul Smith]

On Sat, 2022-09-24 at 21:09 +0300, Eli Zaretskii wrote:
> MinGW does have inttypes.h, if you want to use PRId64 or somesuch,
> and
> if the other supported platforms have that.
> 
> I'm fine with using our own definition for PRId64, if that's what you
> prefer.  On makeint.h, I presume?

makeint.h already has HAVE_MAKEINT_H and it's set by configure and all
the custom config.h (or should be).  So I guess afterward it would be
something like,

  #ifndef PRId64
  # ifdef WINDOWS32
  #  define PRId64 "I64d"
  # else
  #  define PRId64 "lld"
  # endif
  #endif

or something like that...?

[bug #63100] Crash in a loadable plugin

[Dmitry Goncharov]

Follow-up Comment #1, bug #63100 (project make):

When make unloads and then loads a shared object make supplies an 
uninitialized floc to the plugin setup routine.


$ cat makefile
load hello.so
all:; :
hello.so: force; :
force:;
.PHONY: force
$ cat hello.c
#include 
#include 
int plugin_is_GPL_compatible;


int hello_gmk_setup (const gmk_floc *floc)
{
  assert (floc->filenm);
  return 1;
}
$ ~/src/gmake/make/m64/make
hello plugin loaded from makefile:1
:
make: hello.c:9: hello_gmk_setup: Assertion `floc->filenm'
failed.
Aborted



___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63100] Crash in a loadable plugin

[Dmitry Goncharov]

Additional Item Attachment, bug #63100 (project make):

File name: sv63100_fix.diff   Size:0 KB


File name: sv63100_test.diff  Size:1 KB




___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63100] Crash in a loadable plugin

[Dmitry Goncharov]

URL:
  

 Summary: Crash in a loadable plugin
 Project: make
   Submitter: dgoncharov
   Submitted: Sat 24 Sep 2022 08:30:58 PM UTC
Severity: 3 - Normal
  Item Group: Bug
  Status: None
 Privacy: Public
 Assigned to: None
 Open/Closed: Open
 Discussion Lock: Any
   Component Version: SCM
Operating System: POSIX-Based
   Fixed Release: None
   Triage Status: None


___

Follow-up Comments:


---
Date: Sat 24 Sep 2022 08:30:58 PM UTC By: Dmitry Goncharov 
.







___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

Re: GNU make 4.3.90 release candidate available

[Eli Zaretskii]

> From: Paul Smith 
> Cc: bug-make@gnu.org
> Date: Sat, 24 Sep 2022 13:10:00 -0400
> 
> > Here are the proposed patches for the above 2 warnings.  Paul, if you
> > agree with them, I will install them in the repository.
> 
> These changes look good to me, thanks!

Thanks, installed.

Re: GNU make 4.3.90 release candidate available

[Eli Zaretskii]

> From: Paul Smith 
> Cc: bug-make@gnu.org
> Date: Sat, 24 Sep 2022 13:45:57 -0400
> 
> On Wed, 2022-09-21 at 06:18 +0100, Sam James wrote:
> > Can I suggest forwarding to the platform-testers@ mailing list as
> > well?
> 
> I've never heard of this; is it @gnu.org?

Yes, platform-test...@gnu.org.

Re: GNU make 4.3.90 release candidate available

[Eli Zaretskii]

> From: Paul Smith 
> Cc: bug-make@gnu.org
> Date: Sat, 24 Sep 2022 13:42:39 -0400
> 
> On Sat, 2022-09-24 at 14:17 +0300, Eli Zaretskii wrote:
> > Here are the proposed patches:
> > 
> > --- src/function.c~02022-09-18 22:06:17.0 +0300
> > +++ src/function.c  2022-09-24 13:30:01.740875000 +0300
> > @@ -823,6 +823,15 @@ func_wordlist (char *o, char **argv, con
> >    stop = parse_numeric (argv[1],
> >  _("invalid second argument to 'wordlist'
> > function"));
> >  
> > +#ifdef WINDOWS32
> > +  if (start < 1)
> > +    ON (fatal, *expanding_var,
> > +    "invalid first argument to 'wordlist' function: '%I64d'",
> > start);
> > +
> > +  if (stop < 0)
> > +    ON (fatal, *expanding_var,
> > +    "invalid second argument to 'wordlist' function: '%I64d'",
> > stop);
> > +#else
> >    if (start < 1)
> >  ON (fatal, *expanding_var,
> >  "invalid first argument to 'wordlist' function: '%lld'",
> > start);
> > @@ -830,6 +839,7 @@ func_wordlist (char *o, char **argv, con
> >    if (stop < 0)
> >  ON (fatal, *expanding_var,
> >  "invalid second argument to 'wordlist' function: '%lld'",
> > stop);
> > +#endif
> >  
> >    count = stop - start + 1;
> 
> I would prefer to avoid fully duplicating all the code, just to fix the
> string.  I assume that the PR* macros are not portable?

MinGW does have inttypes.h, if you want to use PRId64 or somesuch, and
if the other supported platforms have that.

I'm fine with using our own definition for PRId64, if that's what you
prefer.  On makeint.h, I presume?

Re: GNU make 4.3.90 release candidate available

[Paul Smith]

On Wed, 2022-09-21 at 06:18 +0100, Sam James wrote:
> Can I suggest forwarding to the platform-testers@ mailing list as
> well?

I've never heard of this; is it @gnu.org?  Or somewhere else?  A full
email address would be useful and also if you have any information on
this (a website or whatever) that would be good.

Cheers!

Re: GNU make 4.3.90 release candidate available

[Paul Smith]

On Sat, 2022-09-24 at 14:17 +0300, Eli Zaretskii wrote:
> Here are the proposed patches:
> 
> --- src/function.c~02022-09-18 22:06:17.0 +0300
> +++ src/function.c  2022-09-24 13:30:01.740875000 +0300
> @@ -823,6 +823,15 @@ func_wordlist (char *o, char **argv, con
>    stop = parse_numeric (argv[1],
>  _("invalid second argument to 'wordlist'
> function"));
>  
> +#ifdef WINDOWS32
> +  if (start < 1)
> +    ON (fatal, *expanding_var,
> +    "invalid first argument to 'wordlist' function: '%I64d'",
> start);
> +
> +  if (stop < 0)
> +    ON (fatal, *expanding_var,
> +    "invalid second argument to 'wordlist' function: '%I64d'",
> stop);
> +#else
>    if (start < 1)
>  ON (fatal, *expanding_var,
>  "invalid first argument to 'wordlist' function: '%lld'",
> start);
> @@ -830,6 +839,7 @@ func_wordlist (char *o, char **argv, con
>    if (stop < 0)
>  ON (fatal, *expanding_var,
>  "invalid second argument to 'wordlist' function: '%lld'",
> stop);
> +#endif
>  
>    count = stop - start + 1;

I would prefer to avoid fully duplicating all the code, just to fix the
string.  I assume that the PR* macros are not portable?  Maybe we can
create our own and use those?

> --- src/w32/pathstuff.c~0   2022-07-07 22:46:05.0 +0300
> +++ src/w32/pathstuff.c 2022-09-24 13:45:53.303375000 +0300
> @@ -102,11 +102,11 @@ w32ify(const char *filename, int resolve
>  if (resolve)
>    {
>  char *fp = _fullpath (NULL, filename, sizeof (w32_path));
> -    strncpy (w32_path, fp, sizeof (w32_path));
> +    strncpy (w32_path, fp, sizeof (w32_path) - 1);
>  free (fp);
>    }
>  else
> -  strncpy(w32_path, filename, sizeof (w32_path));
> +  strncpy(w32_path, filename, sizeof (w32_path) - 1);
>  
>  for (p = w32_path; p && *p; p++)
>    if (*p == '\\')
> 
> 
> --- src/w32/w32os.c~0   2022-08-31 23:07:28.0 +0300
> +++ src/w32/w32os.c 2022-09-24 13:58:43.615875000 +0300
> @@ -22,7 +22,9 @@ this program.  If not, see   #include 
>  #include 
>  #include 
> +#if _WIN32_WINNT > 0x0601
>  #include 
> +#endif
>  #include "pathstuff.h"
>  #include "sub_proc.h"
>  #include "w32err.h"
> @@ -429,7 +431,7 @@ osync_get_mutex ()
>    /* Prepare the mutex handle string for our children.
>   2 hex digits per byte + 2 characters for "0x" + null.  */
>    mutex = xmalloc ((2 * sizeof (osync_handle)) + 2 + 1);
> -  sprintf (mutex, "0x%Ix", (unsigned long long)osync_handle);
> +  sprintf (mutex, "0x%Ix", (unsigned long long)(DWORD_PTR)osync_handle);
>  }
>  
>    return mutex;
> @@ -449,7 +451,7 @@ osync_parse_mutex (const char *mutex)
>    if (endp[0] != '\0')
>  OS (fatal, NILF, _("invalid output sync mutex: %s"), mutex);
>  
> -  osync_handle = (HANDLE) i;
> +  osync_handle = (HANDLE) (DWORD_PTR) i;
>  
>    return 1;
>  }

Weird, but OK.

Re: GNU make 4.3.90 release candidate available

[Paul Smith]

On Sat, 2022-09-24 at 14:09 +0300, Eli Zaretskii wrote:
> I've built this with mingw.org's MinGW for native MS-Windows.  I
> found several issues during the build, some of them specific to
> Windows, others more general.  (I didn't yet run the test suite, I
> intend to do that next.)
> 
> Here are the build-time issues that aren't specific to MS-Windows:
> 
>  src/getopt.c: In function '_getopt_internal':
>  src/getopt.c:679:8: warning: suggest explicit braces to avoid
> ambiguous 'else' [-Wdangling-else]
>    679 | if (opterr)
>    |    ^
> 
> (This is not new to this release, I saw the same in Make 4.3.)  I
> ignored this one.

Agreed.  I need to update to the newer getopt library from gnulib
(along with the newer glob library) but this is non-trivial for GNU
make so I will probably wait until after this release.

> Here are the proposed patches for the above 2 warnings.  Paul, if you
> agree with them, I will install them in the repository.

These changes look good to me, thanks!

Re: 4.3.90 release candidate segfaults on linux and solaris

[Paul Smith]

On Sat, 2022-09-24 at 18:23 +0200, Denis Excoffier wrote:
> The segfault also occurs without ‘env -i’.

Thanks for helping me avoid that red herring.

> The crashes in solaris and linux are so similar, and go away also so
> similarly, that i would primarily think about size of types. Under
> cygwin and MacOS all sizes (pointers, long int, size_t, SIZE_MAX,
> time_t) are 8, while under (this) solaris and (this) linux all sizes
> are 4. Only size of int is 4 on all.

Aha!  You're using a 32bit Linux.  That is indeed helpful information,
thanks.  I will check it.

Re: 4.3.90 release candidate segfaults on linux and solaris

[Denis Excoffier]

> On 2022-09-24 17:19, Paul Smith wrote:
> 
> On Sat, 2022-09-24 at 09:36 +0200, Denis Excoffier wrote:
>> In my specific configuration (under linux, with --disable-nls,
>> --disable-load, without using -j, using 'env -i make -d -n'), a
>> segfault always occurs around line 118 of src/expand.c:
>> 
>> My linux is old (2.6.32),
> 
> The kernel version is not very interesting for a userspace program like
> GNU make.  But, it would be interesting if you could provide the
> version of libc you are using; on my system I can use:
> 
>  ~$ /lib/x86_64-linux-gnu/libc.so.6 --version | head -n1
> 
> You can find the library by running "ldd make | grep libc'
> 
> It's also interesting you're running with "env -i" that seems like it
> might be related.  Do you get the crash if you run without that?
> 
> A quick check on my system was not able to show any issues using the
> same setup as you, even building make with ASAN.  I will review the
> code especially around memory handling in the child structures to see
> if I can find an error by inspection.

% ldd make
linux-gate.so.1 (0xb76e4000)
libc.so.6 => /lib/i386-linux-gnu/libc.so.6 (0xb7565000)
/lib/ld-linux.so.2 (0xb76e5000)
% /lib/i386-linux-gnu/libc.so.6 --version | head -2
GNU C Library (Debian GLIBC 2.19-18+deb8u10) stable release version 2.19, by 
Roland McGrath et al.
Copyright (C) 2014 Free Software Foundation, Inc.
%

The segfault also occurs without ‘env -i’. I was just trying to minimize the 
number of
entries in the environment. In any case make seems to always add a few ones
by default (like MAKEFLAGS etc.).

The crashes in solaris and linux are so similar, and go away also so similarly,
that i would primarily think about size of types. Under cygwin and
MacOS all sizes (pointers, long int, size_t, SIZE_MAX, time_t) are 8, while
under (this) solaris and (this) linux all sizes are 4. Only size of int is 4 on 
all.

Regards,

Denis Excoffier.

Re: 4.3.90 release candidate segfaults on linux and solaris

[Paul Smith]

On Sat, 2022-09-24 at 09:36 +0200, Denis Excoffier wrote:
> In my specific configuration (under linux, with --disable-nls,
> --disable-load, without using -j, using 'env -i make -d -n'), a
> segfault always occurs around line 118 of src/expand.c:
> 
> My linux is old (2.6.32),

The kernel version is not very interesting for a userspace program like
GNU make.  But, it would be interesting if you could provide the
version of libc you are using; on my system I can use:

  ~$ /lib/x86_64-linux-gnu/libc.so.6 --version | head -n1

You can find the library by running "ldd make | grep libc'

It's also interesting you're running with "env -i" that seems like it
might be related.  Do you get the crash if you run without that?

A quick check on my system was not able to show any issues using the
same setup as you, even building make with ASAN.  I will review the
code especially around memory handling in the child structures to see
if I can find an error by inspection.

Re: GNU make 4.3.90 release candidate available

[Eli Zaretskii]

I've built this with mingw.org's MinGW for native MS-Windows.  I found
several issues during the build, some of them specific to Windows,
others more general.

Below are the build-time issues that are specific to MS-Windows, and
the proposed patches.  Paul, I'll wait for your okay before installing
those.

Warning in function.c:

 In file included from src/function.c:17:
 src/function.c: In function 'func_wordlist':
 src/makeint.h:524:55: warning: unknown conversion type character 'l' in 
format [-Wformat=]
   524 | #define ON(_t,_a,_f,_n)   _t((_a), INTSTR_LENGTH, (_f), (_n))
   |   ^~~~
 src/function.c:827:5: note: in expansion of macro 'ON'
   827 | ON (fatal, *expanding_var,
   | ^~
 src/makeint.h:524:55: warning: too many arguments for format 
[-Wformat-extra-args]
   524 | #define ON(_t,_a,_f,_n)   _t((_a), INTSTR_LENGTH, (_f), (_n))
   |   ^~~~
 src/function.c:827:5: note: in expansion of macro 'ON'
   827 | ON (fatal, *expanding_var,
   | ^~
 src/makeint.h:524:55: warning: unknown conversion type character 'l' in 
format [-Wformat=]
   524 | #define ON(_t,_a,_f,_n)   _t((_a), INTSTR_LENGTH, (_f), (_n))
   |   ^~~~
 src/function.c:831:5: note: in expansion of macro 'ON'
   831 | ON (fatal, *expanding_var,
   | ^~
 src/makeint.h:524:55: warning: too many arguments for format 
[-Wformat-extra-args]
   524 | #define ON(_t,_a,_f,_n)   _t((_a), INTSTR_LENGTH, (_f), (_n))
   |   ^~~~
 src/function.c:831:5: note: in expansion of macro 'ON'
   831 | ON (fatal, *expanding_var,
   | ^~

This is because %lld is not supported by MSVCRT.

Warnings in pathstuff.c:

 src/w32/pathstuff.c: In function 'w32ify':
 src/w32/pathstuff.c:109:7: warning: 'strncpy' specified bound 260 equals 
destination size [-Wstringop-truncation]
   109 |   strncpy(w32_path, filename, sizeof (w32_path));
   |   ^~
 src/w32/pathstuff.c:105:9: warning: 'strncpy' specified bound 260 equals 
destination size [-Wstringop-truncation]
   105 | strncpy (w32_path, fp, sizeof (w32_path));
   | ^

I fixed those by copying 1 less characters.

Error in w32os.c:

 src/w32/w32os.c:25:10: fatal error: synchapi.h: No such file or directory
25 | #include 
   |  ^~~~
 compilation terminated.

The MS documentation says to use synchapi.h only since Windows 8, so
this inclusion cannot be unconditional

Warning in w32os.c:

 src/w32/w32os.c: In function 'osync_get_mutex':
 src/w32/w32os.c:434:32: warning: cast from pointer to integer of different 
size [-Wpointer-to-int-cast]
   434 |   sprintf (mutex, "0x%Ix", (unsigned long long)osync_handle);
   |^
 src/w32/w32os.c: In function 'osync_parse_mutex':
 src/w32/w32os.c:454:18: warning: cast to pointer from integer of different 
size [-Wint-to-pointer-cast]
   454 |   osync_handle = (HANDLE) i;
   |  ^

I fixed those by casting to DWORD_PTR first.

Here are the proposed patches:

--- src/function.c~02022-09-18 22:06:17.0 +0300
+++ src/function.c  2022-09-24 13:30:01.740875000 +0300
@@ -823,6 +823,15 @@ func_wordlist (char *o, char **argv, con
   stop = parse_numeric (argv[1],
 _("invalid second argument to 'wordlist' function"));
 
+#ifdef WINDOWS32
+  if (start < 1)
+ON (fatal, *expanding_var,
+"invalid first argument to 'wordlist' function: '%I64d'", start);
+
+  if (stop < 0)
+ON (fatal, *expanding_var,
+"invalid second argument to 'wordlist' function: '%I64d'", stop);
+#else
   if (start < 1)
 ON (fatal, *expanding_var,
 "invalid first argument to 'wordlist' function: '%lld'", start);
@@ -830,6 +839,7 @@ func_wordlist (char *o, char **argv, con
   if (stop < 0)
 ON (fatal, *expanding_var,
 "invalid second argument to 'wordlist' function: '%lld'", stop);
+#endif
 
   count = stop - start + 1;
 

--- src/w32/pathstuff.c~0   2022-07-07 22:46:05.0 +0300
+++ src/w32/pathstuff.c 2022-09-24 13:45:53.303375000 +0300
@@ -102,11 +102,11 @@ w32ify(const char *filename, int resolve
 if (resolve)
   {
 char *fp = _fullpath (NULL, filename, sizeof (w32_path));
-strncpy (w32_path, fp, sizeof (w32_path));
+strncpy (w32_path, fp, sizeof (w32_path) - 1);
 free (fp);
   }
 else
-  strncpy(w32_path, filename, sizeof (w32_path));
+  strncpy(w32_path, filename, sizeof (w32_path) - 1);
 
 for (p = w32_path; p && *p; 

Re: GNU make 4.3.90 release candidate available

[Eli Zaretskii]

I've built this with mingw.org's MinGW for native MS-Windows.  I found
several issues during the build, some of them specific to Windows,
others more general.  (I didn't yet run the test suite, I intend to do
that next.)

Here are the build-time issues that aren't specific to MS-Windows:

 src/getopt.c: In function '_getopt_internal':
 src/getopt.c:679:8: warning: suggest explicit braces to avoid ambiguous 
'else' [-Wdangling-else]
   679 | if (opterr)
   |^

(This is not new to this release, I saw the same in Make 4.3.)  I
ignored this one.

 src/implicit.c: In function 'pattern_search':
 src/implicit.c:237:9: warning: unused variable 'dend' [-Wunused-variable]
   237 |   char *dend = depname + deplen;
   | ^~~~

This variable is only used in assertions; the proposed patch is below.

 src/main.c: In function 'main':
 src/main.c:2561:26: warning: variable 'len' set but not used 
[-Wunused-but-set-variable]
  2561 |   size_t len;
   |  ^~~

Same here: the variable is only used in an assertion; the proposed
patch is below.

Here are the proposed patches for the above 2 warnings.  Paul, if you
agree with them, I will install them in the repository.

--- src/implicit.c~02022-09-24 13:36:21.490875000 +0300
+++ src/implicit.c  2022-09-24 13:37:14.16275 +0300
@@ -234,7 +234,9 @@ pattern_search (struct file *file, int a
  We may replace % by $(*F) for second expansion, increasing the length.  */
   size_t deplen = namelen + max_pattern_dep_length + 4;
   char *depname = alloca (deplen);
+#ifndef NDEBUG
   char *dend = depname + deplen;
+#endif
 
   /* The start and length of the stem of FILENAME for the current rule.  */
   const char *stem = 0;

--- src/main.c~02022-09-20 10:55:39.0 +0300
+++ src/main.c  2022-09-24 13:40:33.584625000 +0300
@@ -2558,13 +2558,11 @@ main (int argc, char **argv, char **envp
 
   for (; *av; ++av, ++nv)
 {
-  size_t len;
   char *f;
   char *a = *av;
   const char *mf = makefiles->list[mfidx];
 
-  len = strlen (a);
-  assert (len > 0);
+  assert (strlen (a) > 0);
 
   *nv = a;
 

4.3.90 release candidate segfaults on linux and solaris

[Denis Excoffier]

Hello,

At the beginning, i found a segfault in the source code of autoconf-2.71.

I reduced the case and the current test Makefile is as follows:

% cat Makefile
export PATH = $(shell echo "`pwd`/tests:$$PATH")
foo = $(shell echo yes)

all:
echo $(foo)
%

In my specific configuration (under linux, with --disable-nls,
--disable-load, without using -j, using 'env -i make -d -n'), a
segfault always occurs around line 118 of src/expand.c:

for (ep = environ; *ep != 0; ++ep)
  if ((*ep)[nl] == '=' && strncmp (*ep, v->name, nl) == 0)
return xstrdup ((*ep) + nl + 1);

This is new code, which seems ok, but segfault occurs because in this
environment, some *ep, while not null, is definitely not a
string (e.g. i found *ep = 0x18).

I suppose the environment got corrupted before that. Indeed, several
binary chars appear in the debug printouts with my instrumented code.
However, i was not able to figure out the root cause.

One way to circumvent temporarily the problem is
to comment out the two instances of free_childbase().
After that, the segfault completely disappears, even
with -j, and also under all the tests i can perform here in
compiling many GNU projects and also my own tools.

My linux is old (2.6.32), solaris is solaris 10 (old too...)
Under MacOS (12.6) and Cygwin (3.3.6) no segfault at all.

Hoping someone will be able to track this further.

Regards,

Denis Excoffier.