[bug #59093] Segmentation fault regression in make 4.3 vs. 4.2.1

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[bug #59093] Segmentation fault regression in make 4.3 vs. 4.2.1

David Boyce-5
URL:
  <https://savannah.gnu.org/bugs/?59093>

                 Summary: Segmentation fault regression in make 4.3 vs. 4.2.1
                 Project: make
            Submitted by: tpetazzoni
            Submitted on: Thu 10 Sep 2020 03:35:12 PM UTC
                Severity: 3 - Normal
              Item Group: None
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
       Component Version: None
        Operating System: None
           Fixed Release: None
           Triage Status: None

    _______________________________________________________

Details:

GNU Make has a segmentation fault regression in make 4.3, where the exact same
make code works fine with make 4.2.1.

The stack trace looks like this:

Program received signal SIGSEGV, Segmentation fault.
0x000055e0fca8530d in func_filter_filterout (o=0x55e10693b400 "",
argv=<optimized out>, funcname=<optimized out>)
    at ../../src/function.c:1014
1014 ../../src/function.c: No such file or directory.
(gdb)
(gdb) bt
#0  0x000055e0fca8530d in func_filter_filterout (o=0x55e10693b400 "",
argv=<optimized out>, funcname=<optimized out>)
    at ../../src/function.c:1014
#1  0x000055e0fca86261 in handle_function (op=op@entry=0x7fff1e9c2e70,
stringp=stringp@entry=0x7fff1e9c2e68)
    at ../../src/function.c:2544
#2  0x000055e0fca7fe2f in variable_expand_string (line=<optimized out>,
line@entry=0x0, string=<optimized out>,
    length=length@entry=18446744073709551615) at ../../src/expand.c:258
#3  0x000055e0fca7fc16 in variable_expand (line=<optimized out>) at
../../src/expand.c:566
#4  variable_expand_for_file (file=0x0, line=<optimized out>) at
../../src/expand.c:464
#5  variable_expand_for_file (file=0x0, line=<optimized out>) at
../../src/expand.c:457
#6  allocated_variable_expand_for_file (file=0x0, line=<optimized out>) at
../../src/expand.c:566
#7  expand_argument (str=0x55e106978b98 "$(filter $(VARS),$(.VARIABLES)))",
end=end@entry=0x55e106978bb7 ")")
    at ../../src/expand.c:446
#8  0x000055e0fca8639c in handle_function (op=op@entry=0x7fff1e9c3040,
stringp=stringp@entry=0x7fff1e9c3038)
    at ../../src/function.c:2512
#9  0x000055e0fca7fe2f in variable_expand_string (line=<optimized out>,
line@entry=0x0,
    string=string@entry=0x55e1069cfbb2 " $(sort $(filter
$(VARS),$(.VARIABLES)))",
    length=length@entry=18446744073709551615) at ../../src/expand.c:258
#10 0x000055e0fca7fc7a in variable_expand (line=0x55e1069cfbb2 " $(sort
$(filter $(VARS),$(.VARIABLES)))")
    at ../../src/expand.c:566
#11 variable_expand_for_file (file=0x0, line=0x55e1069cfbb2 " $(sort $(filter
$(VARS),$(.VARIABLES)))")
    at ../../src/expand.c:464
#12 variable_expand_for_file (file=0x0, line=0x55e1069cfbb2 " $(sort $(filter
$(VARS),$(.VARIABLES)))")
    at ../../src/expand.c:457
#13 allocated_variable_expand_for_file (file=0x0, line=0x55e1069cfbb2 " $(sort
$(filter $(VARS),$(.VARIABLES)))")
    at ../../src/expand.c:566
#14 expand_argument (str=0x55e1069cfbb2 " $(sort $(filter
$(VARS),$(.VARIABLES)))", end=end@entry=0x0)
    at ../../src/expand.c:436
#15 0x000055e0fca83fb1 in func_foreach (o=0x55e10693a830 "",
argv=0x7fff1e9c3150, funcname=<optimized out>)
    at ../../src/function.c:868
#16 0x000055e0fca86261 in handle_function (op=op@entry=0x7fff1e9c32c0,
stringp=stringp@entry=0x7fff1e9c32b8)
    at ../../src/function.c:2544
#17 0x000055e0fca7fe2f in variable_expand_string (line=<optimized out>,
line@entry=0x0, string=<optimized out>,
    length=length@entry=18446744073709551615) at ../../src/expand.c:258
#18 0x000055e0fca80792 in variable_expand (line=<optimized out>) at
../../src/expand.c:475
#19 variable_expand_for_file (line=<optimized out>, file=<optimized out>) at
../../src/expand.c:475
#20 0x000055e0fca807f4 in allocated_variable_expand_for_file (line=<optimized
out>, file=file@entry=0x55e0fe9d4280)
    at ../../src/expand.c:566
