Skip to content

Add ThreadSanitizer support #558

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

Closed
wants to merge 7 commits into from
Closed

Conversation

bnoordhuis
Copy link
Contributor

Fixes: #557

quickjs.h Outdated
@@ -1022,7 +1022,6 @@ JS_EXTERN int JS_SetModuleExportList(JSContext *ctx, JSModuleDef *m,

JS_EXTERN const char* JS_GetVersion(void);

#undef JS_EXTERN
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this is a problem for those modules that dfeine something with the same name?

Since the runner works in Unix only, we could leave this as is perhaps, and drop the static ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, fair point, I was ambivalent on whether to make this change or not.

The reason I did is because otherwise it breaks if a downstream user adds -fvisibility=hidden to the build flags, builds the shared library, and run-test262. Probably not very likely.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not very likely indeed!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From our own CI 🙈

 /usr/bin/ld: CMakeFiles/run-test262.dir/run-test262.c.o: warning: relocation against `js_atomics_mutex' in read-only section `.text'
/usr/bin/ld: CMakeFiles/run-test262.dir/run-test262.c.o: in function `js_agent_sleep':
run-test262.c:(.text+0x45a): undefined reference to `js_atomics_mutex'

@bnoordhuis
Copy link
Contributor Author

Grr... Fails on CI; doesn't reproduce locally. Add CI logging; problem disappears 😠

@saghul
Copy link
Contributor

saghul commented Sep 28, 2024

Magical printf!

@bnoordhuis
Copy link
Contributor Author

I'm going to throw in the towel, I can't get it past CI.

@saghul wdyt, should I merge this without turning it on on the CI, or just scrap it altogether?

(I'm inclined towards scrapping. Likely to bitrot without regular testing.)

@saghul
Copy link
Contributor

saghul commented Sep 28, 2024

Maybe one last show with Ubuntu 24.04 as the image? it's not yet "latest" which threw me off when testing mimalloc, maybe a newer version helps?

@bnoordhuis
Copy link
Contributor Author

Nope, same issue. Towel -> ring.

@bnoordhuis bnoordhuis closed this Sep 28, 2024
@saghul
Copy link
Contributor

saghul commented Sep 28, 2024

Oh well!

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.

ThreadSanitizer support
2 participants