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

compiler: don't use types in the global context #3917

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

aykevl
Copy link
Member

@aykevl aykevl commented Sep 18, 2023

This usually works by chance, but leads to crashes. So we should never ever do this.

I'm pretty sure this is the crash behind this issue: #3894

It may also have caused this crash: #3874

I have a suspicion this is also behind the rather crash-prone CircleCI jobs, that we haven't been able to find the source of. But we'll find out soon enough once this fix is merged.

To avoid hitting this issue again in the future, I've created a PR to remove these dangerous functions altogether from the go-llvm API: tinygo-org/go-llvm#54

This usually works by chance, but leads to crashes. So we should never
ever do this.

I'm pretty sure this is the crash behind this issue: #3894

It may also have caused this crash: #3874

I have a suspicion this is also behind the rather crash-prone CircleCI
jobs, that we haven't been able to find the source of. But we'll find
out soon enough once this fix is merged.

To avoid hitting this issue again in the future, I've created a PR to
remove these dangerous functions altogether from the go-llvm API:
tinygo-org/go-llvm#54
This was referenced Sep 18, 2023
@@ -147,7 +147,7 @@ func (c *compilerContext) getTypeCode(typ types.Type) llvm.Value {
c.addError(token.NoPos, fmt.Sprintf("too many levels of pointers for typecode: %s", typstr))
}
return llvm.ConstGEP(c.ctx.Int8Type(), ptr, []llvm.Value{
llvm.ConstInt(llvm.Int32Type(), 1, false),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgryski I believe this was you. Just so you know, the llvm.*Type() methods should never be used (see #1744 for another instance and tinygo-org/go-llvm#54 where I remove this function).

@github-actions
Copy link

Size difference with the dev branch:

Binary size difference
not the same command!
    go: downloading golang.org/x/text v0.3.6
    tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/ntpclient/main.go
not the same command!
    go: downloading github.com/eclipse/paho.mqtt.golang v1.2.0
    tinygo build -size short -o ./build/test.hex -target=wioterminal ./examples/rtl8720dn/mqttsub/
 before   after   diff
  60688   60688      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/adt7410/main.go
   9684    9684      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/adxl345/main.go
  13260   13260      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pybadge ./examples/amg88xx
   8776    8776      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/apa102/main.go
  11720   11720      0   0.00%  tinygo build -size short -o ./build/test.hex -target=nano-33-ble ./examples/apds9960/proximity/main.go
  10392   10392      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/apa102/itsybitsy-m0/main.go
   8196    8196      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/at24cx/main.go
   8308    8308      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bh1750/main.go
   7608    7608      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/blinkm/main.go
  63260   63260      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bmi160/main.go
  27868   27868      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bmp180/main.go
  63272   63272      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bmp280/main.go
  12432   12432      0   0.00%  tinygo build -size short -o ./build/test.hex -target=trinket-m0 ./examples/bmp388/main.go
   7708    7708      0   0.00%  tinygo build -size short -o ./build/test.hex -target=bluepill ./examples/ds1307/sram/main.go
  22008   22008      0   0.00%  tinygo build -size short -o ./build/test.hex -target=bluepill ./examples/ds1307/time/main.go
  68964   68964      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/ds3231/main.go
   4708    4708      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/easystepper/main.go
  24876   24876      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/espat/espconsole/main.go
  25104   25104      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/espat/esphub/main.go
  24860   24860      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/espat/espstation/main.go
  68684   68684      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/flash/console/spi
  64636   64636      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/flash/console/qspi
   7052    7052      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/gc9a01/main.go
  67956   67956      0   0.00%  tinygo build -size short -o ./build/test.hex -target=feather-m0 ./examples/gps/i2c/main.go
  68424   68424      0   0.00%  tinygo build -size short -o ./build/test.hex -target=feather-m0 ./examples/gps/uart/main.go
   8424    8424      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/hcsr04/main.go
   5712    5712      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/hd44780/customchar/main.go
   5756    5756      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/hd44780/text/main.go
  10616   10616      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/hd44780i2c/main.go
  14624   14624      0   0.00%  tinygo build -size short -o ./build/test.hex -target=nano-33-ble ./examples/hts221/main.go
  16904   16904      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/hub75/main.go
  10048   10048      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/basic
  10884   10884      0   0.00%  tinygo build -size short -o ./build/test.hex -target=xiao ./examples/ili9341/basic
  29044   29044      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/pyportal_boing
  10076   10076      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/scroll
  10960   10960      0   0.00%  tinygo build -size short -o ./build/test.hex -target=xiao ./examples/ili9341/scroll
 263524  263524      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/slideshow
  12092   12092      0   0.00%  tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/lis3dh/main.go
  14028   14028      0   0.00%  tinygo build -size short -o ./build/test.hex -target=nano-33-ble ./examples/lps22hb/main.go
  26124   26124      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/lsm303agr/main.go
  12576   12576      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/lsm6ds3/main.go
  10952   10952      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mag3110/main.go
  10200   10200      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp23017/main.go
  10628   10628      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp23017-multiple/main.go
   9868    9868      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp3008/main.go
  66772   66772      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp2515/main.go
  22952   22952      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/microbitmatrix/main.go
  22944   22944      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit-v2 ./examples/microbitmatrix/main.go
   8532    8532      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mma8653/main.go
   8316    8316      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mpu6050/main.go
  74988   74988      0   0.00%  tinygo build -size short -o ./build/test.hex -target=p1am-100 ./examples/p1am/main.go
  12152   12152      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pico ./examples/pca9685/main.go
   6084    6084      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/pcd8544/setbuffer/main.go
   5104    5104      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/pcd8544/setpixel/main.go
   2681    2681      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino ./examples/servo
   7944    7944      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pybadge ./examples/shifter/main.go
  56368   56368      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/sht3x/main.go
  56340   56340      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/shtc3/main.go
   6452    6452      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/ssd1306/i2c_128x32/main.go
   5940    5940      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/ssd1306/spi_128x64/main.go
   5704    5704      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/ssd1331/main.go
   6392    6392      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/st7735/main.go
   6292    6292      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/st7789/main.go
  17212   17212      0   0.00%  tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/thermistor/main.go
  10328   10328      0   0.00%  tinygo build -size short -o ./build/test.hex -target=circuitplay-bluefruit ./examples/tone
  10728   10728      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/tm1637/main.go
   9404    9404      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/touch/resistive/fourwire/main.go
  12460   12460      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/touch/resistive/pyportal_touchpaint/main.go
  15772   15772      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/vl53l1x/main.go
  13812   13812      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/vl6180x/main.go
   6460    6460      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/waveshare-epd/epd2in13/main.go
   6012    6012      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/waveshare-epd/epd2in13x/main.go
   6288    6288      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/waveshare-epd/epd4in2/main.go
 137184  137184      0   0.00%  go: downloading golang.org/x/text v0.3.6
 137176  137176      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/udpstation/main.go
 137500  137500      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/tcpclient/main.go
 137600  137600      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/webclient/main.go
   6976    6976      0   0.00%  tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/ws2812
   5326    5326      0   0.00%  tinygo build -size short -o ./build/test.bin -target=m5stamp-c3          ./examples/ws2812
  61656   61656      0   0.00%  tinygo build -size short -o ./build/test.hex -target=feather-nrf52840 ./examples/is31fl3731/main.go
   1565    1565      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino   ./examples/ws2812
    880     880      0   0.00%  tinygo build -size short -o ./build/test.hex -target=digispark ./examples/ws2812
  32296   32296      0   0.00%  tinygo build -size short -o ./build/test.hex -target=trinket-m0 ./examples/bme280/main.go
  16660   16660      0   0.00%  tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/microphone/main.go
  11336   11336      0   0.00%  tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/buzzer/main.go
  13044   13044      0   0.00%  tinygo build -size short -o ./build/test.hex -target=trinket-m0 ./examples/veml6070/main.go
   6868    6868      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/l293x/simple/main.go
   8788    8788      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/l293x/speed/main.go
   6832    6832      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/l9110x/simple/main.go
   9448    9448      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/l9110x/speed/main.go
   7344    7344      0   0.00%  tinygo build -size short -o ./build/test.hex -target=nucleo-f103rb ./examples/shiftregister/main.go
   7084    7084      0   0.00%  tinygo build -size short -o ./build/test.hex -target=hifive1b ./examples/ssd1351/main.go
  13224   13224      0   0.00%  tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/lis2mdl/main.go
   8472    8472      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/max72xx/main.go
  77416   77416      0   0.00%  tinygo build -size short -o ./build/test.hex -target=feather-m0 ./examples/dht/main.go
  70928   70928      0   0.00%  tinygo build -size short -o ./build/test.hex -target=xiao ./examples/pcf8563/alarm/
   7452    7452      0   0.00%  tinygo build -size short -o ./build/test.hex -target=xiao ./examples/pcf8563/clkout/
  70440   70440      0   0.00%  tinygo build -size short -o ./build/test.hex -target=xiao ./examples/pcf8563/time/
  70800   70800      0   0.00%  tinygo build -size short -o ./build/test.hex -target=xiao ./examples/pcf8563/timer/
  12096   12096      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pico ./examples/qmi8658c/main.go
   8968    8968      0   0.00%  tinygo build -size short -o ./build/test.hex -target=feather-m0 ./examples/ina260/main.go
   9280    9280      0   0.00%  tinygo build -size short -o ./build/test.hex -target=nucleo-l432kc ./examples/aht20/main.go
  72008   72008      0   0.00%  tinygo build -size short -o ./build/test.hex -target=feather-m4 ./examples/sdcard/console/
  82532   82532      0   0.00%  tinygo build -size short -o ./build/test.hex -target=wioterminal ./examples/rtl8720dn/webclient/
  71924   71924      0   0.00%  tinygo build -size short -o ./build/test.hex -target=wioterminal ./examples/rtl8720dn/webserver/
  98452   98452      0   0.00%  go: downloading github.com/eclipse/paho.mqtt.golang v1.2.0
  60292   60292      0   0.00%  tinygo build -size short -o ./build/test.hex -target=feather-m4 ./examples/i2csoft/adt7410/
  10164   10164      0   0.00%  tinygo build -size short -o ./build/test.elf -target=wioterminal ./examples/axp192/m5stack-core2-blinky/
   8924    8924      0   0.00%  tinygo build -size short -o ./build/test.uf2 -target=pico ./examples/xpt2046/main.go
  14560   14560      0   0.00%  tinygo build -size short -o ./build/test.hex -target=nucleo-wl55jc ./examples/sx126x/lora_rxtx/
  25876   25876      0   0.00%  tinygo build -size short -o ./build/test.uf2 -target=pico ./examples/ssd1289/main.go
  11204   11204      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pico ./examples/irremote/main.go
  11220   11220      0   0.00%  tinygo build -size short -o ./build/test.hex -target=badger2040 ./examples/uc8151/main.go
  10308   10308      0   0.00%  tinygo build -size short -o ./build/test.uf2 -target=pico ./examples/scd4x/main.go
   8744    8744      0   0.00%  tinygo build -size short -o ./build/test.uf2 -target=circuitplay-express ./examples/makeybutton/main.go
   9608    9608      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/ds18b20/main.go
  81220   81220      0   0.00%  tinygo build -size short -o ./build/test.hex -target=nucleo-wl55jc ./examples/lora/lorawan/atcmd/
  15740   15740      0   0.00%  tinygo build -size short -o ./build/test.uf2 -target=pico ./examples/as560x/main.go
   9800    9800      0   0.00%  tinygo build -size short -o ./build/test.uf2 -target=pico ./examples/mpu6886/main.go
   7892    7892      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/ttp229/main.go
3459024 3459024      0   0.00%   sum

@aykevl
Copy link
Member Author

aykevl commented Sep 18, 2023

Checked CircleCI crashes and this appears to be the bug behind at least some of those crashes. Perhaps most (but it's hard to say). At least, I see it crashing on one of the lines where I've made a fix.

@dgryski
Copy link
Member

dgryski commented Sep 18, 2023

Thanks for fixing those. Whenever I delve into the llvm stuff I always feel out of my depth..

@deadprogram deadprogram added this to the v0.30.0 milestone Sep 19, 2023
@deadprogram
Copy link
Member

Now merging, thanks @aykevl and @dgryski for review.

@deadprogram deadprogram merged commit 42da765 into dev Sep 19, 2023
22 checks passed
@deadprogram deadprogram deleted the avoid-global-context branch September 19, 2023 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants