-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-9898: [C++][Gandiva] Fix linking issue with castINT/FLOAT functions #8096
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
Conversation
|
Would it be possible to add an example to cause SIGSEGV? |
5815a51 to
61b8485
Compare
b1b3605 to
8af48ae
Compare
5e8059c to
36dc61d
Compare
I have added java tests that causes crashes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did we make this an external function only for this message? whats the perf penalty for doing this..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really for this message. As you can see from attached commits earlier I removed the std::stiring from error path in castInt, but the castInt function also started failing after this commit 8041ae5 (for the same reason as castFloat); earlier there was no dependency so it worked. Since it is not a header only library, we need to either compile all the dependent cc files, along with the precompiled functions, to llvm bit code; or move this function to a stub function. I think its better to move it to a stub function since the dependencies may change.
|
also looks like this needs a rebase |
praveenbingo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…ions Moving the castint/float functions to gdv_function_stubs outside of precompiled module Closes #8096 from projjal/castint and squashes the following commits: 85179a5 <Projjal Chanda> moved castInt to gdv_fn_stubs c09077e <Projjal Chanda> fixed castfloat function ddc429d <Projjal Chanda> added java test case f666f54 <Projjal Chanda> fix error handling in castint Authored-by: Projjal Chanda <[email protected]> Signed-off-by: Praveen <[email protected]>
…ions Moving the castint/float functions to gdv_function_stubs outside of precompiled module Closes apache#8096 from projjal/castint and squashes the following commits: 85179a5 <Projjal Chanda> moved castInt to gdv_fn_stubs c09077e <Projjal Chanda> fixed castfloat function ddc429d <Projjal Chanda> added java test case f666f54 <Projjal Chanda> fix error handling in castint Authored-by: Projjal Chanda <[email protected]> Signed-off-by: Praveen <[email protected]>
…ions Moving the castint/float functions to gdv_function_stubs outside of precompiled module Closes apache#8096 from projjal/castint and squashes the following commits: 85179a5 <Projjal Chanda> moved castInt to gdv_fn_stubs c09077e <Projjal Chanda> fixed castfloat function ddc429d <Projjal Chanda> added java test case f666f54 <Projjal Chanda> fix error handling in castint Authored-by: Projjal Chanda <[email protected]> Signed-off-by: Praveen <[email protected]>
Moving the castint/float functions to gdv_function_stubs outside of precompiled module