Skip to content

Commit 8922227

Browse files
author
Alrik Vidstrom
committed
Fix MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL redefine bug
The redefines break paragraph 3.2.4 of the C++14 specification (a subcategory of the ODR - One Definition Rule) in an insidious way. There's no diagnostic required in these cases, which means no warnings, no errors required by the compiler. It's also undefined behavior. There's a conditional compilation of the function call_fn() in the Callback implementation, and in practice, it's inlined in the call() function in the same object file. Depending on the MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL setting you get an extra "ldr r3,[r3, #0]" instruction or not inlined in call(). Without that indirection, calling a nontrivial callback leads to the execution of "bx r3", where r3 contains the address of the ops structure (dynamically dispatched operations), instead of the address contained in the first member of that structure, which would lead to the type-erased function pointer. And because the structure is at an even address, there's a UsageFault when the CPU tries to enter Arm mode, which isn't supported on Cortex-M. When there's a single definition of MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL everything is fine, because there's only one definition of call_fn() and call() - the last function of the two is the only one that exists in practice, in the binary. But when we redefine MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL we get two definitions of the call() function at the same time, breaking paragraph 3.2.4. And remember, no diagnostic required, and this is undefined behavior. So anything goes. On macOS, USBDevice::endpoint_add() is then linked against call() from USBHost.o, which contains the extra "ldr r3,[r3, #0]" instruction. On Windows, it's instead linked against the call() function from USBHostSerial.o, which contains no indirection, and it leads to a crash. These two translation units are compiled with different settings because the redefines are applied to some parts of the library but not all of them. Remember, breaking 3.2.4 is undefined behavior, so it's ok to link this at random. But keeping them equal wouldn't be ok anyway because 3.2.4 would still be broken by the library in combination with the core.
1 parent 93aab88 commit 8922227

File tree

3 files changed

+0
-6
lines changed

3 files changed

+0
-6
lines changed

src/USBHost/USBDeviceConnected.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@
2121
#include "USBHost/USBEndpoint.h"
2222
#include "USBHost/USBHostConf.h"
2323
#include "rtos.h"
24-
#undef MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL
25-
#define MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL 0
2624
#include "Callback.h"
2725

2826
class USBHostHub;

src/USBHost/USBEndpoint.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
#ifndef USBENDPOINT_H
1818
#define USBENDPOINT_H
1919

20-
#undef MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL
21-
#define MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL 0
2220
#include "Callback.h"
2321
#include "USBHostTypes.h"
2422
#include "rtos.h"

src/USBHost/USBHostConf.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
#ifndef USBHOST_CONF_H
1818
#define USBHOST_CONF_H
1919

20-
#undef MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL
21-
#define MBED_CONF_PLATFORM_CALLBACK_NONTRIVIAL 0
2220
#include "Callback.h"
2321
#include "Arduino.h"
2422

0 commit comments

Comments
 (0)