Teensyduino 1.54 Beta #8

Status
Not open for further replies.
ryanrs said:
So what's the process to get bug fixes into the teensy core library? I've spend a fair amount of time debugging a couple problems I've run into, but it kinda feels like I'm posting into the wind over in the tech support forum.
Click to expand...

... from observation ...

If the changes are against a current Beta - then posting on the latest Beta thread can get them 'logged' perhaps. Post 'current beta thread' details and links to the relevant thread and issue if the 'work' is shown there.

If you have a fix for a known issue - making a Github Pull Request against the source involved can allow Paul to easily observe and address the issue. And then again post on the 'beta thread' to help make it visible.

If the problem is one demonstrated by an example that can be posted - but unknown resolution - but involves Beta code - the same generally applies.


All code changes go through Paul so the value and safety of the change needs to be apparent. Of course All of Everything goes through Paul - and unfortunately these are not 'normal times with normal process'. Production is shorthanded and there are abnormal supply constraints requiring attention to make up for component shortages affecting production of more and more items using electronics. The current TeensyDuino beta has incorporated quite a few new and useful updates and has lagged due to the 'abnormal times' and outside tasks.

And then Arduino team drops a new IDE forcing a quick same day Beta 8 drop to support it ...
 
There are two bugs discussed in my support thread.

One is a memcpy bug from 2014 or earlier, that Frank B mentioned had been fixed, then un-fixed in the intervening years. Basically memcpy is trying to be clever by doing unaligned 32 bit reads and writes, but that causes corruption when copying across the 0x20000000 boundary. The thing is, unaligned access usually works on teensy 3.2, except when crossing the 0x20000000 boundary. This bug was mentioned in passing by another user, so I posted a test case that verifies the bug. I did not write a fix.

The other bug is a race condition in teensy3 usb_dev.c because the usb buffer descriptors are not declared volatile, and need to be. This bug has probably existed since the teensy3 usb code was written. There was a note about it and a partial workaround posted in 2018, but I'm pretty sure this bug has been corrupting teensy3 usb since teensy3 was released. I posted a simple fix. (n.b. it's not a compiler bug).

Basically I just want these fixes to get into the library and "stick" so I don't have to keep my own set of patches.
 
Well, here we go again... Arduino released 1.8.15 yesterday.

I've compiled Teensyduino's patches. Going to fix a few small issues and then upload 1.54-beta9 this afternoon...
 
Again sorry hard to know what to do here...

As mentioned @Paul is probably the best one to answer some of these and your thread has had comments in it by several of the members...
And there are several of us up here who try to help out when we can. But some of these like issue are often hard to know what is right..

ryanrs said:
There are two bugs discussed in my support thread.

One is a memcpy bug from 2014 or earlier, that Frank B mentioned had been fixed, then un-fixed in the intervening years. Basically memcpy is trying to be clever by doing unaligned 32 bit reads and writes, but that causes corruption when copying across the 0x20000000 boundary. The thing is, unaligned access usually works on teensy 3.2, except when crossing the 0x20000000 boundary. This bug was mentioned in passing by another user, so I posted a test case that verifies the bug. I did not write a fix.

The other bug is a race condition in teensy3 usb_dev.c because the usb buffer descriptors are not declared volatile, and need to be. This bug has probably existed since the teensy3 usb code was written. There was a note about it and a partial workaround posted in 2018, but I'm pretty sure this bug has been corrupting teensy3 usb since teensy3 was released. I posted a simple fix. (n.b. it's not a compiler bug).

Basically I just want these fixes to get into the library and "stick" so I don't have to keep my own set of patches.
Click to expand...

Again if you have clear issues with fixes that don't cause major side effects than the best way to get things to stick is to try to create a Pull Request with the actual fix and pointers to thread(s) that describe the issues you are solving and what tests you have run to show that it works...

I have not commented on either of these issues as unclear to me, what the right answers are:

That is memcpy issue: I believe has the: cores\teensy3\memcpy-arm7m.S file? all relating to if the memcpy support unaligned copy or not?
https://github.com/PaulStoffregen/cores/blob/master/teensy3/memcpy-armv7m.S#L37

In particular if the options: #define __ARM_FEATURE_UNALIGNED 1
Should be set or not set? The particular question is what breaks if I don't enable unaligned memcpy?
From looking at the Code changes on this file, it is unclear if there is a clear right answer as this has changed several times.
The current stuff is setup to allow the build to choose it or not?
Code: 
/*
 * Let __ARM_FEATURE_UNALIGNED be set by the architecture and the compiler flags:
 *   -munaligned-access
 *   -mno-unaligned-access
 * instead of always setting it here.
 *
#define __ARM_FEATURE_UNALIGNED 1
 */

