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.

Sign up for GitHub

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

Jump to bottom

apps/platform, os/binfmt: add exceptions support in loadable #6068

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

apps/platform, os/binfmt: add exceptions support in loadable #6068

wants to merge 1 commit into from

Conversation

abhishek-samsung
Copy link
Contributor

Add support for cpp exceptions in loadable for RTL8730E and RTL8721CSM.

@abhishek-samsung abhishek-samsung marked this pull request as ready for review February 19, 2024 13:29
@abhishek-samsung
Copy link
Contributor Author

test_cpp_exceptions.patch

  • The following PR doesn't work with the current docker version. We need newer toolchain for this PR to work (--force-group-allocations flag was added in the newer toolchain versions).
  • cout is not working in loadable. Hence, the test patch only uses printf.

output result :

binary_manager_load_binary: Load success! [Name: common] [Version: 200204] [Partition: A] [Text start : 0x60372000] [Compressed Binary]
binary_manager_load: app1 Header Checking Success
......
binary_manager_load_binary: Load success! [Name: app1] [Version: 190412] [Partition: A] [Text start : 0x60368000] [Compressed Binary]
.......
This is WIFI App
============ Test STL(Array) ============================ <-- test exceptions in apps
......
============ Test RTTI ==================================
extended
============ Test Exception =============================
 return address 603ad32b
 return address 603ac339
 return address 603ad32b
 return address 603ac339
Catch exception runtime error
============ thread::operator= test =====================
TASH>>binary_manager_load_binary: Load success! [Name: app2] [Version: 190412] [Partition: A] [Text start : 0x603a4000] [Compressed Binary]
pause of 1  seconds ended
pause of 2  seconds ended
pause of 3  seconds ended
pause of 4  seconds ended
pause of 5  seconds ended
All threads joined!
============ thread::joinable test ======================
============ thread::join test ==========================
Spawning 3 threads...
Done spawning threads. Now waiting for them to join:
pause of 1  seconds ended
pause of 2  seconds ended
pause of 3  seconds ended
All threads joined!

TASH>>
TASH>>
TASH>>
TASH>>cxxtest
TASH>>============ Test STL(Array) ============================ <-- test exceptions in common binary
......
============ Test RTTI ==================================
extended
============ Test Exception =============================
 return address 60389707
 return address 60388181
 return address 60389707
 return address 60388181
Catch exception runtime error
============ thread::operator= test =====================
pause of 1  seconds ended
pause of 2  seconds ended
pause of 3  seconds ended
pause of 4  seconds ended
pause of 5  seconds ended
All threads joined!
============ thread::joinable test ======================
============ thread::join test ==========================
Spawning 3 threads...
Done spawning threads. Now waiting for them to join:
pause of 1  seconds ended
pause of 2  seconds ended
pause of 3  seconds ended
All threads joined!

@abhishek-samsung abhishek-samsung changed the title apps/platform, os/binfmt: add exceptions support in loadable [DO NOT MERGE] apps/platform, os/binfmt: add exceptions support in loadable Feb 19, 2024
r-prabu
r-prabu reviewed Feb 19, 2024
View reviewed changes
lib/libc/machine/arm/armv7-m/arch_elf.c
@@ -193,7 +193,7 @@ int up_relocate(FAR const Elf32_Rel *rel, FAR const Elf32_Sym *sym, uintptr_t ad
}
break;

#ifdef CONFIG_ARMV7M_TARGET2_PREL
#if defined(CONFIG_ARMV7M_TARGET2_PREL) || defined(CONFIG_ARMV8M_TARGET2_PREL)
Copy link

Choose a reason for hiding this comment

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

Is arch_elf.c module is common for armv7-m & armv8-m?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this file is same for both.

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean the "same"? This file is in the armv7-m. Do you mean that armv8-m chip uses armv7-m's file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TizenRT/lib/libc/machine/arm/Make.defs

Lines 62 to 65 in 72bf01c

include ${TOPDIR}/../lib/libc/machine/arm/armv7-m/Make.defs
else ifeq ($(CONFIG_ARCH_ARMV8M_FAMILY),y) # Cortex-M33/M55 is ARMv8-M
include ${TOPDIR}/../lib/libc/machine/arm/armv7-m/Make.defs
else ifeq ($(CONFIG_ARCH_ARMV7A_FAMILY),y) # ARMV7A

