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

dr_set_sve_vector_length is not exported, causing link errors when used from libdynamorio #6575

Open
derekbruening opened this issue Jan 23, 2024 · 5 comments

Comments

@derekbruening
Copy link
Contributor

Quoting from #6544 (comment)

All standalone decode/encode functions should be available in libdynamorio. Looking at dr_set_sve_vector_length() and dr_get_sve_vector_length() in encode_api.h: they are missing DR_API specifiers and this is why they are not exported when in a shared library.

This has now broken our internal build with PR #6544 adding a call to dr_set_sve_vector_length to raw2trace:

ld: error: undefined reference...: dr_set_sve_vector_length        
@derekbruening
Copy link
Contributor Author

There also seems to be no test of either API routine?

@derekbruening
Copy link
Contributor Author

Adding a call to one of these from ir_aarch64.c immediately reproduces the problem:

suite/tests/api/ir_aarch64.c:6931: undefined reference to `dr_get_sve_vector_length'

@derekbruening
Copy link
Contributor Author

Also, the set routine returns null, but says it will not work if on an SVE system: how does the user know A) whether on an SVE system and B) whether the set routine succeeded? Should it return a bool instead?

@derekbruening
Copy link
Contributor Author

I'm adding this test, but it seems it may fail on actual SVE hardware. I'll leave updating it as future work as I'm not sure how to detect the SVE hardware. I think the set routine should return a bool.

derekbruening added a commit that referenced this issue Jan 23, 2024
dr_set_sve_vector_length() and dr_get_sve_vector_length() in
encode_api.h were are missing DR_API specifiers and thus were not
exported from libdynamorio, causing link errors.

Adds a test to ir_aarch64.c which fails without the fix.  However, the
test may fail on actual SVE hardware: updating it is future work,
maybe by changing dr_set_sve_vector_length() to return a bool.

Also tested on the same internal build where the failure was observed.

Issue: #6575
derekbruening added a commit that referenced this issue Jan 23, 2024
dr_set_sve_vector_length() and dr_get_sve_vector_length() in
encode_api.h were missing DR_API specifiers and thus were not exported
from libdynamorio, causing link errors.

Adds a test to ir_aarch64.c which fails without the fix. However, the
test may fail on actual SVE hardware: updating it is future work, maybe
by changing dr_set_sve_vector_length() to return a bool.

Also tested on the same internal build where the failure was observed.

Issue: #6575
@derekbruening
Copy link
Contributor Author

Re-assigning to get the test to work on SVE hardware. My suggestion is to have the set routine return a bool as well.

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

No branches or pull requests

2 participants