llvm dev - May 2013 - [LLVMdev] Usage of getenv() inside LLVM and thread safety

Hello,

In Rubinius we're seeing an occasional crash inside LLVM that always happens
inside getenv(), which is used for example when creating a MCContext (inside
lib/MC/MCContext.cpp, it checks getenv("AS_SECURE_LOG_FILE")).

The problem is that getenv() and friends aren't thread safe and Rubinius
provides a multithreaded system. We can relatively easily get locking setup
around the getenv() calls we do in Rubinius, but that's really complex to be
able to do for LLVM. I also don't know what the guarantees are here that
LLVM wants to provide regarding thread safety of code that happens to have a
getenv() call inside it.

What would be the best approach to solving this? Would it be necessary to put
locking around this inside our usage of LLVM? Should LLVM provide a locking
mechanism for this or should there be a way to make the getenv() calls optional
in the places there are used (like here in MCContext).

Regarding the thread safety, this is what the open group says about getenv():

"The getenv() function need not be reentrant. A function that is not
required to be reentrant is not required to be thread-safe."

http://pubs.opengroup.org/onlinepubs/009696799/functions/getenv.html

-- 
Regards,

Dirkjan Bussink

That sounds like a missed multi-threading issue with LLVM.  I can't imagine
why the user should be forced to serialize creation of MCContext objects. I
would suggest filing a bug for this.  A simple lock probably wouldn't be
too detrimental to performance here, since MCContext objects shouldn't be
created too often.


On Thu, May 23, 2013 at 9:49 AM, Dirkjan Bussink <d.bussink at
gmail.com>wrote:
> Hello,
>
> In Rubinius we're seeing an occasional crash inside LLVM that always
> happens inside getenv(), which is used for example when creating a
> MCContext (inside lib/MC/MCContext.cpp, it checks
> getenv("AS_SECURE_LOG_FILE")).
>
> The problem is that getenv() and friends aren't thread safe and
Rubinius
> provides a multithreaded system. We can relatively easily get locking setup
> around the getenv() calls we do in Rubinius, but that's really complex
to
> be able to do for LLVM. I also don't know what the guarantees are here
that
> LLVM wants to provide regarding thread safety of code that happens to have
> a getenv() call inside it.
>
> What would be the best approach to solving this? Would it be necessary to
> put locking around this inside our usage of LLVM? Should LLVM provide a
> locking mechanism for this or should there be a way to make the getenv()
> calls optional in the places there are used (like here in MCContext).
>
> Regarding the thread safety, this is what the open group says about
> getenv():
>
> "The getenv() function need not be reentrant. A function that is not
> required to be reentrant is not required to be thread-safe."
>
> http://pubs.opengroup.org/onlinepubs/009696799/functions/getenv.html
>
> --
> Regards,
>
> Dirkjan Bussink
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>


-- 

Thanks,

Justin Holewinski
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130523/191f4698/attachment.html>

Right.  glibc's amusing stance is that you setenv/putenv are not thread
safe, but getenv is.  I assume Ruby exposes setenv and therefore simply not
calling setenv isn't an option.

Would it solve your problems if all getenv() calls happened at
cl::ParseCommandLineOptions() time?


On Thu, May 23, 2013 at 9:49 AM, Dirkjan Bussink <d.bussink at
gmail.com>wrote:
> Hello,
>
> In Rubinius we're seeing an occasional crash inside LLVM that always
> happens inside getenv(), which is used for example when creating a
> MCContext (inside lib/MC/MCContext.cpp, it checks
> getenv("AS_SECURE_LOG_FILE")).
>
> The problem is that getenv() and friends aren't thread safe and
Rubinius
> provides a multithreaded system. We can relatively easily get locking setup
> around the getenv() calls we do in Rubinius, but that's really complex
to
> be able to do for LLVM. I also don't know what the guarantees are here
that
> LLVM wants to provide regarding thread safety of code that happens to have
> a getenv() call inside it.
>
> What would be the best approach to solving this? Would it be necessary to
> put locking around this inside our usage of LLVM? Should LLVM provide a
> locking mechanism for this or should there be a way to make the getenv()
> calls optional in the places there are used (like here in MCContext).
>
> Regarding the thread safety, this is what the open group says about
> getenv():
>
> "The getenv() function need not be reentrant. A function that is not
> required to be reentrant is not required to be thread-safe."
>
> http://pubs.opengroup.org/onlinepubs/009696799/functions/getenv.html
>
> --
> Regards,
>
> Dirkjan Bussink
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130523/a154954e/attachment.html>

On Thu, May 23, 2013 at 8:13 AM, Justin Holewinski <
justin.holewinski at gmail.com> wrote:
> That sounds like a missed multi-threading issue with LLVM.  I can't
> imagine why the user should be forced to serialize creation of MCContext
> objects. I would suggest filing a bug for this.  A simple lock probably
> wouldn't be too detrimental to performance here, since MCContext
objects
> shouldn't be created too often.
>
> The deeper question is why we are even checking a "global" here
in the
first place? It goes against LLVM's library-based design. So I don't
think
introducing locking around this is the "right" solution. Can we just
make
this value an argument to the constructor?

-- Sean Silva
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130523/3baf62d1/attachment.html>

Yeah, Ruby allows basically for arbitrary getenv / setenv calls, so they can
happen at any time. But since is though the Ruby API's, we can lock around
this to prevent issues like this. Not really possible with MCContext, unless
locking with the same lock around that as well then.

I think ParseCommandLineOptions would be fine, since we don't use that and I
guess it's a one time thing to run at startup then right?

-- 
Dirkjan

On May 23, 2013, at 16:15 , Reid Kleckner <rnk at google.com> wrote:
> Right.  glibc's amusing stance is that you setenv/putenv are not thread
safe, but getenv is.  I assume Ruby exposes setenv and therefore simply not
calling setenv isn't an option.
> 
> Would it solve your problems if all getenv() calls happened at
cl::ParseCommandLineOptions() time?
> 
> 
> On Thu, May 23, 2013 at 9:49 AM, Dirkjan Bussink <d.bussink at
gmail.com> wrote:
> Hello,
> 
> In Rubinius we're seeing an occasional crash inside LLVM that always
happens inside getenv(), which is used for example when creating a MCContext
(inside lib/MC/MCContext.cpp, it checks getenv("AS_SECURE_LOG_FILE")).
> 
> The problem is that getenv() and friends aren't thread safe and
Rubinius provides a multithreaded system. We can relatively easily get locking
setup around the getenv() calls we do in Rubinius, but that's really complex
to be able to do for LLVM. I also don't know what the guarantees are here
that LLVM wants to provide regarding thread safety of code that happens to have
a getenv() call inside it.
> 
> What would be the best approach to solving this? Would it be necessary to
put locking around this inside our usage of LLVM? Should LLVM provide a locking
mechanism for this or should there be a way to make the getenv() calls optional
in the places there are used (like here in MCContext).
> 
> Regarding the thread safety, this is what the open group says about
getenv():
> 
> "The getenv() function need not be reentrant. A function that is not
required to be reentrant is not required to be thread-safe."
> 
> http://pubs.opengroup.org/onlinepubs/009696799/functions/getenv.html
> 
> --
> Regards,
> 
> Dirkjan Bussink
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>