Skip to content

Commit

Permalink
[fix] Pass argp to posix_spawn in ld_logger
Browse files Browse the repository at this point in the history
GNU make 4.3 uses posix_spawn() for initiating recursive make calls
in subdirectories:

make -C <subdirectory>

Since we overwrote posix_spawn() through LD_PRELOAD and we didn't
forward environment variables to the subprocess, the build of the
submodule didn't get the environment variables set by a Makefile.
This commit fixes this issue so the logging phase behaves the same
way as the original build.

Fixes #3860
  • Loading branch information
bruntib committed Apr 3, 2024
1 parent 168abd8 commit f7c5a9a
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 10 deletions.
9 changes: 1 addition & 8 deletions analyzer/tools/build-logger/src/ldlogger-hooks.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,12 @@
the program to execute, so we're not reusing CC_LOGGER_CALL_EXEC. \
*/ \
tryLog(path, argv); \
/* \
Note that envp is not passed through (which would be the environment that \
that the process will be spawned it), but is replaced by environ, which is \
a global variable that stores our environment. We added quite a few \
variables into it, and for some reason, they are not present in envp. \
*/ \
(void)envp; \
CALL_ORIGINAL_FN(funName_, \
(pid_t *restrict, const char *restrict, \
const posix_spawn_file_actions_t *restrict, \
const posix_spawnattr_t *restrict, char *const[restrict], \
char *const[restrict]), \
pid, path, file_actions, attrp, argv, environ)
pid, path, file_actions, attrp, argv, envp)

// FIXME: What does this function do? Does it do anything? Does it *need* to do
// the thing we are not sure it does?
Expand Down
33 changes: 31 additions & 2 deletions analyzer/tools/build-logger/tests/unit/test_paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import glob
import os
import shutil
import subprocess
import tempfile
from typing import Mapping
from . import BasicLoggerTest, empty_env, REPO_ROOT
Expand Down Expand Up @@ -78,19 +79,47 @@ def test_compiler_path2(self):
actual_json = self.read_actual_json()
self.assertEqual(actual_json, "[\n]")

def test_envp_forwarding(self):
"""
Test if environment variables are forwarded by make command properly
while logging.
"""
self.tearDown() # Cleanup the previous iteration.
self.setUp()

# This test project has an exported environment variable defined in the
# Makefile. This should keep its values even during the build of
# submodules: make -C <subdir>.
test_proj = \
os.path.join(os.path.dirname(__file__), 'makefile_test_proj')

logger_env = self.get_envvars()
logger_env["CC_LOGGER_GCC_LIKE"] = "gcc"

subprocess.Popen(
['make'],
env=logger_env,
cwd=test_proj).communicate()

self.assert_json(
command=f"gcc main.c -o /dev/null -DHELLO=world",
file="main.c",
directory=os.path.join(test_proj, 'dir')
)

def test_simple(self):
"""The most simple case: just compile a single file."""
logger_env = self.get_envvars()
file = self.source_file
binary = self.binary_file
self.assume_successful_command(
["g++", file, "-Werror", "-o", binary], logger_env
["gcc", file, "-Werror", "-o", binary], logger_env
)
self.assume_successful_command(
[binary], env=empty_env, outs="--VARIABLE--"
)
self.assert_json(
command=f"g++ {file} -Werror -o {binary}",
command=f"gcc {file} -Werror -o {binary}",
file=file,
)

Expand Down

0 comments on commit f7c5a9a

Please sign in to comment.