The USB issue...

Maybe volatile is valid fix? Actually never used that on a structure, but instead always used on members in structures.
I guess the question would be, is this a sledgehammer fix? (sort of like the current one) and/or is there specific places that used this structure or pointer to structure that needed to be volatile?

Also I had not tried it out as https://forum.pjrc.com/threads/67081-Haunted-USB-Serial-on-Teensy-3-2?p=278849&viewfull=1#post278849
your next post which you deleted, had comment for the delete: whoops, my bug, not lib
So thought your problem had been resolved.

Again probably something for Paul to take a look at.
 
For memcpy, I have not dug through the ARM assembly. But if it's like other memcpy implementations, there is a code path for when the src and dst are not relatively aligned (src % 4 != dst % 4). For the relatively unaligned case, memcpy needs to do one of 3 things:
a) do 32-bit unaligned reads and aligned writes (or vice-versa)
b) fall back to byte-by-byte copy
c) do some shifts and ORs on the registers so you can do 32-bit aligned reads AND writes

Right now I think memcpy is doing (a), when it needs to do (b) or (c) because (a) does not work at 0x20000000.

Which method memcpy uses internally should have no externally visible effects except speed (and correctness in case of (a), heh).

Turning off __ARM_FEATURE_UNALIGNED for the Teensy 3.2 (and other chips with split SRAM banks) is probably the right answer, but again, I haven't dived into the assembly.



For the USB bug, adding 'volatile' is not a sledgehammer fix. It is a minimal, targeted change to ensure store ordering between the two structure fields, which is important because the USB hardware peeks at the memory when the compiler isn't looking.

If you want, you can add the volatile label to both structure fields instead of the typedef, which is equivalent in this case.


Re. the deleted post, the deletion reason is in reference to the deleted post, not the rest of the thread.
 
I saw the thread about volatile on the buffer descriptor table. Put it on my list of issues to investigate. Haven't had time to do much on that list lately... but it's definitely on the list so I won't forget.

On the memcpy issue, can you point me to (hopefully simple) test code to reproduce the problem?
 
I Ithink I wrote some ( a lot of ) posts about the memory boundary on T3, including the memcpy issues. It hit us several times. Maybe with some digging you can find them - including testcode.
The fix was simple and I don't know why it was switched back to unaligned mode.

With working hardfault-code they would have been clearly visible. As of now, T3 dies silently and may worry many users.

There are several other open pullrequests, mainly for T4 - but audio, too - thought they would be merged? Any complains about them?

As a little reminder: The waveplayer does not work on LC due to wrong audioblocksize / Bug in its code.
 
Last edited:
luni said:
it still breaks compatibility with the STL for newer compilers. E.g. this:
....
compiles and works perfectly with gcc 9 and TD1.53 but generates compile errors with TD1.54 (and gcc9).
Click to expand...

I can't reproduce this problem. On Arduino 1.8.14 with Teensyduino 1.54-beta8 it compiles for all boards except Teensy 2.0 and Teensy++ 2.0. Ran it on Teensy 4.0 and it prints "answer" to the serial monitor.

screenshot.png

Also works on Arduino 1.8.15 (yes, Arduino released again a couple days ago) with the work-in-progress 1.54-beta9.

I tested only on Linux x86-64. Do I need to try on Windows or MacOS?
 
PaulStoffregen said:
I can't reproduce this problem. On Arduino 1.8.14 with Teensyduino 1.54-beta8 it compiles for all boards except Teensy 2.0 and Teensy++ 2.0. Ran it on Teensy 4.0 and it prints "answer" to the serial monitor.

...
I tested only on Linux x86-64. Do I need to try on Windows or MacOS?
Click to expand...

I tried on T_4.1 w/ Windows IDE 1.8.14 and TD 1.54 b8:

>> No problem, no warnings: Printed all three mapped values
Code: 
first
second
answer

Goes non-responsive with a number not in the list ... probably a Fault but don't have an active set of FB_PrintHardFault
 
ryanrs said:
Here's test code for memcpy. It's simple and 100% reproducible.
Click to expand...

Yes, confirmed, I am able to reproduce the memcpy bug. Working on it now....



Frank B said:
I Ithink I wrote some ( a lot of ) posts about the memory boundary on T3, including the memcpy issues. It hit us several times. Maybe with some digging you can find them - including testcode.
The fix was simple and I don't know why it was switched back to unaligned mode.
Click to expand...

I also have no idea. I didn't change it. Committed a fix to never use unaligned access.

https://github.com/PaulStoffregen/cores/commit/a648248ae0adade4155c886a4e71d93dace89f91