Yes, both use the same file.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks strange. No need to move this into the common? Or make a common for m core if r core is different?

Copy link
Contributor

Choose a reason for hiding this comment

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

Need to check this

@abhishek-samsung abhishek-samsung changed the title [DO NOT MERGE] apps/platform, os/binfmt: add exceptions support in loadable apps/platform, os/binfmt: add exceptions support in loadable Apr 15, 2024
sunghan-chang
sunghan-chang reviewed Apr 16, 2024
View reviewed changes
apps/platform/gnu/gnu_unwind_find_exidx.c Outdated Show resolved Hide resolved
build/configs/rtl8730e/cpp_loadable/defconfig Outdated Show resolved Hide resolved
os/dbuild.sh Outdated Show resolved Hide resolved
@abhishek-samsung
Copy link
Contributor Author

I will comment here once I finish the testing the loadable_apps config changes, please dont merge the PR till then.

@abhishek-samsung
apps/platform, os/binfmt: add exceptions support in loadable
4d138a0 
Add support for cpp exceptions in loadable for RTL8730E and
RTL8721CSM.

Signed-off-by: Abhishek Akkabathula <a.akkabathul@samsung.com>
@abhishek-samsung
Copy link
Contributor Author

I have verified it by adding simple exception code in helloxx,

diff --git a/apps/examples/helloxx/helloxx_main.cxx b/apps/examples/helloxx/helloxx_main.cxx
index a4d711883..72c1f9fc8 100644
--- a/apps/examples/helloxx/helloxx_main.cxx
+++ b/apps/examples/helloxx/helloxx_main.cxx
@@ -173,6 +173,17 @@ extern "C"
                g_HelloWorld.HelloWorld();
 #endif
 
+#if defined(CONFIG_LIBCXX_EXCEPTION)
+               try
+               {
+                       throw 5;
+               }
+               catch (int &e)
+               {
+                       printf("exception caught : %d\n", e);
+               }
+#endif
+
                delete pHelloWorld;
                return 0;
        }

result :

TASH>>c++ version used : 201103
helloxx_main: Saying hello from the dynamically constructed instance
CHelloWorld::HelloWorld: Hello, World!!
helloxx_main: Saying hello from the instance constructed on the stack
CHelloWorld::HelloWorld: Hello, World!!
helloxx_main: Saying hello from the statically constructed instance
CHelloWorld::HelloWorld: Hello, World!!
exception caught : 5

@abhishek-samsung abhishek-samsung mentioned this pull request Apr 26, 2024
Merged
sunghan-chang
sunghan-chang reviewed Apr 30, 2024
View reviewed changes
build/configs/rtl8730e/loadable_apps/defconfig
@@ -1082,7 +1086,7 @@ CONFIG_PM_DEVNAME_LEN=32
CONFIG_PM_SLICEMS=100
CONFIG_PM_NDOMAINS=1
CONFIG_PM_MEMORY=2
# CONFIG_PM_DVFS is not set
CONFIG_PM_DVFS=y
Copy link
Contributor

Choose a reason for hiding this comment

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

@ritesh55555 Is it ok to enable this now?

Copy link
Contributor

Choose a reason for hiding this comment

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

It will only enable an api for now. No other effect on working of the system.

lib/libc/machine/arm/armv7-m/arch_elf.c
@@ -193,7 +193,7 @@ int up_relocate(FAR const Elf32_Rel *rel, FAR const Elf32_Sym *sym, uintptr_t ad
}
break;

#ifdef CONFIG_ARMV7M_TARGET2_PREL
#if defined(CONFIG_ARMV7M_TARGET2_PREL) || defined(CONFIG_ARMV8M_TARGET2_PREL)
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to check this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sunghan-chang sunghan-chang sunghan-chang left review comments

@kishore-sn kishore-sn kishore-sn left review comments

@ritesh55555 ritesh55555 ritesh55555 left review comments

@r-prabu r-prabu r-prabu approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@abhishek-samsung @sunghan-chang @kishore-sn @ritesh55555 @r-prabu