bugmake - Bugs: bug #59093, Segmentation fault regression in...

 
 

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

Submitted by:  Thomas Petazzoni <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
Component Version:  None Operating System:  None
Fixed Release:  None Triage Status:  None

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Thu 12 Nov 2020 10:08:18 PM UTC, comment #7: 

I don't like the idea of adding new variables to control stack limit sizes.  That's just too esoteric for anyone who just wants to run  make to deal with.  This should be invisible to users.

Given that the situation you have is not actually a large recursion of functions, I looked at the implementation of filter/filter-out and it's just a stupid implementation which is probably slow as well.

I've added a new patch that rewrites this to be smarter and not use lots of stack.  Please remove the previous patches that try to reset the stack size, and apply SV-59093-1.patch and see if that fixes your problem.

(file #50264)

Paul D. Smith <psmith>
Project Administrator
Thu 12 Nov 2020 08:23:42 PM UTC, comment #6: 

Any news about this bug ?

Thomas Petazzoni <tpetazzoni>
Sat 19 Sep 2020 08:07:33 PM UTC, comment #5: 

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.

Dmitry Goncharov <dgoncharov>
Sun 13 Sep 2020 12:29:04 PM UTC, comment #4: 

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!

Paul D. Smith <psmith>
Project Administrator
Sun 13 Sep 2020 12:23:58 PM UTC, comment #3: 

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.

Paul D. Smith <psmith>
Project Administrator
Sun 13 Sep 2020 03:32:21 AM UTC, comment #2: 

This patch in the attachment solves the issue.

Dmitry Goncharov <dgoncharov>
Sun 13 Sep 2020 03:30:35 AM UTC, comment #1: 

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

Dmitry Goncharov <dgoncharov>
Thu 10 Sep 2020 03:35:12 PM UTC, original submission:  

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).

Thomas Petazzoni <tpetazzoni>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #50264:  SV-59093-1.patch added by psmith (7KiB - text/x-patch - Rewrite filter/filter-out)
file #49822:  defss.diff added by dgoncharov (2KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by psmith (Posted a comment)
  • -email is unavailable- added by dgoncharov (Posted a comment)
  • -email is unavailable- added by tpetazzoni (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-11-12 psmith Attached File- => Added SV-59093-1.patch, #50264
    2020-09-19 dgoncharov Attached File- => Added defss.diff, #49822
    2020-09-13 dgoncharov Attached File- => Added sv59093_set_stack_size.diff, #49791

    Back to the top


    Powered by Savane 3.5