[lldb] Make ProcessLauncherPosixFork (mostly) async-signal-safe

Multithreaded applications using fork(2) need to be extra careful about
what they do in the fork child. Without any special precaution

[lldb] Make ProcessLauncherPosixFork (mostly) async-signal-safe

Multithreaded applications using fork(2) need to be extra careful about
what they do in the fork child. Without any special precautions (which
only really work if you can fully control all threads) they can only
safely call async-signal-safe functions. This is because the forked
child will contain snapshot of the parents memory at a random moment in
the execution of all of the non-forking threads (this is where the
similarity with signals comes in).

For example, the other threads could have been holding locks that can
now never be released in the child process and any attempt to obtain
them would block. This is what sometimes happen when using tcmalloc --
our fork child ends up hanging in the memory allocation routine. It is
also what happened with our logging code, which is why we added a
pthread_atfork hackaround.

This patch implements a proper fix to the problem, by which is to make
the child code async-signal-safe. The ProcessLaunchInfo structure is
transformed into a simpler ForkLaunchInfo representation, one which can
be read without allocating memory and invoking complex library
functions.

Strictly speaking this implementation is not async-signal-safe, as it
still invokes library functions outside of the posix-blessed set of
entry points. Strictly adhering to the spec would mean reimplementing a
lot of the functionality in pure C, so instead I rely on the fact that
any reasonable implementation of some functions (e.g.,
basic_string::c_str()) will not start allocating memory or doing other
unsafe things.

The new child code does not call into our logging infrastructure, which
enables us to remove the pthread_atfork call from there.

Differential Revision: https://reviews.llvm.org/D116165

show more ...

Remove redundant return and continue statements (NFC)

Identified with readability-redundant-control-flow.

Update logic to close inherited file descriptors.

This patch adds the support to close all inherited fds into the child
process by iterating over /proc/self/fd entries.

Differential Revision: https

Update logic to close inherited file descriptors.

This patch adds the support to close all inherited fds into the child
process by iterating over /proc/self/fd entries.

Differential Revision: https://reviews.llvm.org/D105732

show more ...

Replace LLVM_ATTRIBUTE_NORETURN with C++11 [[noreturn]]. NFC

[[noreturn]] can be used since Oct 2016 when the minimum compiler requirement was bumped to GCC 4.8/MSVC 2015.

[lldb][NFC] Use C++ versions of the deprecated C standard library headers

The C headers are deprecated so as requested in D102845, this is replacing them
all with their (not deprecated) C++ equivale

[lldb][NFC] Use C++ versions of the deprecated C standard library headers

The C headers are deprecated so as requested in D102845, this is replacing them
all with their (not deprecated) C++ equivalent.

Reviewed By: shafik

Differential Revision: https://reviews.llvm.org/D103084

show more ...

[lldb][NFC] Fix all formatting errors in .cpp file headers

Summary:
A *.cpp file header in LLDB (and in LLDB) should like this:
```
//===-- TestUtilities.cpp ----------------------------------------

[lldb][NFC] Fix all formatting errors in .cpp file headers

Summary:
A *.cpp file header in LLDB (and in LLDB) should like this:
```
//===-- TestUtilities.cpp -------------------------------------------------===//
```
However in LLDB most of our source files have arbitrary changes to this format and
these changes are spreading through LLDB as folks usually just use the existing
source files as templates for their new files (most notably the unnecessary
editor language indicator `-*- C++ -*-` is spreading and in every review
someone is pointing out that this is wrong, resulting in people pointing out that this
is done in the same way in other files).

This patch removes most of these inconsistencies including the editor language indicators,
all the different missing/additional '-' characters, files that center the file name, missing
trailing `===//` (mostly caused by clang-format breaking the line).

Reviewers: aprantl, espindola, jfb, shafik, JDevlieghere

Reviewed By: JDevlieghere

Subscribers: dexonsmith, wuzish, emaste, sdardis, nemanjai, kbarton, MaskRay, atanasyan, arphaman, jfb, abidh, jsji, JDevlieghere, usaxena95, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D73258

show more ...

Fix file names in file headers. NFC

llvm-svn: 360554

[lldb] Add missing EINTR handling

Differential Revision: https://reviews.llvm.org/D59606

llvm-svn: 356703

Move FileAction, ProcessInfo and ProcessLaunchInfo from Target to Host

Summary:
These classes describe the details of the process we are about to
launch, and so they are naturally used by the launch

Move FileAction, ProcessInfo and ProcessLaunchInfo from Target to Host

Summary:
These classes describe the details of the process we are about to
launch, and so they are naturally used by the launching code in the Host
module. Previously they were present in Target because that is the most
important (but by far not the only) user of the launching code.

