Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Too Many Keyboards! → make stackdump #13416

Closed
elfmimi opened this issue Jul 2, 2021 · 9 comments
Closed

[Bug] Too Many Keyboards! → make stackdump #13416

elfmimi opened this issue Jul 2, 2021 · 9 comments

Comments

@elfmimi
Copy link
Contributor

elfmimi commented Jul 2, 2021

Describe the Bug

We were bound to hit this problem. and today is that day.

make stops with a very weird error message. like...

QMK Firmware 0.13.16
      0 [] make 9468 cygwin_exception::open_stackdumpfile: Dumping stack trace to make.exe.stackdump
☒ Invalid return_code: 35584

System Information

  • Keyboard: maybe affect all keyboards . tested with crkbd/rev1:default
  • Operating system: Windows 10 + QMK MSYS
  • QMK Firmware version: 0.13.16 11a406f

Additional Context

Reducing the number of keyboards in the working repository helps. I mean, if you leave only the directories for keyboards you need under 'keyboards' directory and delete all others, then it will compile.

for advanced git users, it is a good opportunity to tryout git separse-checkout. example follows...

git sparse-checkout init
git sparse-checkout set '/*' '!/keyboards' /keyboards/crkbd
@shelaf
Copy link
Contributor

shelaf commented Jul 2, 2021

Additional information:
The make command returns no useful information. It just creates make.exe.stackdump file.

$ pwd
/home/shela/qmk_firmware

$ ls make.exe.stackdump
ls: cannot access 'make.exe.stackdump': No such file or directory

$ make crkbd/rev1:default
QMK Firmware 0.13.16

$ echo $?
0

$ ls make.exe.stackdump
make.exe.stackdump

@fauxpark
Copy link
Member

fauxpark commented Jul 2, 2021

This issue AFAIK only occurs in MSYS2 (ie. it is a problem with MSYS make or something about this specific environment) and relates to the following line:

else ifeq ($$(call TRY_TO_MATCH_RULE_FROM_LIST,$$(shell util/list_keyboards.sh | sort -u)),true)

Apparently, there is some kind of size limit on arguments passed to call. Replacing the

util/list_keyboards.sh | sort -u

with

qmk list-keyboards

will work for the moment, as it lists slightly fewer boards. It does not take into account .noci or boards which are technically valid due to presence of a rules.mk with DEFAULT_FOLDER=<foo>.

So, it is only a bandaid to fix a larger problem, which is... that the Makefile logic is kinda horrible spaghetti code forcing make to do things it probably wasn't designed for.

@xyzz
Copy link
Contributor

xyzz commented Jul 5, 2021

The issue is stack overflow in make due to extremely deep recursion. As can be seen in gdb session (ignore me typing the keyboard name wrong):

make1
make2
make3

Default msys2 make has 2MB stack size, as can be seen by running dumpbin on it:

OPTIONAL HEADER VALUES
             20B magic # (PE32+)
            2.36 linker version
           22800 size of code
           2FA00 size of initialized data
            3C00 size of uninitialized data
            1000 entry point (0000000100401000)
            1000 base of code
       100400000 image base (0000000100400000 to 00000001004EEFFF)
            1000 section alignment
             200 file alignment
            4.00 operating system version
            0.00 image version
            5.02 subsystem version
               0 Win32 version
           EF000 size of image
             600 size of headers
           FD280 checksum
               3 subsystem (Windows CUI)
            8000 DLL characteristics
                   Terminal Server Aware
          200000 size of stack reserve
            1000 size of stack commit
          100000 size of heap reserve
            1000 size of heap commit
               0 loader flags
              10 number of directories
               0 [       0] RVA [size] of Export Directory
           36000 [    11BC] RVA [size] of Import Directory
           38000 [     4E8] RVA [size] of Resource Directory
           2E000 [    1350] RVA [size] of Exception Directory
               0 [       0] RVA [size] of Certificates Directory
           39000 [     4C0] RVA [size] of Base Relocation Directory
           2D000 [      1C] RVA [size] of Debug Directory
               0 [       0] RVA [size] of Architecture Directory
               0 [       0] RVA [size] of Global Pointer Directory
               0 [       0] RVA [size] of Thread Storage Directory
               0 [       0] RVA [size] of Load Configuration Directory
               0 [       0] RVA [size] of Bound Import Directory
           36488 [     438] RVA [size] of Import Address Table Directory
               0 [       0] RVA [size] of Delay Import Directory
               0 [       0] RVA [size] of COM Descriptor Directory
               0 [       0] RVA [size] of Reserved Directory

We can change this easily with editbin, for example for 128M stack:

editbin /stack:134217728 usr/bin/make.exe

(FWIW, it doesn't seem to waste any physical memory until the corresponding pages are used, so a large value like 128M should be OK).

Here's an edited make.exe you can drop in C:\QMK_MSYS\usr\bin to fix this: https://cdn.discordapp.com/attachments/798171873951219754/861743181439172638/make.exe

or alternatively, qmk-msys could provide a patched version with a larger stack

@elfmimi elfmimi changed the title [Bug] Too Many Keyboards! [Bug] Too Many Keyboards! → make stackdump Jul 18, 2021
@elfmimi
Copy link
Contributor Author

elfmimi commented Jul 18, 2021

QMK MSYS Release 1.4.4 should fix this issue by increasing the stack size of make.exe to 128MB.
https://github.com/qmk/qmk_distro_msys/releases/tag/1.4.4

@ZigMeowNyan
Copy link

Ran into this today and gave a shot at an MSYS2 bug report explaining how to fix the issue at compile/link time instead of patching after the fact with a Windows SDK binary.

@zvecr
Copy link
Member

zvecr commented Feb 28, 2022

Should be fixed now #15988 is merged (master@0.16.0).

@zvecr zvecr closed this as completed Feb 28, 2022
@Bobstin
Copy link

Bobstin commented Jul 14, 2022

Apologies for reviving a closed issue, but I am getting this issue with MSYS 1.7.1 and QMK firmware 0.15.16

@zvecr
Copy link
Member

zvecr commented Jul 14, 2022

0.15.16 < 0.16.0

Update your repo, or use one of the mitigations above.

@Bobstin
Copy link

Bobstin commented Jul 14, 2022

🤦 Thought this was a MSYS issue and updated that. Thanks for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants