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

 
 

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

Submitter:  Thomas Petazzoni <tpetazzoni>
Submitted:  Thu 10 Sep 2020 03:35:12 PM UTC
   
 
Severity:  3 - Normal Item Group:  None
Status:  Fixed Privacy:  Public
Assigned to:  psmith Open/Closed:  Closed
Component Version:  None Operating System:  Any
Fixed Release:  4.4 Triage Status:  Medium Effort
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 19 May 2022 06:43:56 AM UTC, comment #10: 

This bug has been fixed more than one year ago. Would it be possible to publish a new release of GNU Make that contains this fix? Even if it's a make 4.3.1 minor release, for example. Thanks!

Thomas Petazzoni <tpetazzoni>
Mon 15 Mar 2021 06:55:09 AM UTC, comment #9: 

Thanks for testing.  I'm closing this issue as fixed.  Cheers!

Paul D. Smith <psmith>
Group administrator
Wed 20 Jan 2021 05:02:06 PM UTC, comment #8: 


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


I have tested the SV-59093-1.patch, applied on top of make-4.3-2.fc33.src.rpm.
Test case is running "make printvars VARS=TOPDIR" inside buildroot-2020.11.

The segfault no longer occurs with this patch applied.
No difference is observed in "make test" output.

So looks good to me.

Ralph Siemsen <rfs613>
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>
Group 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>
Group 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>
Group 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 patrickdepinguin
  • -email is unavailable- added by rfs613 (Posted a comment)
  • -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)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-03-15 psmith StatusNone Fixed
        Assigned toNone psmith
        Open/ClosedOpen Closed
        Operating SystemNone Any
        Fixed ReleaseNone 4.4
        Triage StatusNone Medium Effort
    2021-01-22 patrickdepinguin Carbon-CopyRemoved -email is unavailable- -
    2021-01-22 patrickdepinguin Carbon-Copy- Added patrickdepinguin
    2021-01-22 patrickdepinguin Carbon-Copy- Added -email is unavailable-
    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.13-d3ae.
    Corresponding source code