Heap allocations in interrupts

[aykevl]

There are a number of heap allocations in interrupts, especially in USB-CDC code. I'm not very familiar with this code, so maybe someone more familiar can take a look?

All these issues are latent bugs that may not normally cause problems but can definitely cause memory corruption eventually (and will be near-impossible to find when they happen).

1. ATsamd21

Here is one heap allocation on the ATsamd21:

https://github.com/tinygo-org/tinygo/blob/ffeff5570692c019c734e3ca28db85bc48430f7f/src/machine/machine_atsamd21.go#L1944

Which is called from here:

https://github.com/tinygo-org/tinygo/blob/ffeff5570692c019c734e3ca28db85bc48430f7f/src/machine/machine_atsamd21.go#L1865

Which is called from this interrupt:

https://github.com/tinygo-org/tinygo/blob/ffeff5570692c019c734e3ca28db85bc48430f7f/src/machine/machine_atsamd21.go#L1619

2. ATsamd51

Here is a heap allocation on the ATsamd51:

https://github.com/tinygo-org/tinygo/blob/ffeff5570692c019c734e3ca28db85bc48430f7f/src/machine/machine_atsamd51.go#L2239

Which is called from here:

https://github.com/tinygo-org/tinygo/blob/ffeff5570692c019c734e3ca28db85bc48430f7f/src/machine/machine_atsamd51.go#L2160

Which is called in this interrupt:

https://github.com/tinygo-org/tinygo/blob/ffeff5570692c019c734e3ca28db85bc48430f7f/src/machine/machine_atsamd51.go#L1914

3. append on nRF52840

I haven't tracked this one down further, but it appears that there is a call to the built-in append function happening in an interrupt somewhere.

[deadprogram]

This commit should address most or all of the USBCDC heap allocations for ATSAMD21: https://github.com/tinygo-org/tinygo/commit/8397109c44bb118b5837894620e0c1ab70a7da50

@aykevl what do you think?

[aykevl]

Oops, I forgot about your commit! Sorry about that. These issues are now fixed in the dev branch.

I think this issue is now fixed, but I'm not 100% certain. EDIT: they're not, there is the bug in the bluetooth package that needs fixing.

[deadprogram]

Heh, I also forgot about it.

[deadprogram]

there is the bug in the bluetooth package that needs fixing.

I am pretty sure that there is an easy way to use the new -print-allocs flag to find these. Can you please give me an example of how to use it?

[aykevl]

I already know where it is, here: https://github.com/tinygo-org/bluetooth/blob/release/adapter_nrf528xx.go#L109-L110

Unfortunately it appears that the -print-allocs flag can't see where the heap allocation happens exactly. Probably some debug information is missing, possibly even in the x/tools/go/ssa package.

[aykevl]

Oh and this is how you can use it:

$ tinygo build -o test.hex -print-allocs=bluetooth  -target=pca10040-s132v6 ./examples/advertisement
/home/ayke/src/tinygo.org/x/bluetooth/adapter_nrf528xx-full.go: object allocated on the heap: escapes at unknown line

(Normally it would show the lines where it is allocated and where it escapes).

See tinygo help:

[...]
  -print-allocs string
        regular expression of functions for which heap allocations should be printed

(If there is any way to clarify this message, please say so.)

[aykevl]

I've fixed the missing line number issue in this PR: https://github.com/tinygo-org/tinygo/pull/1838 Now it gives this much more helpful message:

$ tinygo build -o test.hex -print-allocs=bluetooth  -target=pca10040-s132v6 ./examples/advertisement
/home/ayke/src/tinygo.org/x/bluetooth/adapter_nrf528xx-full.go:79:38: object allocated on the heap: escapes at line 79

[deadprogram]

So -print-allocs=bluetooth means it will match any function in the bluetooth package? e.g. bluetooth.handleEvent in this case.

[aykevl]

Yes, it will match any function with bluetooth in the name (including in the function name). So for example bluetooth.handle will match any function with that regular expression (including bluetooth.handleEvent).

[deadprogram]

The upcoming release prevents these, so tagging to be closed on next release.

[deadprogram]

This is part of the v0.28 release so now closing this issue. Thanks!