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

ARROW-5894: [Gandiva][C++] Added a linker script for libgandiva.so to restrict libstdc++ symbols. #4883

Closed
wants to merge 3 commits into from

Conversation

brills
Copy link
Contributor

@brills brills commented Jul 15, 2019

I tried more aggressive restrictions that exports only gandiva:: but unit tests crashed. (see the previous commit in this PR).

@pitrou
Copy link
Member

pitrou commented Jul 17, 2019

@brills Does it still crash? I don't see anything on Travis.

@brills
Copy link
Contributor Author

brills commented Jul 17, 2019

It's not with the latest commit which only hides the libstdc++ symbols.

@codecov-io
Copy link

Codecov Report

Merging #4883 into master will decrease coverage by 12.18%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #4883       +/-   ##
===========================================
- Coverage   87.42%   75.24%   -12.19%     
===========================================
  Files         994       57      -937     
  Lines      140102     3708   -136394     
  Branches     1418        0     -1418     
===========================================
- Hits       122481     2790   -119691     
+ Misses      17259      918    -16341     
+ Partials      362        0      -362
Impacted Files Coverage Δ
r/src/compute.cpp 100% <0%> (ø) ⬆️
python/pyarrow/ipc.pxi
cpp/src/arrow/csv/chunker-test.cc
cpp/src/parquet/column_page.h
cpp/src/parquet/bloom_filter-test.cc
cpp/src/arrow/array/builder_decimal.cc
cpp/src/plasma/client.cc
cpp/src/arrow/io/test-common.h
cpp/src/arrow/util/int-util-test.cc
cpp/src/arrow/python/io.cc
... and 928 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c350bba...67e1347. Read the comment docs.

Copy link
Member

@wesm wesm left a comment

Choose a reason for hiding this comment

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

+1

@wesm wesm closed this in 0061f00 Jul 17, 2019
kszucs pushed a commit that referenced this pull request Jul 22, 2019
… restrict libstdc++ symbols.

I tried more aggressive restrictions that exports only *gandiva::* but unit tests crashed. (see the previous commit in this PR).

Author: Zhuo Peng <1835738+brills@users.noreply.github.com>

Closes #4883 from brills/linker-script and squashes the following commits:

67e1347 <Zhuo Peng> cmake format
778a194 <Zhuo Peng> Only restrict symbols from libstdc++
a8e0bac <Zhuo Peng> Added a linker script for Gandiva to limit exported symbols
pprudhvi pushed a commit to pprudhvi/arrow that referenced this pull request Aug 11, 2019
… restrict libstdc++ symbols.

I tried more aggressive restrictions that exports only *gandiva::* but unit tests crashed. (see the previous commit in this PR).

Author: Zhuo Peng <1835738+brills@users.noreply.github.com>

Closes apache#4883 from brills/linker-script and squashes the following commits:

67e1347 <Zhuo Peng> cmake format
778a194 <Zhuo Peng> Only restrict symbols from libstdc++
a8e0bac <Zhuo Peng> Added a linker script for Gandiva to limit exported symbols
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.

4 participants