Skip to content

Conversation

@multiplemonomials
Copy link
Collaborator

Summary of changes

Did some checking into finally fixing this annoying test failure that has been happening for years. The hal-mpu stack fault test has been dying on most MCUs 100% of the time, and it wasn't super obvious why.

When debugging the test, I noticed that it was dying because it was executing a garbage instruction (NOT because it was executing from RAM -- that's an intended part of the test). Why was this happening? Well, stepping through the disassembly of mpu_fault_test_stack(), it looked like the compiler had reordered the instructions, and was executing line 141 before line 147! So it was executing the RAM instruction before said instruction had been initialized with a value.

Why was that happening? Well, I think that the cast that this test does breaks aliasing rules in C++ -- it's casting an integer into a function pointer, and the optimizer is allowed by the standard to not have to consider the case where a function pointer points to a regular variable. So it does not make the connection that the argument to mpu_fault_test() is actually used by said function.

Luckily, fixing this is pretty straightforward. We just have to mark the variable as volatile (and apply the same fix to mpu_fault_test_heap()). That way the compiler isn't allowed to reorder the access in this manner.

Impact of changes

Migration actions required

Documentation

None


Pull request type

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[X] Tests / results supplied as part of this PR
{{__sync;677f7397-a210-43cf-8efa-d23bf653564a}}
{{__version;1.3.0}}
{{__timeout;20}}
{{__host_test_name;default_auto}}
{{__testcase_count;6}}
>>> Running 6 test cases...
{{__testcase_name;MPU - init}}
{{__testcase_name;MPU - free}}
{{__testcase_name;MPU - data fault}}
{{__testcase_name;MPU - bss fault}}
{{__testcase_name;MPU - stack fault}}
{{__testcase_name;MPU - heap fault}}

>>> Running case #1: 'MPU - init'...
{{__testcase_start;MPU - init}}
{{__testcase_finish;MPU - init;1;0}}
>>> 'MPU - init': 1 passed, 0 failed
<greentea test suite>:0::PASS

>>> Running case #2: 'MPU - free'...
{{__testcase_start;MPU - free}}
{{__testcase_finish;MPU - free;1;0}}
>>> 'MPU - free': 1 passed, 0 failed
<greentea test suite>:0::PASS

>>> Running case #3: 'MPU - data fault'...
{{__testcase_start;MPU - data fault}}
{{__testcase_finish;MPU - data fault;1;0}}
>>> 'MPU - data fault': 1 passed, 0 failed
<greentea test suite>:0::PASS

@multiplemonomials multiplemonomials merged commit 32098dc into master Jan 12, 2025
52 checks passed
@multiplemonomials multiplemonomials deleted the bugfix/fix-hal-mpu-test branch January 12, 2025 19:45
JojoS62 pushed a commit that referenced this pull request Jan 13, 2025
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