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

Visibility of symbols (version-script, shared, etc.) #2

Closed
umarcor opened this issue Apr 12, 2020 · 7 comments · Fixed by #8
Closed

Visibility of symbols (version-script, shared, etc.) #2

umarcor opened this issue Apr 12, 2020 · 7 comments · Fixed by #8

Comments

@umarcor
Copy link
Member

umarcor commented Apr 12, 2020

@umarcor
Copy link
Member Author

umarcor commented Apr 13, 2020

I updated https://github.com/umarcor/ghdl-cosim/blob/symbols/vhpidirect/shared/shghdl/run.sh to include multiple tests for -shared, -Wl,-shared, -Wl,-Wl,--version-script= and -Wl,-Wl,-u,ghdl_main. See log: https://github.com/umarcor/ghdl-cosim/runs/583992860?check_suite_focus=true

Non-working (./tb.so: undefined symbol: ghdl_main):

  • ghdl -e -o tb tb
    • FUNC LOCAL DEFAULT 15 ghdl_main
  • ghdl -e -o tb.so tb
    • FUNC LOCAL DEFAULT 15 ghdl_main
  • ghdl -e -Wl,-shared -Wl,-Wl,--version-script=../../vhpidirect.ver -o tb.so tb
    • empty
  • ghdl -e -Wl,-shared -Wl,-Wl,-u,ghdl_main -o tb.so tb
    • FUNC LOCAL DEFAULT 13 ghdl_main
  • ghdl -e -shared -Wl,-Wl,--version-script=../../vhpidirect.ver -o tb.so tb
    • empty

Working:

  • ghdl -e -Wl,-Wl,--version-script=../../vhpidirect.ver -o tb tb
    • FUNC GLOBAL DEFAULT 15 ghdl_main@@VHPIDIRECT
    • FUNC GLOBAL DEFAULT 15 ghdl_main
  • ghdl -e -Wl,-Wl,--version-script=../../vhpidirect.ver -o tb.so tb
    • FUNC GLOBAL DEFAULT 15 ghdl_main@@VHPIDIRECT
    • FUNC GLOBAL DEFAULT 15 ghdl_main
  • ghdl -e -Wl,-shared -Wl,-Wl,--version-script=../../vhpidirect.ver -Wl,-Wl,-u,ghdl_main -o tb.so tb
    • FUNC GLOBAL DEFAULT 13 ghdl_main@@VHPIDIRECT
    • FUNC GLOBAL DEFAULT 13 ghdl_main
  • ghdl -e -shared -Wl,-Wl,-u,ghdl_main -o tb.so tb
    • FUNC GLOBAL DEFAULT 12 ghdl_main
    • FUNC GLOBAL DEFAULT 12 ghdl_main
  • ghdl -e -shared -Wl,-Wl,--version-script=../../vhpidirect.ver -Wl,-Wl,-u,ghdl_main -o tb.so tb
    • FUNC GLOBAL DEFAULT 13 ghdl_main@@VHPIDIRECT
    • FUNC GLOBAL DEFAULT 13 ghdl_main

Notes:

  • There is no difference between -o tb or -o tb.so, it's just the name of the file.
  • If neither -Wl,-shared nor -shared are used, -Wl,-Wl,--version-script= alone DOES work. However, this is unlikely to work on Windows, as the output is a binary.
  • With -Wl,-shared, BOTH -Wl,-Wl,--version-script and -Wl,-Wl,-u,ghdl_main are REQUIRED. Using one of them only does NOT work. This should work on Windows, because the artifact should be a DLL.
  • With -shared, -Wl,-Wl,-u,ghdl_main is required, and -Wl,-Wl,--version-script seems irrelevant.

Ideally, wither -Wl,-shared or -shared would be compatible and honour -Wl,-Wl,--version-script= without requiring -Wl,-Wl,-u,ghdl_main; the single difference with a regular build being that an SO/DLL is generated.

/cc @RocketRoss

@tgingold
Copy link
Member

So the easiest solution is to automatically insert -Wl,-u,ghdl_main when -shared is used.

@umarcor
Copy link
Member Author

umarcor commented Apr 14, 2020

So the easiest solution is to automatically insert -Wl,-u,ghdl_main when -shared is used.

