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

Currently-harmless UB in stack_reset() and stack_reallocate() #2881

Closed
nicowilliams opened this issue Sep 7, 2023 · 4 comments · Fixed by #2926
Closed

Currently-harmless UB in stack_reset() and stack_reallocate() #2881

nicowilliams opened this issue Sep 7, 2023 · 4 comments · Fixed by #2926
Labels

Comments

@nicowilliams
Copy link
Contributor

Reported here.

clang w/ -fsanitize=undefied reports:

../src/exec_stack.h:64:32: runtime error: applying zero offset to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../src/exec_stack.h:64:32 in
../src/exec_stack.h:83:36: runtime error: applying zero offset to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../src/exec_stack.h:83:36 in

These are cases doing pointer arithmetic with NULL pointers and then yield NULL pointers. If the compiler were to try to optimize these functions away due to this, then that would cause jq to break immediately -- the tests certainly wouldn't pass, especially with valgrind. Therefore it is safe to say that for all the tested builds of jq this bit of UB is not currently a problem.

@emanuele6 emanuele6 added the bug label Sep 8, 2023
@emanuele6
Copy link
Member

@nicowilliams I think this is the same bug reported in and fixed by #2108

@nicowilliams
Copy link
Contributor Author

nicowilliams commented Oct 4, 2023

@nicowilliams I think this is the same bug reported in and fixed by #2108

Yes. Though #2108 would need to be rebased since it has other fixes that are no longer needed.

@emanuele6
Copy link
Member

Can we just fix this? UBSAN occasionally hits this e.g. when running jq --build-configuration, and it's annoying.

@nicowilliams
Copy link
Contributor Author

Yes, we can. I meant only that this didn't cause problems yet, so we didn't have to fix it for 1.7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants