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

Heap addendum to handle changes in NON-OS SDK 3.0.x #8746

Merged
merged 10 commits into from
Dec 16, 2022

Conversation

mhightower83
Copy link
Contributor

@mhightower83 mhightower83 commented Dec 7, 2022

Changes to better support new SDKs v3.0.0-v3.0.5

WPA2 Enterprise connections

References - merged PRs:

The NON-OS SDK 3.0.x has breaking changes to the pvPortMalloc function. They added a new bool argument for selecting a heap.

void *pvPortMalloc (size_t sz, const char *, unsigned, bool);

To avoid breaking the build, I added a new thin wrapper function sdk3_pvPortMalloc to heap.cpp.
Edited new SDK LIBs to call pvPortMalloc's replacement sdk3_pvPortMalloc.

They also added pvPortZallocIram and pvPortCallocIram, which are not a problem to support. Support added to heap.cpp.

Issues with WPA2 Enterprise in new SDKs:

  • v3.0.0 and v3.0.1 - have the same memory leak and duplicate free bugs from before
  • v3.0.2 through v3.0.5 - have the same memory leak; however, no duplicate free crash.
  • memory leak can be seen by cycling through setup, connect, disconnect, and clear setup - repeatedly.

Updated wpa2_eap_patch.cpp and binary patch scripts to handle v3.0.0 through v3.0.5.
Patched SDKs v3.0.0 through v3.0.5

Duplicate Non-32-bit exception handler

Issue: At v3.0.0 and above libmain.a supplies a built-in exception handler (load_non_32_wide_handler) for non-32-bit access. Our non-32-bit access handler (non32xfer_exception_handler) overrides it.

Solution: Add "weak" attribute to symbol load_non_32_wide_handler. Adjust the build to default to the SDK's built-in non-32-bit handler. If there is a need to use our non-32-bit handler, make the selection from the Arduino IDE Tools menu Non-32-Bit Access: "Byte/Word access to IRAM/PROGMEM (very slow)".

With SDKs v3.0.0 and above a "non-32-bit exception handler" is always present.

Edited to describe the current contents of the PR.

The NON-OS SDK 3.0.x has breaking changes to pvPortMalloc. They added one more
argument for selecting a heap. To avoid breaking the build, I renamed their
broken version pvEsprMalloc. To be used, the LIBS need to be edited.

They also added pvPortZallocIram and pvPortCallocIram, which are not a
problem.

Issues with WPA2 Enterprise in new SDKs:
* v3.0.0 and v3.0.1 - have the same memory leak and duplicate free bugs from before
* v3.0.2 through v3.0.5 - have the same memory leak; however, _no_ duplicate free crash.
* memory leak can be seen by cycling through setup, connect, disconnect, and clear setup - repeatedly.

Updated `wpa2_eap_patch.cpp` and binary patch scripts to handle v3.0.0 through v3.0.5.
Patched SDKs v3.0.0 through v3.0.5
@mhightower83 mhightower83 marked this pull request as ready for review December 9, 2022 17:17
non32xfer_exception_handler in v3.0.x
Reduce resource requirements for non32xfer_exception_handler on SDKs 3.0.x
Updated script to replaced pvPortMalloc with sdk3_pvPortMalloc
in SDK v3.0 libraries

if [[ ${VERSION} == "NONOSDK3V0" ]]; then
xtensa-lx106-elf-objcopy --weaken-symbol load_non_32_wide_handler libmain.a
elif [[ ${VERSION:0:9} == "NONOSDK30" ]]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: for this and the above, bash has a pretty good string matcher through case

case $VERSION in
"NONOSDK3V0") echo "foo" ;;
"NONOSDK30"*) echo "bar" ;;
"I write this in Github Web, syntax may be wrong") echo "baz" ;;
esac

cores/esp8266/heap.cpp Outdated Show resolved Hide resolved
cores/esp8266/heap.cpp Outdated Show resolved Hide resolved
@mcspr mcspr merged commit 4a0b66b into esp8266:master Dec 16, 2022
@mhightower83 mhightower83 deleted the pr-enterprise-305 branch December 22, 2022 18:04
hasenradball pushed a commit to hasenradball/Arduino that referenced this pull request Nov 18, 2024
## WPA2 Enterprise connections
References - merged PRs:
* esp8266#8529
* esp8266#8566 - these occurred with connect/disconnect with WPA-Enterprise
* esp8266#8736 (comment)

The NON-OS SDK 3.0.x has breaking changes to the [`pvPortMalloc`](https://github.com/espressif/ESP8266_NONOS_SDK/blob/bf890b22e57a41d5cda00f9c8191f3f7035a87b4/include/mem.h#L42) function. They added a new `bool` argument for selecting a heap. 
```cpp
void *pvPortMalloc (size_t sz, const char *, unsigned, bool);
```

To avoid breaking the build, I added a new thin wrapper function `sdk3_pvPortMalloc` to `heap.cpp`. 
Edited new SDK LIBs to call `pvPortMalloc`'s replacement `sdk3_pvPortMalloc`.

They also added `pvPortZallocIram` and `pvPortCallocIram`, which are not a problem to support. Support added to `heap.cpp`.

Issues with WPA2 Enterprise in new SDKs:
* v3.0.0 and v3.0.1 - have the same memory leak and duplicate free bugs from before
* v3.0.2 through v3.0.5 - have the same memory leak; however, _no_ duplicate free crash.
* memory leak can be seen by cycling through setup, connect, disconnect, and clear setup - repeatedly.

Updated `wpa2_eap_patch.cpp` and binary patch scripts to handle v3.0.0 through v3.0.5.
Patched SDKs v3.0.0 through v3.0.5

## Duplicate Non-32-bit exception handler
Issue: At v3.0.0 and above `libmain.a` supplies a built-in exception handler (`load_non_32_wide_handler`) for non-32-bit access. Our non-32-bit access handler (`non32xfer_exception_handler`) overrides it. 

Solution: Add "weak" attribute to symbol `load_non_32_wide_handler`. Adjust the build to default to the SDK's built-in non-32-bit handler.  If there is a need to use our non-32-bit handler, make the selection from the Arduino IDE Tools menu `Non-32-Bit Access: "Byte/Word access to IRAM/PROGMEM (very slow)"`.

With SDKs v3.0.0 and above a "non-32-bit exception handler" is always present.
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.

2 participants