That would be a step forward, but I'm afraid it'd be a partial fix for ghdl_main only. It would allow to build a shared library (hopefully a DLL on MSYS2; didn't try it yet), which can be dynamically loaded in order to execute a GHDL simulation. Unfortunately, it would still hide other functions which might be defined in the same library, wouldn't it?

For instance, in https://github.com/VUnit/cosim/blob/master/cosim/vhpidirect/c/grt.ver, 8 additional functions are defined. These are used from Python (https://github.com/VUnit/cosim/blob/master/examples/buffer/corun.py#L80), in order to share pointers to simulation data from Python, C/C++ and VHDL/GHDL at the same time (which is the underlaying topic in this conversation: VUnit/vunit#603).

The build procedure that we are currently using is ghdl -e -Wl,-Wl,--version-script=../../vhpidirect.ver -o tb tb, which is not strictly correct, although it works. I was convinced that the following would work:

gcc -shared -Wl,`ghdl --list-link tb` -Wl,--version-script=../../vhpidirect.ver -o tb.so

which should be equivalent to:

ghdl -e -Wl,-shared -Wl,-Wl,--version-script=../../vhpidirect.ver -o tb.so tb

As per ghdl/ghdl#800 (comment).

However, when I tried it in order to document it, it does not work (as reported above).


For reference, a year ago you already explained that adding -shared to GHDL itself might not be desirable: ghdl/ghdl#800 (comment). Some months later, there was this conversation about making GHDL a shared lib by default, and providing a CLI to interact with it: ghdl/ghdl#840 (comment). Hence, I am ok with this not being a priority at all. I'd just like to know whether this should work by finding the correct set of options for GCC, or if you expect some further feature to be required inside GHDL (such as building a SYM table explicitly).

@tgingold
Copy link
Member

tgingold commented Apr 14, 2020 via email

@umarcor
Copy link
Member Author

umarcor commented Apr 15, 2020

If we don't use a version script, all (public) symbols will be exported. So no function will be hidden.

The issue with ghdl_main is different: this symbol is defined in libgrt.a but not undefined by the .so, so by default not included in the .so. By adding -u ghdl_main, it gets linked in the .so.

I run another set of tests, including another C function (print_something) in the binary/library generated by GHDL, which is then loaded dynamically from C, but not used in VHDL. Loading this function is tested before trying to load (and execute) ghdl_main. Hence, if the error is produced by ghdl_main not being found, it means that print_something succeeded. The table below shows previous tests and new ones:

-Wl,test.c -Wl,shared -shared -Wl,-Wl,--version-script=./test.ver -Wl,-Wl,-u,ghdl_main undefined print_something ghdl_main
ghdl_main - LOCAL
x ok - V & G
x x ghdl_main - empty
x x ghdl_main - LOCAL
x x x ok - V & G
x x ghdl_main - empty
x x ok - V & G
x x x ok - V & G
x print_something LOCAL LOCAL
x x ok V & G V & G
x x x ghdl_main V & G empty
x x x print_something LOCAL LOCAL
x x x x ok V & G V & G
x x x ghdl_main V & G empty
x x x ok G & G G & G
x x x x ok V & G V & G
  • G & G: listed twice as GLOBAL
  • V & G: listed twice, as GLOBAL @@VHPIDIRECT and GLOBAL

Hence:

  • When neither -Wl,-shared nor -shared are used, version-script is required in order to make either ghdl_main or any other symbol visible. This is what made me think that the visibility of all symbols was handled equally.
  • When -shared is used, -u,ghdl_main needs to be added, but any other symbol is already visible.
  • When -Wl,-shared is used, -u,ghdl_main needs to be added, and version-script needs to be added too in order to make ghdl_main and any other symbols visible.

Hence, yes, it would make sense to add -Wl,-u,ghdl_main by default when -shared is used. That would make ghdl_main and all other symbols visible, as you said.

So, the recommended procedure will be ghdl -e -shared -o tb.so tb, and any of the following will provide similar artifacts but with filtered symbols. Is it correct?

ghdl -e -Wl,shared -Wl,--version-script=./test.ver -Wl,-u,ghdl_main -o tb.so tb

gcc test.c -shared -Wl,`ghdl --list-link tb` -Wl,--version-script=./test.ver -Wl,-u,ghdl_main -o tb.so

We could revisit this choice and maybe build a libgrt.so

First off, the answer to my doubt was that I was overthinking 😄 . There is already a suitable set of GCC arguments, and it is not as cumbersome as I thought. Nonetheless, I believe that ghdl -e -shared -o tb.so tb is (will be) much nicer than any of the other alternatives.

Regarding building and distributing libgrt.so, that's quite interesting... I was thinking about putting all ghdl into libghdl.so and making the ghdl binary be just a thin layer to provide terminal access to it. However, it makes a lot of sense to focus on libgrt!

  • Would all the binaries generated by LLVM/GCC backends dynamically load libgrt, instead of having it embedded?
    • Would this modify distribution restrictions (due to licensing) of binaries generated by GHDL? In a sense, it would allow alternative implementation of libgrt.so to be used at runtime (should any exist).
  • If that is the new default, would there be a -static option to preserve the current binary type (with embedded grt but runtime dependent on libgnat)? So, the default would be a binary using libgrt; -shared would be a shared lib using libgrt; and -static would be a binary with embedded ligrt.

@tgingold
Copy link
Member

tgingold commented Apr 15, 2020 via email

@umarcor
Copy link
Member Author

umarcor commented Apr 15, 2020

Yes, that's correct.

Awesome. Shall I delay #8 until -Wl,-Wl,-u,ghdl_main is added automatically to ghdl -e (and update it accordingly before merging)? I will update #8 accordingly before merging.

We could move to LGPL, but I am not sure the distribution restrictions are really a problem.

There is the use case to distribute behavioural/functional executable models of a given non-open hardware design. As a matter of fact, this is how ARM uses Verilator. Design Simulation Models (DSMs) are said to be generated from the actual Verilog sources (which you don't get to see). By the same token, Verilator is presented as an alternative to distribution of encrypted bitstreams for the same evaluation purposes. The claim is that there is so much obfuscation from Verilog to C++/SystemC that it is not possible to reverse engineer the netlist. I believe that many of the companies in https://www.veripool.org/wiki/verilator use it and endorse it for similar purposes.

Anyway, on the one hand, GPL does not require sources to be shipped with the binary, they can be made available upon demand through access to a private server. On the other hand, as you said, there is the plan to add a new backend/elaborator to plug VHDL into Verilator (or to generate compatible C++/SystemC sources?). That would be a solution for BSD-alike users.

Currently there is no dependency on libgnat.

Good to know!

I am not sure about all these possibilities.

It's ok. If the default is not going to change, there is no need to think about it.

@eine eine closed this as completed in #8 Apr 15, 2020
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 a pull request may close this issue.

2 participants