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

OS_GetErrorName Seg Faults #1435

Closed
ahatstat opened this issue Dec 13, 2023 · 2 comments · Fixed by #1436
Closed

OS_GetErrorName Seg Faults #1435

ahatstat opened this issue Dec 13, 2023 · 2 comments · Fixed by #1436

Comments

@ahatstat
Copy link
Contributor

Describe the bug
The coverage test coverage-shared-errors-testrunner seg faults on a build for ARM/Linux. The seg fault happens in function OS_GetErrorName at this line.

*err_name[sizeof(*err_name) - 1] = 0;

Here err_name is a pointer to a char array. The order of operations for C indicates that the array subscripting [] will be evaluated before the pointer dereferecing *. This results in dereferencing a value outside of the char array defined in the coverage test for OS_GetErrorName.

I believe the syntax should be '(*err_name)[sizeof(*err_name) - 1] = 0; to ensure the pointer is dereferenced before the array subscripting. Making this change fixes the seg fault on my system.

To Reproduce
Run the coverage-shared-errors-testrunner coverage test.

Expected behavior
The test passes and does not seg fault.

System observed on:

  • ARM Cortex A9
  • Linux
  • Versions Draco-RC4
@ahatstat
Copy link
Contributor Author

./coverage-shared-errors-testrunner -d

[BEGIN] UNIT TEST

[BEGIN] 01 SETUP
[  END] No test cases

[BEGIN] 01 OS_GetErrorName
[DEBUG] utstubs.c:925:OCS_strncpy called (DEFAULT,0)
Segmentation fault (core dumped)

@ahatstat
Copy link
Contributor Author

Here is the Godbolt compiler explorer showing the Assembly for this line as is. Note the 1190 offset which is well past the end of the char array. It thinks each array element size is 35 bytes. 34 * 35 = 1190.

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 a pull request may close this issue.

1 participant