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

Add symbol visibility information to all the functions that are exported #36

Merged
merged 3 commits into from
Sep 10, 2021

Conversation

imciner2
Copy link
Member

@imciner2 imciner2 commented Feb 3, 2021

This adds the symbol visibility information to the main API functions that QDLDL exports so that they are exposed inside shared libraries on all platforms. This is mainly an issue on Windows (which defaults to exporting no symbols in a shared library), but could also be a problem if someone tries to use the flag -fvisibility=hidden on other platforms.

Before this change (compiled with -fvisibility=hidden), readelf -Ws out/libqdldl.so shows:

Symbol table '.dynsym' contains 5 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND 
     1: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _ITM_deregisterTMCloneTable
     2: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __gmon_start__
     3: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _ITM_registerTMCloneTable
     4: 0000000000000000     0 FUNC    WEAK   DEFAULT  UND __cxa_finalize@GLIBC_2.2.5 (2)

After this change (compiled with -fvisibility=hidden still), readelf -Ws out/libqdldl.so shows:

Symbol table '.dynsym' contains 10 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND 
     1: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _ITM_deregisterTMCloneTable
     2: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __gmon_start__
     3: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _ITM_registerTMCloneTable
     4: 0000000000000000     0 FUNC    WEAK   DEFAULT  UND __cxa_finalize@GLIBC_2.2.5 (2)
     5: 0000000000001240  1120 FUNC    GLOBAL DEFAULT   11 QDLDL_factor
     6: 00000000000016a0    96 FUNC    GLOBAL DEFAULT   11 QDLDL_Lsolve
     7: 0000000000001700    89 FUNC    GLOBAL DEFAULT   11 QDLDL_Ltsolve
     8: 0000000000001760   211 FUNC    GLOBAL DEFAULT   11 QDLDL_solve
     9: 0000000000001120   278 FUNC    GLOBAL DEFAULT   11 QDLDL_etree

I am marking as a draft since I still want to do some testing on platforms other than Linux.

include/qdldl.h Outdated Show resolved Hide resolved
include/qdldl.h Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Feb 3, 2021

Pull Request Test Coverage Report for Build 176

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 170: 0.0%
Covered Lines: 87
Relevant Lines: 87

💛 - Coveralls

@imciner2 imciner2 marked this pull request as ready for review April 28, 2021 20:29
@coveralls
Copy link

coveralls commented Jun 8, 2021

Pull Request Test Coverage Report for Build 1221859168

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 1065854028: 0.0%
Covered Lines: 87
Relevant Lines: 87

💛 - Coveralls

@bstellato bstellato merged commit 5808ef9 into osqp:master Sep 10, 2021
@imciner2 imciner2 deleted the im/visibility branch September 10, 2021 16:18
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