#21 0x000055e0fca8bc75 in new_job (file=0x55e0fe9d4280) at
../../src/job.c:1808
#22 0x000055e0fca9746c in remake_file (file=0x55e0fe9d4280) at
../../src/remake.c:1234
#23 update_file_1 (depth=<optimized out>, file=0x55e0fe9d4280) at
../../src/remake.c:835
#24 update_file (file=file@entry=0x55e0fe9d4280, depth=<optimized out>) at
../../src/remake.c:336
#25 0x000055e0fca97dc4 in update_goal_chain (goaldeps=<optimized out>) at
../../src/remake.c:151
#26 0x000055e0fca7bf25 in main (argc=<optimized out>, argv=<optimized out>,
envp=<optimized out>)
    at ../../src/main.c:2589

To reproduce:

$ git clone https://github.com/buildroot/buildroot.git
$ cd buildroot
$ make BR2_HAVE_DOT_CONFIG=y -s printvars VARS=%_LICENSE

Works with make 4.2.1 (and produces the expected output), segfaults with make
4.3 (with the above stack trace captured using gdb).




    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?59093>

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


Reply | Threaded
Open this post in threaded view
|

[bug #59093] Segmentation fault regression in make 4.3 vs. 4.2.1

David Boyce-5
Follow-up Comment #1, bug #59093 (project make):

This is caused by stack overflow.
A change introduced in commit 4f3a41c60a02f6df9fc0725698ade64825907822
prevents setting stack size if posix_spawn is used.

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?59093>

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


Reply | Threaded
Open this post in threaded view
|

[bug #59093] Segmentation fault regression in make 4.3 vs. 4.2.1

David Boyce-5
Additional Item Attachment, bug #59093 (project make):

File name: sv59093_set_stack_size.diff    Size:0 KB
   
<https://file.savannah.gnu.org/file/sv59093_set_stack_size.diff?file_id=49791>



    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?59093>

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


Reply | Threaded
Open this post in threaded view
|

[bug #59093] Segmentation fault regression in make 4.3 vs. 4.2.1

David Boyce-5
Follow-up Comment #2, bug #59093 (project make):

This patch in the attachment solves the issue.

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?59093>

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


Reply | Threaded
Open this post in threaded view
|

[bug #59093] Segmentation fault regression in make 4.3 vs. 4.2.1

David Boyce-5
Follow-up Comment #3, bug #59093 (project make):

I don't think I agree with this change.  It's one thing for make to increase
it's own stack size, but it's not good for make to be resetting the stack size
of processes it starts.  That could result in all sorts of issues.

If the user invokes make and the stack size they specify is too small then
maybe they should increase the stack size before invoking make (they can do
this with ulimit -s before invoking make).  We could use sigaltstack() to
allow us to catch this signal and at least print a useful message to the
user.

Alternatively, maybe it's possible for make to dynamically reset the stack
limit before invoking posix_spawn().  It's kind of gross but the reality is
that at the time when we spawn a new process our stack is almost certainly
very small.  I don't know, really, what performance or other issues might
ensue from constantly resetting the stack up and down.

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?59093>

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


Reply | Threaded
Open this post in threaded view
|

[bug #59093] Segmentation fault regression in make 4.3 vs. 4.2.1

David Boyce-5
Follow-up Comment #4, bug #59093 (project make):

Of course another solution is to work on reducing the stack needed by make.
None of this will be cheap of course.  Some ideas: switch more places from
alloca() to malloc(), and/or convert some algorithms from recursive function
calls to loops.

I don't see any good way to avoid function calls being recursive unless we're
willing to rewrite essentially the entire variable expansion, including all
function call expansion, into one huge function ... scary!

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?59093>

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


Reply | Threaded
Open this post in threaded view
|

[bug #59093] Segmentation fault regression in make 4.3 vs. 4.2.1

David Boyce-5
Additional Item Attachment, bug #59093 (project make):

File name: defss.diff                     Size:2 KB
    <https://file.savannah.gnu.org/file/defss.diff?file_id=49822>



    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?59093>

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


Reply | Threaded
Open this post in threaded view
|

[bug #59093] Segmentation fault regression in make 4.3 vs. 4.2.1

David Boyce-5
Follow-up Comment #5, bug #59093 (project make):

The only issue i encountered with make children inheriting a high value of
RLIMIT_STACK was a 32 bit compiler running out of heap when compiling a large
file.

i attached another patch.
This patch sets stack limit to a default hardcoded value when posix_spawn is
enabled. The default is 128Mb. This deprives a 32 bit compiler of 128 Mb out
of its ~2Gb of space.

For those (if any) makefiles which need more than 128Mb of stack we can
introduce 2 new make functions "setrlimit" and "getrlimit". The makefile could
then call "setrlimit". This will relieve the users from the crash and from
having to know the reason of the crash.
Paul, if you think we should add "setrlimit" and "getrlimit" functions please
let me know, i'll submit a patch.


> If the user invokes make and the stack size they specify is too small then
maybe they should increase the stack size before invoking make (they can do
this with ulimit -s before invoking make).

The user won't know what causes the crash.


> Alternatively, maybe it's possible for make to dynamically reset the stack
limit before invoking posix_spawn().

make would have to know the amount of stack occupied to set this smaller
limit.
Also, what to do is this occupied amount is large?


> I don't see any good way to avoid function calls being recursive

Another option is to keep recursion, but rewrite the code to allow tail call
optimization.


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?59093>

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