LoadStoreError on attempt to modify return value from Storage.readArrayBuffer()

[GaryOtt]

This will cause a reboot:

var s = require("Storage");
s.write("ch0", E.toUint8Array([0]));
s.readArrayBuffer("ch0")[0] = 1;

Found to occur on ESP32 and ESP8266. I will test more boards when I can. Present in the last release as well as a build compiled this afternoon.

In my application, I actually need a Uint8Array and I find this work around returns a writeable object:
new Uint8Array(new Uint8Array(s.readArrayBuffer("ch0")))

[gfwilliams]

Hmm. I think this is likely a classic ESPxx issue to do with unaligned reads from flash? I thought the compiler was supposed to handle this now though :(

new Uint8Array(new Uint8Array(s.readArrayBuffer("ch0")))

I think that will create a new Uint8Array in RAM rather than trying to read from flash directly.

[GaryOtt]

likely a classic ESPxx issue to do with unaligned reads from flash?

It looks like you're exactly describing a LoadStoreAlignment error but the result I'm getting LoadStoreError instead.

I have an idea what maybe going on but I'm still very much learning my way around the Espruino code. If I am right, the recent introduction ofStorage.readArrayBuffer() caused the potential to use old code in a new way.

jswrap_storage_readArrayBuffer() calls jsfReadFile() which returns the result of jsvNewFlashString(), jsvNewNativeString() or jsvNewNativeString(). At the risk of stating the obvious, these three functions return JSVars containing strings and not a char arrays or a JSVar containing an array buffer.

A little talked about JavaScript feature is that strings are immutable. All JavaScript string functions return new strings rather than carry out in place modification of the original. We can assign new values to string variables but cannot modify existing string values. Unlike JavaScript strings, array buffers are mutable.

It seems that as a result, the above code causes an attempt to write to locations that aren't (and should not be) intended for writing to. If it is possible to modify the contents of Storage without calling Storage.write(), then that is also a bug.

I haven't tried it on an official board yet because my old WiFi 1.1 is refusing to accept firmware updates. I'm trying to persuade a colleague to try it on a pixel.js and I'm hoping my boss agrees that buying me a Bangle is a justifiable business expense :)

---

I think that will create a new Uint8Array in RAM rather than trying to read from flash directly.

It's not a work around that will suit everyone but my application now works reliably.

[gfwilliams]

Ahh - sorry, I didn't read the code properly.

On ARM platforms a write to flash will just be ignored, so the s.readArrayBuffer("ch0")[0] = 1; has no effect.

In Espruino while Strings aren't writable externally, internally they are - just not jsvNewFlashString (or jsvNewNativeString if it points to flash memory);

I guess the solution is going to be detecting a write to an ArrayBuffer backed with a string that's in flash memory and then doing a copy-on-write to RAM. It's a bit hacky though - I wonder whether for now a warning about the read-only nature of the array might be an idea.

Or... while const isn't implemented in Espruino, if it were it might be more sensible to constify the array

[GaryOtt]

I guess the solution is going to be detecting a write to an ArrayBuffer backed with a string that's in flash memory and then doing a copy-on-write to RAM

I actually thought that was a reasonable solution. The really hacky solution would be to always load into RAM without knowing if the user wants an output they can change or not.

Or... while const isn't implemented in Espruino, if it were it might be more sensible to constify the array

That would be a non-standard implementation of const. When declaring any form of object constant, it is the pointer to the object that is constant. Any values contained by the object can still be mutated.

Garys-MacBook-Pro-2:~ gary$ node
Welcome to Node.js v12.18.1.
Type ".help" for more information.
> const bob = new ArrayBuffer(1);
undefined
> bob[0] = 1
1
> bob[0]
1
> bob[0] = 2
2
> bob[0]
2
> bob = new ArrayBuffer(1);
Uncaught TypeError: Assignment to constant variable.
> 

I think the only way to give the user the choice of a read-only or writeable output would be to add an optional boolean parameter to Storage.readArrayBuffer().

[GaryOtt]

Also, can we agree the wording "constant variable" does not deserve duplication in Espruino?!

[gfwilliams]

Yep - it's more that adding const likely means adding a flag to the JsVar structure in Espruino, which would allow me to flag things as const other than just variable names.

The only issue with the copy on write is that jsvNewNativeString can reference RAM too - so we'd have to detect if the memory address pointed to was in fact flash memory or not.