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

Add executable analaysis tool for floating point checks. #11314

Merged
merged 1 commit into from
Sep 4, 2019
Merged

Add executable analaysis tool for floating point checks. #11314

merged 1 commit into from
Sep 4, 2019

Conversation

hugueskamba
Copy link
Collaborator

Description

Add a Python3 script for checking the presence of floating point symbols in an ELF format file.

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change
[X] Additional tool

Reviewers

@madchutney @evedon @donatieng

Release Notes

@ciarmcom ciarmcom requested review from donatieng, evedon, madchutney and a team August 23, 2019 15:00
@ciarmcom
Copy link
Member

@hugueskamba, thank you for your changes.
@donatieng @madchutney @evedon @ARMmbed/mbed-os-tools @ARMmbed/mbed-os-maintainers please review.

@ciarmcom ciarmcom requested a review from a team August 23, 2019 15:00
@0xc0170
Copy link
Contributor

0xc0170 commented Aug 26, 2019

What's the use for this script? I can see it only prints if anything found? Is this for testing purpose, CI related?

@hugueskamba
Copy link
Collaborator Author

What's the use for this script? I can see it only prints if anything found? Is this for testing purpose, CI related?

The script is part of the requirement to not have floating point instructions when building example applications. It allows its user to check if floating point function symbols have been included in the executable generated. The functionality is to be eventually included in the build tool but was agreed to be made available as a standalone script for the time being.

0xc0170
0xc0170 reviewed Aug 27, 2019
View reviewed changes
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

what about testing this script? any unit test at least?

@hugueskamba
Copy link
Collaborator Author

hugueskamba commented Aug 28, 2019 •
edited
Loading

what about testing this script? any unit test at least?

@0xc0170
efa8050

@hugueskamba hugueskamba force-pushed the hk-iotcore-1279-executable_analysis_tools branch from 29badb8 to efa8050 Compare August 28, 2019 07:06
0xc0170
0xc0170 approved these changes Aug 28, 2019
View reviewed changes
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

@ARMmbed/mbed-os-tools Please review

@hugueskamba hugueskamba force-pushed the hk-iotcore-1279-executable_analysis_tools branch 3 times, most recently from 72361f9 to 9f976a7 Compare August 29, 2019 06:58
@0xc0170
Copy link
Contributor

0xc0170 commented Aug 29, 2019

@hugueskamba Is this targeting 5.14 (I believe it can come in the patch release). As we are close to 5.14 code freeze. If it is in 5.14, reviewers need a poke to get this approved and in CI soon.

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 29, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Aug 29, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

paul-szczepanek-arm
paul-szczepanek-arm suggested changes Aug 29, 2019
View reviewed changes
tools/executable_analysis_tools/elf-float-checker.py Outdated
)
# Command syntax:
# `arm-none-eabi-readelf --symbols <elf_file>`
cmd = ["arm-none-eabi-readelf", "--symbols", elf_file]
Copy link
Member

Choose a reason for hiding this comment

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

use objdump -t instead for portability

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can you give a bit more information what you think objdump is more portable than readelf?

Copy link
Member

Choose a reason for hiding this comment

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

CI doesn't have the latter

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

d317feb

tools/executable_analysis_tools/elf-float-checker.py Show resolved Hide resolved
@hugueskamba hugueskamba force-pushed the hk-iotcore-1279-executable_analysis_tools branch from d317feb to 4f1a7c2 Compare August 30, 2019 10:21
@hugueskamba
Copy link
Collaborator Author

This force-push squashed all commits.

@hugueskamba hugueskamba force-pushed the hk-iotcore-1279-executable_analysis_tools branch from 4f1a7c2 to 487bbe6 Compare August 30, 2019 10:27
@hugueskamba
Copy link
Collaborator Author

This force-push rebased from master.

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 30, 2019

CI restarted

@mbed-ci
Copy link

mbed-ci commented Aug 30, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 2
Build artifacts

@0xc0170 0xc0170 merged commit a924c02 into ARMmbed:master Sep 4, 2019
@hugueskamba hugueskamba deleted the hk-iotcore-1279-executable_analysis_tools branch September 10, 2019 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@madchutney madchutney madchutney approved these changes

@paul-szczepanek-arm paul-szczepanek-arm paul-szczepanek-arm approved these changes

@donatieng donatieng Awaiting requested review from donatieng

@evedon evedon Awaiting requested review from evedon

Assignees
No one assigned
Labels
release-version: 5.14.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@hugueskamba @ciarmcom @0xc0170 @mbed-ci @madchutney @paul-szczepanek-arm