[Openmp-commits] [PATCH] D73657: [OPENMP] Load plugins from same directory as the libomptarget.so and quick fail mechanism for offloading plugins

Roman Lebedev via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Sat Feb 8 12:25:01 PST 2020

lebedev.ri added inline comments.

Comment at: openmp/libomptarget/src/rtl.cpp:72
+    DP("dlopen() failed: %s\n", dlerror());
+  char *libomptarget_dir_name = new char[256];
+  if (dlinfo(handle, RTLD_DI_ORIGIN, libomptarget_dir_name) == -1)
jdoerfert wrote:
> gregrodgers wrote:
> > JonChesterfield wrote:
> > > ABataev wrote:
> > > > Why need to use dynamic allocation here?
> > > To keep it off the stack, but char[256] seems fine as a local to me too.
> > > 
> > > Is there a cross platform equivalent to this in the llvm libraries? Seems a bit suspect that dlinfo doesn't take the size of the buffer it writes to.
> > > 
> > > Plus I thought PATH_MAX was 256 and Google suggests Linux sets it to 260 now, so a wrapper with associated ready made error handling would be nice.
> > Sadly, nothing in the OpenMP runtime uses llvm libraries.    
> > We don't know the size till dlinfo gives us the name.  Hmmm, maybe there is a dlinfo on the size of the name.
> This all sounds scary. I am by far no expert in this but maybe `PATH_MAX` is the right size here, see als `llvm/lib/Support/Unix/Path.inc`
It is most certainly not `260`, and not 256 on linux, but 4095.
On hurd it is righfully not defined at all - the code should not be making assumptions
about the upper bound on it's length, because clearly it is easy to fall short.



More information about the Openmp-commits mailing list