qjs-ng doesnt work when compiled with asan

[satk0]

Hey Again, pancake has just found out, that compiling qjs-ng with asan enabled, makes qjs-ng unusable:

It was already solved by pancake in r2, here: radareorg/radare2@fd4bbe0

It would be great to patch it upstream. Can I get a green light for making a PR implementing just that?

[bnoordhuis]

We build and test with asan (and ubsan and....) on the CI so it should already work out of the box.

Do you build with NDEBUG defined or without? I ask because of this:

https://github.com/quickjs-ng/quickjs/blob/eae9b23843ef6c29a2acb7caab53a9b46f772d34/quickjs.c#L2481-L2489

[saghul]

Sounds related to #521

[satk0]

Thanks for the quick response, we undefine NDEBUG here: https://github.com/radareorg/radare2/blob/189610e66471342e01ca2df60188930f47af1749/shlr/qjs/Makefile#L29

[saghul]

Ah, I see you are not using our build system. We define __ASAN__ when compiling under it. Can you try doing that?

[satk0]

Sure, I am trying it rn, I am sure it's going to work

[satk0]

Thanks, I've made a PR defining the variable you requested here: radareorg/radare2#23556

Will see what pancake will say. Will inform you on that

[trufae]

Any reason not to use the preprocessor directive instead of the custom ASAN ?

[saghul]

Assuming this still holds true: https://stackoverflow.com/a/78444624

GCC (at least not in all versions with support for ASAN) doesn't have __has_feature so we'd have to check for __SANITIZE_ADDRESS__, which in turn Clang doesn't support.

So using our own seems more fool proof here.

[trufae]

Sounds good, i guess visual studio situation is even worse, but this ifdef at least saved me some time. But considering there’s no generic way to check this sounds like the way to go. Thanks

[satk0]

Thanks for all, I can close the issue now, sorry for taking your time

[saghul]

No problem @satk0 !

[bnoordhuis]

I landed a fix just now (56e5ffa) so there should be no changes necessary anymore on r2's side.