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

Update the sdk bootstrap code to fail fast on clear problems with the lib from alfasim #390

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

Conversation

prusse-martin
Copy link
Member

The problems covered in this update:

  • Can not load the lib (not found or corrupted);
  • Can not fetch the address of a function from the lib;

ASIM-5532

Comment on lines 62 to 72
#define LOAD_SDK_PROC_MAP_TYPE(func_name, func_type) {\
dlerror();\
api->func_name = (func_type)dlsym(api->handle, #func_name);\
char *last_error = dlerror();
if (last_error != NULL) {\
dlclose(api->handle);\
api->handle = nullptr;\
return SDK_FAILED_TO_LOAD_FUNCTION;\
}\
}
#define LOAD_SDK_PROC(func_name) LOAD_SDK_PROC_MAP_TYPE(func_name, func_name ## _func)
Copy link
Member Author

Choose a reason for hiding this comment

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

It is many years since I wrote any macro.
Is this (and the ones in the other bootstrap file) OK?

@prusse-martin prusse-martin marked this pull request as draft June 14, 2024 18:48
… lib from alfasim

The problems covered in this update:

- Can not load the lib (not found or corrupted);
- Can not fetch the address of a function from the lib;

ASIM-5532
@prusse-martin prusse-martin force-pushed the fb-ASIM-5532-move-freeze-hooks-sdk-dll-load-fail-fast branch from 389605a to dbf8d65 Compare June 14, 2024 20:09
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@prusse-martin prusse-martin marked this pull request as ready for review June 14, 2024 20:13
Copy link
Contributor

@eniohayashi eniohayashi left a comment

Choose a reason for hiding this comment

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

Nice!!

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Nice work!

@nicoddemus
Copy link
Member

@prusse-martin ping

@prusse-martin
Copy link
Member Author

prusse-martin commented Feb 5, 2025

IIRC the "cpp wrapper" bootstrap the sdk api in static initialization and the build and packing tools from sdk will try to load the plugin dll in a context where the api can't be bootstrapped causing it to fail.

But without this the plugin execution could error out at some point while alfasim is executing the simulation (say with a seemingly random access violation) because bootstrap would fail silently.

@nicoddemus It is up to you the future of this.

@nicoddemus
Copy link
Member

nicoddemus commented Feb 5, 2025

Can't you work to get this merged? It is already approved, was there anything missing here?

@nicoddemus
Copy link
Member

@eniohayashi @ThalesCarl

@eniohayashi
Copy link
Contributor

@eniohayashi @ThalesCarl

I think it could be merged. @prusse-martin Was there any problem that made you postpone it?

@prusse-martin
Copy link
Member Author

@eniohayashi @ThalesCarl

I think it could be merged. @prusse-martin Was there any problem that made you postpone it?

Yes.

IIRC the "cpp wrapper" bootstrap the sdk api in static initialization and the build and packing tools from sdk will try to load the plugin dll in a context where the api can't be bootstrapped causing it to fail.

#390 (comment)

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 this pull request may close these issues.

3 participants