But this alone is not enough to solve the problem. Looks like gcc is sometimes replacing "memcpy" with "__builtin_mempy", which also seems to do unaligned access. I'm trying different fixes now....
 
Looks like we also need either "-fno-builtin-memcpy" or "-mno-unaligned-access" in the compiler flags. The later allows explicitly calling __builtin_mempy() to work.

Code: 
    // straddles BOUNDARY, upper half is not copied
    //memcpy(&across, p - 2, 4);
    __builtin_memcpy(&across, p - 2, 4);
    serial_printf("across  0x%08x\n", across);

I'm going to put "-mno-unaligned-access" into beta9. Hopefully it won't cause any major performance issues?
 
PaulStoffregen said:
I can't reproduce this problem. On Arduino 1.8.14 with Teensyduino 1.54-beta8 it compiles for all boards except Teensy 2.0 and Teensy++ 2.0. Ran it on Teensy 4.0 and it prints "answer" to the serial monitor.
....
Also works on Arduino 1.8.15 (yes, Arduino released again a couple days ago) with the work-in-progress 1.54-beta9.

I tested only on Linux x86-64. Do I need to try on Windows or MacOS?
Click to expand...

Looks like my post was a bit unclear.
As mentioned here https://forum.pjrc.com/threads/66357-Teensyduino-1-54-Beta-7?p=274861&viewfull=1#post274861, I only observe this issue when using newer compilers (tested with GCC9). Apparently, it was introduced by this commit: https://github.com/PaulStoffregen/co...04356bda7e2a7e.

This is not a show stopper but it would be nice to be fixed. (I routinely compile my libraries with newer compilers to avoid later compatibility issues. Can't do that anymore due to this issue).

(I can only test under Windows but I doubt that this is OS related)
 
luni said:
Looks like my post was a bit unclear.
As mentioned here https://forum.pjrc.com/threads/66357-Teensyduino-1-54-Beta-7?p=274861&viewfull=1#post274861, I only observe this issue when using newer compilers (tested with GCC9). Apparently, it was introduced by this commit: https://github.com/PaulStoffregen/co...04356bda7e2a7e.

This is not a show stopper but it would be nice to be fixed. (I routinely compile my libraries with newer compilers to avoid later compatibility issues. Can't do that anymore due to this issue).

(I can only test under Windows but I doubt that this is OS related)
Click to expand...

Ok have to ask - how do you update the toolchain to GCC9. Did it once a long time ago but can't remember how I did it?
 
ryanrs said:
So what's the process to get bug fixes into the teensy core library? I've spend a fair amount of time debugging a couple problems I've run into, but it kinda feels like I'm posting into the wind over in the tech support forum.
Click to expand...

I've committed fixes for both, so they'll be in 1.54-beta9... hopefully very soon. And nice work on getting to the bottom of that old USB buffering bug. In hindsight the buffer descriptor table should have been declared volatile all along.

And yeah, sorry I've been so terribly unresponsive lately. We've been really overwhelmed for over a year, trying to keep the company running short-staffed due to covid19 social distancing requirements. It hasn't left much time to focus on software, or even keep up with the forum at times.
 
Frank B said:
As a little reminder: The waveplayer does not work on LC due to wrong audioblocksize / Bug in its code.
Click to expand...

I'm going to look at audio library stuff for beta10. If I dive into audio now, it'll cause too much delay for beta9. It's now been 4 days since Arduino released 1.8.15, I really need to cut off too much more and get beta9 wrapped up with support for Arduino 1.8.15.

Same with the hard fault handler - I have some concerns there and it's just too big to hold up beta9. Will look at it later this week...
 
luni said:
I can recheck later today. Just to make sure: You did test it with GCC9 right?
Click to expand...

@luni
Followed the instructions to install arm9 (installed the 9-2020-q2-update) and changed the following in the boards.txt file to
Code: 
#teensy41.build.toolchain=arm/bin/
teensy41.build.toolchain=arm9/bin/
using IDE 1.8.14beta8 and then reran the example but it seems to work for me:
Code: 
first
second
answer
only tested on a T4.1 though.

EDIT: These are the instructions that I followed:
There is a directory "arm" under Arduino/hardware/tools - copy the whole directory and name it i.e. "arm9" - in result, you should have two directories now, "arm" and "arm9"
  1. Download (the zip-file) a newer toolchain from https://developer.arm.com/tools-and...eveloper-tools/gnu-toolchain/gnu-rm/downloads
  2. Extract it to the new directory - choose overwrite to overwrite the old version (additional files from the original version will stay)
  3. Edit boards.txt. There is a line teensy40.build.toolchain=arm/bin/ - change that to teensy40.build.toolchain=arm9/bin/
  4. done :)
Click to expand...
 
Status
Not open for further replies.
Back
Top