On Fri, Sep 2, 2016 at 3:24 PM, Benoit Canet <[email protected] > wrote:
> This is needed by go c-shared libraries to setup > argc and argv correctly > Because this patch is so strange, I think we need more explanations. Clearly, argc and argv should *not* be passed when loading a library - loading a shared library has NOTHING to do with the argc/argv of the entire program. And yet, since 1996, the dlopen() of Linux's glibc (and only this implementation) indeed passes these things. See https://github.com/cloudius-systems/osv/issues/795 which explains some of these things. And probably say Fixes #795. > > Signed-off-by: Benoît Canet <[email protected]> > --- > core/app.cc | 4 +++- > core/elf.cc | 10 +++++----- > include/osv/elf.hh | 5 +++-- > 3 files changed, 11 insertions(+), 8 deletions(-) > > diff --git a/core/app.cc b/core/app.cc > index 2cf3e42..2c3257b 100644 > --- a/core/app.cc > +++ b/core/app.cc > @@ -174,7 +174,9 @@ application::application(const std::string& command, > > merge_in_environ(new_program, env); > prepare_argc_argv(); > - _lib = current_program->get_library(_command); > + char **argv = _contig_argv.get(); > + std::vector<std::string> extra_path; > + _lib = current_program->get_library(_command, extra_path, _argc, > argv); > I'm not very happy about these extra options to get_library(). Not only do I consider the wholly irrelevant to get_library's job, you can also note that Linux's dlopen() doesn't take these extra options, and rather finds the "current argc/argv" from somewhere. I wonder if we could do the same. E.g., could run_init_funcs() take the current app's argv/argc directly out of the current app of this thread? By I don't feel strongly about this. I think your approach is good too. } catch(const std::exception &e) { > throw launch_error(e.what()); > } > diff --git a/core/elf.cc b/core/elf.cc > index efa574e..3a3e3e8 100644 > --- a/core/elf.cc > +++ b/core/elf.cc > @@ -928,19 +928,19 @@ std::string object::pathname() > } > > // Run the object's static constructors or similar initialization > -void object::run_init_funcs() > +void object::run_init_funcs(int argc, char** argv) > { > if (dynamic_exists(DT_INIT)) { > auto func = dynamic_ptr<void>(DT_INIT); > if (func) { > - reinterpret_cast<void(*)()>(func)(); > + reinterpret_cast<void(*)(int, char**)>(func)(argc, argv); > Looking at https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=dcf0671d905200c449f92ead6cf43c184637a0d5 I see that at least in 1996, Linux was also passing envp. Can you please check what it passes today, and pass the same? } > } > if (dynamic_exists(DT_INIT_ARRAY)) { > auto funcs = dynamic_ptr<void (*)()>(DT_INIT_ARRAY); > auto nr = dynamic_val(DT_INIT_ARRAYSZ) / sizeof(*funcs); > for (auto i = 0u; i < nr; ++i) { > - funcs[i](); > + reinterpret_cast<void(*)(int, char**)>(funcs[i])(argc, argv); > You wouldn't need this reinterpret cast if in the dynamic_ptr above you passed the right type. > } > } > } > @@ -1164,7 +1164,7 @@ program::load_object(std::string name, > std::vector<std::string> extra_path, > } > > std::shared_ptr<object> > -program::get_library(std::string name, std::vector<std::string> > extra_path) > +program::get_library(std::string name, std::vector<std::string> > extra_path, int argc, char **argv) > { > SCOPE_LOCK(_mutex); > std::vector<std::shared_ptr<object>> loaded_objects; > @@ -1177,7 +1177,7 @@ program::get_library(std::string name, > std::vector<std::string> extra_path) > // first) and finally make the loaded objects visible in search order. > auto size = loaded_objects.size(); > for (int i = size - 1; i >= 0; i--) { > - loaded_objects[i]->run_init_funcs(); > + loaded_objects[i]->run_init_funcs(argc, argv); > } > for (unsigned i = 0; i < size; i++) { > loaded_objects[i]->setprivate(false); > diff --git a/include/osv/elf.hh b/include/osv/elf.hh > index fabc6e6..1ff4af2 100644 > --- a/include/osv/elf.hh > +++ b/include/osv/elf.hh > @@ -334,7 +334,7 @@ public: > const std::vector<Elf64_Phdr> *phdrs(); > std::string soname(); > std::string pathname(); > - void run_init_funcs(); > + void run_init_funcs(int argc, char** argv); > void run_fini_funcs(); > template <typename T = void> > T* lookup(const char* name); > @@ -525,7 +525,8 @@ public: > * set_search_path(). > */ > std::shared_ptr<elf::object> > - get_library(std::string lib, std::vector<std::string> extra_path = > {}); > + get_library(std::string lib, std::vector<std::string> extra_path = {}, > + int argc = 0, char **argv = nullptr); > > /** > * Set the default search path for get_library(). > -- > 2.7.4 > > -- > You received this message because you are subscribed to the Google Groups > "OSv Development" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "OSv Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
