VROOM-Project / vroom

Vehicle Routing Open-source Optimization Machine
http://vroom-project.org/
BSD 2-Clause "Simplified" License
1.31k stars 332 forks source link

Adding -fPIC flag in makefile? #617

Closed krashish8 closed 2 years ago

krashish8 commented 2 years ago

Problem When using libvroom as a dependency for other projects, building VROOM without -fPIC flag gives an error. With the -fpic or -fPIC flag, gcc creates an object file with position-independent code. This is usually necessary when creating a shared object.

Is it a good idea to add this flag to the makefile? Or are there some tradeoffs with using this flag, hence, the users must do this themselves depending on requirements?

To Reproduce Building vrpRouting (pgRouting) with VROOM without the fPIC flag gives an error: https://github.com/krashish8/vrprouting/runs/4330973080?check_suite_focus=true#step:9:70

Adding this flag before building VROOM resolves this error: https://github.com/krashish8/vrprouting/blob/c1839b715ef64d769a5b025a3d9c0e68e1633a9c/.github/workflows/ubuntu.yml#L65-L76

As of now, we were updating the makefile and adding this flag before building VROOM, as mentioned in the above GitHub Actions workflow file. We can continue doing this, so it's not a big issue, still any thoughts on this?

jcoupey commented 2 years ago

Thanks for the detailed report. I'm not familiar with this flag and my first question would be why is this a problem within pgRouting and not in our toy example?

Is it a good idea to add this flag to the makefile?

I'd like to reverse the question: is there any drawback of adding this flag for users that only want to build and run and thus don't care about using a shared object?

krashish8 commented 2 years ago

Why is this a problem within pgRouting and not in our toy example?

I'd say that pgRouting is a PostgreSQL extension, and a PostgreSQL extension is a shared library and not a standalone executable. We can't normally link a static library to a dynamic library, so we need to add -fPIC flag to make the code position independent, so that it can be linked to a dynamic library.

I'm not an expert on this, so I'll quote some links from StackOverflow answers that I referred :)

Quoting from "When to use dynamic vs. static libraries":

Static libraries increase the size of the code in your binary. They're always loaded and whatever version of the code you compiled with is the version of the code that will run.

Dynamic libraries are stored and versioned separately. It's possible for a version of the dynamic library to be loaded that wasn't the original one that shipped with your code if the update is considered binary compatible with the original version.

Additionally dynamic libraries aren't necessarily loaded -- they're usually loaded when first called -- and can be shared among components that use the same library (multiple data loads, one code load).

So, if we only want to link it into programs, it doesn't need PIC code. Otherwise, if we intend to link shared libraries against it, we need PIC code in the static library.

More about gcc -fPIC option here: https://stackoverflow.com/questions/5311515/gcc-fpic-option

With the toy example, I did an "unrealistic" test: I built VROOM with clang++-10 and compiled the example with g++-9 and it gave an error to recompile with -fPIE (Position Independent Executable). I then added -fPIC flag while building VROOM and this passed. This is not something that would happen in reality, but this means that when using different compilers (that are incompatible), position independent code must be created.

Ref: https://github.com/krashish8/vroom/runs/4355259409?check_suite_focus=true


Is there any drawback of adding this flag?

Quoting from "Why isn't all code compiled position independent?"

It adds an indirection. With position independent code you have to load the address of your function and then jump to it. Normally the address of the function is already present in the instruction stream.

So, it can disable some optimizations such as inlining and cloning.

Also quoting from "Why not always use fpic (Position Independent Code)?":

When you make PIC code, it's not only the code that's position independent, it's the data as well. And not all code or data can be addressed simply by using relative offsets, it has to be resolved at load-time (when the library/program is loaded into memory) or even during run-time.

Also, using relative addressing means that the CPU have to translate the relative offsets into absolute addresses, instead of it being done by the compiler.

On system with virtual memory there's often no need to spend load- or run-time on these relative address resolutions when the compiler can do it once and for all.

I'd say that for an executable, -fPIC flag is not of much use. But for a library, it is useful and somewhat mandatory when it is supposed to be linked with a "shared" library. There's a small tradeoff that the position-independent code adds an indirection, but that shall be okay when using a shared library.

It's your call on what is better. I feel that adding the -fPIC flag does more good than harm. For users who want to use VROOM with a shared library, they can do so by adding the -fPIC flag in the Makefile, same as what we do. :)

jcoupey commented 2 years ago

Thanks for the feedback and context.

If building PIC comes at the cost of adding indirection and preventing some compile-time optimizations, I'd rather not have it as a default, even if it's maybe hard to assert the impact. Maybe others more familiar with the matter can comment...

A middle-ground solution could be to provide different build targets: the default one without PIC for the (most common) situation where users need to build the vroom binary and another one with PIC to build the shared object?

krashish8 commented 2 years ago

Yes, this makes sense