Since the launching code has other customers, must of which do not care
about Targets, it makes sense to move these classes to the Host layer,
next to the launching code.

This move reduces the number of times that Target is included from host
to 8 (it used to be 14).

Reviewers: zturner, clayborg, jingham, davide, teemperor

Subscribers: emaste, mgorny, lldb-commits

Differential Revision: https://reviews.llvm.org/D56602

llvm-svn: 353047

show more ...

Update the file headers across all of the LLVM projects in the monorepo
to reflect the new license.

We understand that people may be surprised that we're moving the header
entirely to discuss the ne

Update the file headers across all of the LLVM projects in the monorepo
to reflect the new license.

We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.

Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.

llvm-svn: 351636

show more ...

Fix typos.

Reviewers: lldb-commits

Subscribers: srhines, ki.stfu

Differential Revision: https://reviews.llvm.org/D52884

llvm-svn: 343825

ProcessLauncherPosixFork: move setgid call into the if(debug) branch

This call was originally being only made when launching for debug (as an
attempt to make sure we don't impart extra privileges on

ProcessLauncherPosixFork: move setgid call into the if(debug) branch

This call was originally being only made when launching for debug (as an
attempt to make sure we don't impart extra privileges on the launched
process), but after the debug and non-debug paths were merged, it made
it's way into generic code. This was causing problems in locked down
android environments which disallowed calling setgid even if it would be
a no-op. This prevented launching llgs from lldb-server platform.

Overall I'm not sure we should be calling setgid in the first place
(it seems random -- e.g. why don't we call setuid then as well).
However, all our other copies of launch code have it, so I choose to
keep it for now.

llvm-svn: 333073

show more ...

Reflow paragraphs in comments.

This is intended as a clean up after the big clang-format commit
(r280751), which unfortunately resulted in many of the comment
paragraphs in LLDB being very hard to r

Reflow paragraphs in comments.

This is intended as a clean up after the big clang-format commit
(r280751), which unfortunately resulted in many of the comment
paragraphs in LLDB being very hard to read.

FYI, the script I used was:

import textwrap
import commands
import os
import sys
import re
tmp = "%s.tmp"%sys.argv[1]
out = open(tmp, "w+")
with open(sys.argv[1], "r") as f:
header = ""
text = ""
comment = re.compile(r'^( *//) ([^ ].*)$')
special = re.compile(r'^((([A-Z]+[: ])|([0-9]+ )).*)|(.*;)$')
for line in f:
match = comment.match(line)
if match and not special.match(match.group(2)):
# skip intentionally short comments.
if not text and len(match.group(2)) < 40:
out.write(line)
continue

if text:
text += " " + match.group(2)
else:
header = match.group(1)
text = match.group(2)

continue

if text:
filled = textwrap.wrap(text, width=(78-len(header)),
break_long_words=False)
for l in filled:
out.write(header+" "+l+'\n')
text = ""

out.write(line)

os.rename(tmp, sys.argv[1])

Differential Revision: https://reviews.llvm.org/D46144

llvm-svn: 331197

show more ...

Add Utility/Environment class for handling... environments

Summary:
There was some confusion in the code about how to represent process
environment. Most of the code (ab)used the Args class for this

Add Utility/Environment class for handling... environments

Summary:
There was some confusion in the code about how to represent process
environment. Most of the code (ab)used the Args class for this purpose,
but some of it used a more basic StringList class instead. In either
case, the fact that the underlying abstraction did not provide primitive
operations for the typical environment operations meant that even a
simple operation like checking for an environment variable value was
several lines of code.

This patch adds a separate Environment class, which is essentialy a
llvm::StringMap<std::string> in disguise. To standard StringMap
functionality, it adds a couple of new functions, which are specific to
the environment use case:
- (most important) envp conversion for passing into execve() and likes.
Instead of trying to maintain a constantly up-to-date envp view, it
provides a function which creates a envp view on demand, with the
expectation that this will be called as the very last thing before
handing the value to the system function.
- insert(StringRef KeyEqValue) - splits KeyEqValue into (key, value)
pair and inserts it into the environment map.
- compose(value_type KeyValue) - takes a map entry and converts in back
into "KEY=VALUE" representation.

With this interface most of the environment-manipulating code becomes
one-liners. The only tricky part was maintaining compatibility in
SBLaunchInfo, which expects that the environment entries are accessible
by index and that the returned const char* is backed by the launch info
object (random access into maps is hard and the map stores the entry in
a deconstructed form, so we cannot just return a .c_str() value). To
solve this, I have the SBLaunchInfo convert the environment into the
"envp" form, and use it to answer the environment queries. Extra code is
added to make sure the envp version is always in sync.

(This also improves the layering situation as Args was in the Interpreter module
whereas Environment is in Utility.)

Reviewers: zturner, davide, jingham, clayborg

Subscribers: emaste, lldb-commits, mgorny

Differential Revision: https://reviews.llvm.org/D41359

llvm-svn: 322174

show more ...

Logging: Disable logging after fork()

Summary:
We had a bug where if we had forked (in the ProcessLauncherPosixFork)
while another thread was writing a log message, we would deadlock. This
happened

Logging: Disable logging after fork()

Summary:
We had a bug where if we had forked (in the ProcessLauncherPosixFork)
while another thread was writing a log message, we would deadlock. This
happened because the fork child inherited the locked log rwmutex, which
would never get unlocked. This meant the child got stuck trying to
disable all log channels.

The bug existed for a while but only started being apparent after
D37930, which started using ThreadLauncher (which uses logging) instead
of std::thread (which does not) for launching TaskPool threads.

The fix is to use pthread_atfork to disable logging in the forked child.

Reviewers: zturner, eugene, clayborg

Subscribers: lldb-commits

Differential Revision: https://reviews.llvm.org/D38938

llvm-svn: 316368

show more ...

More precise c library feature detection for Android.

llvm-svn: 313436

Fix NetBSD/FreeBSD build after r308304

llvm-svn: 308307

Clean up lldb-types.h

Summary:
It defined a couple of types (condition_t) which we don't use anymore,
as we have c++11 goodies now. I remove these definitions.

Also it unnecessarily included a coup

Clean up lldb-types.h

Summary:
It defined a couple of types (condition_t) which we don't use anymore,
as we have c++11 goodies now. I remove these definitions.

Also it unnecessarily included a couple of headers which weren't
necessary for it's operation. I remove these, and place the includes in
the relevant files (usually .cpp, usually in Host code) which use them.
This allows us to reduce namespace pollution in most of the lldb files
which don't need the OS-specific definitions.

Reviewers: zturner, jingham

Subscribers: ki.stfu, lldb-commits

Differential Revision: https://reviews.llvm.org/D35113

llvm-svn: 308304

show more ...

ProcessLauncherPosixFork: Fetch errno early

I was seeing some unlikely errno values here. I am not sure if this will
help, but it nontheless seems like a good idea to stash errno value
before issuin

ProcessLauncherPosixFork: Fetch errno early

I was seeing some unlikely errno values here. I am not sure if this will
help, but it nontheless seems like a good idea to stash errno value
before issuing other syscalls.

llvm-svn: 305778

show more ...

replace uses of strerror with llvm::sys::StrError

strerror is not thread-safe. llvm's StrError tries hard to retrieve the
string in a thread-safe way and falls back to strerror only if it does
not h

replace uses of strerror with llvm::sys::StrError

strerror is not thread-safe. llvm's StrError tries hard to retrieve the
string in a thread-safe way and falls back to strerror only if it does
not have another way.

llvm-svn: 304795

show more ...

Rename Error -> Status.

This renames the LLDB error class to Status, as discussed
on the lldb-dev mailing list.

A change of this magnitude cannot easily be done without
find and replace, but that h

Rename Error -> Status.

This renames the LLDB error class to Status, as discussed
on the lldb-dev mailing list.

A change of this magnitude cannot easily be done without
find and replace, but that has potential to catch unwanted
occurrences of common strings such as "Error". Every effort
was made to find all the obvious things such as the word "Error"
appearing in a string, etc, but it's possible there are still
some lingering occurences left around. Hopefully nothing too
serious.

llvm-svn: 302872

show more ...

Move FileSpec from Host -> Utility.

llvm-svn: 298536

Remove lldb streams from the Log class completely

Summary:
previously we switched to llvm streams for log output, this completes
the switch for the error streams.

I also clean up the includes and r

Remove lldb streams from the Log class completely

Summary:
previously we switched to llvm streams for log output, this completes
the switch for the error streams.

I also clean up the includes and remove the unused argument from
DisableAllLogChannels().

This required adding a bit of boiler plate to convert the output in the
command interpreter, but that should go away when we switch command
results to use llvm streams as well.

Reviewers: zturner, eugene

Subscribers: lldb-commits, emaste

Differential Revision: https://reviews.llvm.org/D30894

llvm-svn: 297812

show more ...

Move Log from Core -> Utility.

All references to Host and Core have been removed, so this
class can now safely be lowered into Utility.

Differential Revision: https://reviews.llvm.org/D30559

llvm-

Move Log from Core -> Utility.

All references to Host and Core have been removed, so this
class can now safely be lowered into Utility.

Differential Revision: https://reviews.llvm.org/D30559

llvm-svn: 296909

show more ...

Fix android build of the new process launcher

Android does not define the "cross-platform" symbol PT_TRACE_ME. Define
it ourselves, so we can keep the rest of the code generic.

llvm-svn: 293779