Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove uses of FILENAME_MAX in the runtime #26381

Merged
merged 28 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9d841d9
Reduce-FILENAME_MAX-in-runtime-directory
rchinmay Jan 30, 2021
bba606e
Deleting Files added by mistake
rchinmay Jan 30, 2021
2e63718
Change-to-chpl_mem_-functions
rchinmay Feb 3, 2021
84b75d6
Changing free to chpl_mem_free functions
rchinmay Feb 3, 2021
820aff0
Added CHPL_RT_MD_FILENAME enumeration
rchinmay Feb 4, 2021
021170e
Review Changes
rchinmay Feb 11, 2021
13b0220
Merge branch 'Replace-FILENAME_MAX-in-runtime' into runtime-no-filena…
riftEmber Dec 9, 2024
57476c2
Use calculated buffer sizes for snprintf calls
riftEmber Dec 9, 2024
aa9398e
Remove commented use and extraneous free of sysFilename
riftEmber Dec 9, 2024
717dd47
Use helper to build up command str
riftEmber Dec 9, 2024
d0fb700
Reuse helper for another command str
riftEmber Dec 9, 2024
2f0efff
Comment helper function body
riftEmber Dec 9, 2024
6f0911a
Put back sysFilename comment
riftEmber Dec 9, 2024
d9e8dbd
Format cleanups
riftEmber Dec 9, 2024
36009a4
Add assert.h include
riftEmber Dec 9, 2024
f269fb2
Remove now-unused size arg to propagate_environment
riftEmber Dec 10, 2024
e4a8ff6
Apply reviewer feedback
riftEmber Dec 10, 2024
1791d45
Merge branch 'main' into runtime-no-filename-max
riftEmber Dec 16, 2024
2344862
Fix __attribute__ for new formals
riftEmber Dec 16, 2024
bb5b8a4
Merge branch 'main' into runtime-no-filename-max
riftEmber Dec 16, 2024
2ed21a3
Fix propagate_environment for chpl_append_to_buf changes
riftEmber Dec 17, 2024
aa5b506
Merge branch 'main' into runtime-no-filename-max
riftEmber Dec 21, 2024
7c9306a
Quote arguments to slurm command line
riftEmber Dec 21, 2024
d050271
Revert "Quote arguments to slurm command line"
riftEmber Dec 23, 2024
10cd575
Merge branch 'main' into runtime-no-filename-max
riftEmber Dec 23, 2024
659c09f
Merge branch 'main' into runtime-no-filename-max
riftEmber Jan 7, 2025
9571d5d
Add required args to chpl_mem_free calls
riftEmber Jan 7, 2025
dc7c518
Fix sizeof->strlen on char*
riftEmber Jan 7, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions runtime/include/chpl-mem-desc.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ extern "C" {
m(SET_WIDE_STRING, "set wide string", true ), \
m(GET_WIDE_STRING, "get wide string", true ), \
m(COMMAND_BUFFER, "command buffer", true ), \
m(FILENAME, "filename string", true ), \
m(COMM_UTIL, "comm layer utility space", false), \
m(COMM_XMIT_RCV_BUF, "comm layer transmit/receive buffer", false), \
m(COMM_FRK_SND_INFO, "comm layer sent remote fork info", false), \
Expand Down
3 changes: 3 additions & 0 deletions runtime/include/chpllaunch.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ typedef struct {
int chpl_doDryRun(void);
void chpl_append_to_largv(int* largc, const char*** largv, int* largv_len,
const char* arg);
void chpl_append_to_cmd(char** cmdBufPtr, int* charsWritten,
const char* format, ...)
__attribute__((format(printf, 3, 4)));
int chpl_run_utility1K(const char *command, char *const argv[],
char *outbuf, int outbuflen);
int chpl_run_cmdstr(const char *commandStr, char *outbuf, int outbuflen);
Expand Down
40 changes: 40 additions & 0 deletions runtime/src/chpl-launcher-common.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <sys/select.h>
#include <errno.h>
#include <string.h>
#include <assert.h>
#include "chplcgfns.h"
#include "chpl-comm-launch.h"
#include "chpl-comm-locales.h"
Expand Down Expand Up @@ -102,6 +103,45 @@ void chpl_append_to_largv(int* largc, const char*** largv, int* largv_len,
(*largv)[(*largc)++] = (arg);
}

// Helper for appending arguments to a variable-size command buffer.
// - Requires cmdBufPtr points to an uninitialized pointer on first call, and
// charsWritten is 0.
// - After exceeding an initial allocated size estimate, each call will allocate
// additional memory as needed.
void chpl_append_to_cmd(char** cmdBufPtr, int* charsWritten,
const char* format, ...) {
// Estimate of a buffer size that probably won't require extending, to avoid
// reallocations and copying.
static const int initialSize = 2048;

va_list argsForLen, argsForPrint;
va_start(argsForLen, format);
va_copy(argsForPrint, argsForLen);

// Allocate buffer to initial size on first call
if (*cmdBufPtr == NULL) {
assert(*charsWritten == 0);
*cmdBufPtr = (char*)chpl_mem_allocMany(initialSize, sizeof(char),
CHPL_RT_MD_COMMAND_BUFFER, -1, 0);
}

// Determine additional characters to be written
const int addedLen = vsnprintf(NULL, 0, format, argsForLen);
va_end(argsForLen);
int newLen = *charsWritten + addedLen;

// Allocate more memory if needed
if (newLen >= initialSize) {
*cmdBufPtr = (char*)chpl_mem_realloc(*cmdBufPtr, newLen * sizeof(char),
CHPL_RT_MD_COMMAND_BUFFER, -1, 0);
}

// Write the new characters
vsnprintf(*cmdBufPtr + *charsWritten, addedLen + 1, format, argsForPrint);
va_end(argsForPrint);
*charsWritten = newLen;
}

//
// Use this function to run short utility programs that will return less
// than 1024 characters of output. The program must not expect any input.
Expand Down
21 changes: 15 additions & 6 deletions runtime/src/launch/pbs-aprun/launch-pbs-aprun.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ static char* walltime = NULL;
static char* queue = NULL;
static int generate_qsub_script = 0;

static char expectFilename[FILENAME_MAX];
static char* expectFilename = NULL;

extern int fileno(FILE *stream);

Expand Down Expand Up @@ -236,8 +236,12 @@ static char** chpl_launch_create_argv(int argc, char* argv[],
} else {
mypid = 0;
}
snprintf(expectFilename, FILENAME_MAX, "%s%d",
baseExpectFilename, (int)mypid);
int expectFilenameLen =
strlen(baseExpectFilename) + snprintf(NULL, 0, "%d", (int)mypid) + 1;
expectFilename = (char*)chpl_mem_allocMany(expectFilenameLen, sizeof(char),
CHPL_RT_MD_FILENAME, -1, 0);
snprintf(expectFilename, expectFilenameLen, "%s%d", baseExpectFilename,
(int)mypid);

initAprunAttributes();
numCoresPerLocale = getCoresPerLocale();
Expand Down Expand Up @@ -386,10 +390,15 @@ static void genQsubScript(int argc, char *argv[], int numLocales) {
static void chpl_launch_cleanup(void) {
if (!chpl_doDryRun() && !debug) {
if (unlink(expectFilename)) {
char msg[FILENAME_MAX + 35];
snprintf(msg, FILENAME_MAX + 35, "Error removing temporary file '%s': %s",
char* format = "Error removing temporary file '%s': %s";
int msgLen =
strlen(format) + strlen(expectFilename) + strlen(strerror(errno));
char* msg = (char*)chpl_mem_allocMany(msgLen, sizeof(char),
CHPL_RT_MD_COMMAND_BUFFER, -1, 0);
snprintf(msg, msgLen, "Error removing temporary file '%s': %s",
expectFilename, strerror(errno));
chpl_warning(msg, 0, 0);
chpl_mem_free(msg, 0, 0);
}
}
}
Expand All @@ -414,7 +423,7 @@ int chpl_launch(int argc, char* argv[], int32_t numLocales,
argv[0]);
chpl_launch_cleanup();
}

chpl_mem_free(expectFilename, 0, 0);
return retcode;
}

Expand Down
19 changes: 14 additions & 5 deletions runtime/src/launch/pbs-gasnetrun_ibv/launch-pbs-gasnetrun_ibv.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ static char* walltime = NULL;
#define basePBSFilename ".chpl-pbs-qsub-"
#define baseExpectFilename ".chpl-expect-"

char pbsFilename[FILENAME_MAX];
char expectFilename[FILENAME_MAX];
char* pbsFilename = NULL;
char* expectFilename = NULL;

#define launcherAccountEnvvar "CHPL_LAUNCHER_ACCOUNT"

Expand Down Expand Up @@ -167,10 +167,17 @@ static char* chpl_launch_create_command(int argc, char* argv[],
#else
mypid = 0;
#endif
snprintf(expectFilename, sizeof(expectFilename), "%s%d", baseExpectFilename,
(int)mypid);
snprintf(pbsFilename, sizeof(pbsFilename), "%s%d", basePBSFilename,
int expectFilenameLen =
strlen(baseExpectFilename) + snprintf(NULL, 0, "%d", (int)mypid) + 1;
expectFilename = (char*)chpl_mem_allocMany(expectFilenameLen, sizeof(char),
CHPL_RT_MD_FILENAME, -1, 0);
int pbsFilenameLen =
strlen(basePBSFilename) + snprintf(NULL, 0, "%d", (int)mypid) + 1;
pbsFilename = (char*)chpl_mem_allocMany(pbsFilenameLen, sizeof(char),
CHPL_RT_MD_FILENAME, -1, 0);
snprintf(expectFilename, expectFilenameLen, "%s%d", baseExpectFilename,
(int)mypid);
snprintf(pbsFilename, pbsFilenameLen, "%s%d", basePBSFilename, (int)mypid);

pbsFile = fopen(pbsFilename, "w");
fprintf(pbsFile, "#!/bin/sh\n\n");
Expand Down Expand Up @@ -254,6 +261,8 @@ static void chpl_launch_cleanup(void) {
}
}
#endif
chpl_mem_free(pbsFilename, 0, 0);
chpl_mem_free(expectFilename, 0, 0);
}


Expand Down
97 changes: 53 additions & 44 deletions runtime/src/launch/slurm-gasnetrun_common/slurm-gasnetrun_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ static char* nodelist = NULL;
static char* partition = NULL;
static char* exclude = NULL;
static char* gpusPerNode = NULL;
char slurmFilename[FILENAME_MAX];
char* slurmFilename = NULL;

/* copies of binary to run per node */
#define procsPerNode 1
Expand Down Expand Up @@ -173,17 +173,21 @@ static void genNumLocalesOptions(FILE* slurmFile, sbatchVersion sbatch,
}
}

static int propagate_environment(char* buf, size_t size)
// Append environment variables using chpl_append_to_cmd.
// charsWritten may be NULL, in which case it is ignored.
static void propagate_environment(char** buf, int* charsWritten)
{
int len = 0;
int ignoredCharsWritten;
if (!charsWritten) {
charsWritten = &ignoredCharsWritten;
}

// Indiscriminately propagate all environment variables.
// We could do this more selectively, but we would be likely
// to leave out something important.
char *enviro_keys = chpl_get_enviro_keys(',');
if (enviro_keys)
len += snprintf(buf, size, " -E '%s'", enviro_keys);
return len;
chpl_append_to_cmd(buf, charsWritten, " -E '%s'", enviro_keys);
}

static char* chpl_launch_create_command(int argc, char* argv[],
Expand All @@ -192,8 +196,7 @@ static char* chpl_launch_create_command(int argc, char* argv[],

int i;
int size;
char baseCommand[2*FILENAME_MAX];
char envProp[2*FILENAME_MAX];
char* baseCommand = NULL;
char* command;
FILE* slurmFile;
char* projectString = getenv(launcherAccountEnvvar);
Expand Down Expand Up @@ -273,7 +276,12 @@ static char* chpl_launch_create_command(int argc, char* argv[],
} else {
mypid = getpid();
}
snprintf(slurmFilename, sizeof(slurmFilename), "%s%d", baseSBATCHFilename, (int)mypid);
int slurmFilenameLen =
strlen(baseSBATCHFilename) + snprintf(NULL, 0, "%d", (int)mypid) + 1;
slurmFilename = (char*)chpl_mem_allocMany(slurmFilenameLen, sizeof(char),
CHPL_RT_MD_FILENAME, -1, 0);
snprintf(slurmFilename, slurmFilenameLen, "%s%d", baseSBATCHFilename,
(int)mypid);

if (getenv("CHPL_LAUNCHER_USE_SBATCH") != NULL) {
slurmFile = fopen(slurmFilename, "w");
Expand All @@ -297,8 +305,10 @@ static char* chpl_launch_create_command(int argc, char* argv[],
CHPL_THIRD_PARTY, WRAP_TO_STR(LAUNCH_PATH), GASNETRUN_LAUNCHER,
numLocales, numLocales);

propagate_environment(envProp, sizeof(envProp));
char* envProp = NULL;
propagate_environment(&envProp, NULL);
fprintf(slurmFile, "%s", envProp);
chpl_mem_free(envProp, 0, 0);

fprintf(slurmFile, " %s %s", chpl_get_real_binary_wrapper(), chpl_get_real_binary_name());

Expand All @@ -309,56 +319,54 @@ static char* chpl_launch_create_command(int argc, char* argv[],

fclose(slurmFile);
chmod(slurmFilename, 0755);

snprintf(baseCommand, sizeof(baseCommand), "sbatch %s\n", slurmFilename);
char* format="sbatch %s\n";
int baseCommandLen = strlen(slurmFilename) + strlen(format);
baseCommand = (char*)chpl_mem_allocMany(baseCommandLen, sizeof(char),
CHPL_RT_MD_COMMAND_BUFFER, -1, 0);
snprintf(baseCommand, baseCommandLen, format, slurmFilename);
} else {
char iCom[2*FILENAME_MAX-10];
char* iCom = NULL;
int len = 0;

if (!getSlurmDebug()) {
len += snprintf(iCom+len, sizeof(iCom)-len, "--quiet ");
chpl_append_to_cmd(&iCom, &len, "--quiet ");
}
len += snprintf(iCom+len, sizeof(iCom)-len, "-J %s ", jobName);
len += snprintf(iCom+len, sizeof(iCom)-len, "-N %d ", numNodes);
len += snprintf(iCom+len, sizeof(iCom)-len, "--ntasks=%d ", numLocales);
if (nodeAccessStr != NULL)
len += snprintf(iCom+len, sizeof(iCom)-len, "--%s ", nodeAccessStr);
if (walltime)
len += snprintf(iCom+len, sizeof(iCom)-len, "--time=%s ", walltime);
if (nodelist)
len += snprintf(iCom+len, sizeof(iCom)-len, "--nodelist=%s ", nodelist);
if(partition)
len += snprintf(iCom+len, sizeof(iCom)-len, "--partition=%s ", partition);
if(exclude)
len += snprintf(iCom+len, sizeof(iCom)-len, "--exclude=%s ", exclude);
if(gpusPerNode)
len += snprintf(iCom+len, sizeof(iCom)-len, "--gpus-per-node=%s ", gpusPerNode);
if(projectString && strlen(projectString) > 0)
len += snprintf(iCom+len, sizeof(iCom)-len, "--account=%s ",
projectString);
if (constraint)
len += snprintf(iCom+len, sizeof(iCom)-len, " -C %s", constraint);
len += snprintf(iCom+len, sizeof(iCom)-len,
" %s/%s/%s -n %d -N %d -c 0",
chpl_append_to_cmd(&iCom, &len, "-J %s ", jobName);
chpl_append_to_cmd(&iCom, &len, "-N %d ", numNodes);
chpl_append_to_cmd(&iCom, &len, "--ntasks=%d ", numLocales);
if (nodeAccessStr != NULL) chpl_append_to_cmd(&iCom, &len, "--%s ", nodeAccessStr);
if (walltime) chpl_append_to_cmd(&iCom, &len, "--time=%s ", walltime);
if (nodelist) chpl_append_to_cmd(&iCom, &len, "--nodelist=%s ", nodelist);
if (partition) chpl_append_to_cmd(&iCom, &len, "--partition=%s ", partition);
if (exclude) chpl_append_to_cmd(&iCom, &len, "--exclude=%s ", exclude);
if (gpusPerNode) chpl_append_to_cmd(&iCom, &len, "--gpus-per-node=%s ", gpusPerNode);
if (projectString && strlen(projectString) > 0)
chpl_append_to_cmd(&iCom, &len, "--account=%s ", projectString);
if (constraint) chpl_append_to_cmd(&iCom, &len, "-C %s", constraint);
chpl_append_to_cmd(&iCom, &len, " %s/%s/%s -n %d -N %d -c 0",
CHPL_THIRD_PARTY, WRAP_TO_STR(LAUNCH_PATH),
GASNETRUN_LAUNCHER, numLocales, numNodes);
len += propagate_environment(iCom+len, sizeof(iCom) - len);
len += snprintf(iCom+len, sizeof(iCom)-len, " %s %s",
chpl_get_real_binary_wrapper(),
propagate_environment(&iCom, &len);
chpl_append_to_cmd(&iCom, &len, " %s %s", chpl_get_real_binary_wrapper(),
chpl_get_real_binary_name());
for (i=1; i<argc; i++) {
len += snprintf(iCom+len, sizeof(iCom)-len, " '%s'", argv[i]);
chpl_append_to_cmd(&iCom, &len, " '%s'", argv[i]);
}

snprintf(baseCommand, sizeof(baseCommand), "salloc %s", iCom);
char* format = "salloc %s";
int baseCommandLen = strlen(format) + len + 1;
baseCommand = (char*)chpl_mem_allocMany(baseCommandLen, sizeof(char),
CHPL_RT_MD_COMMAND_BUFFER, -1, 0);
snprintf(baseCommand, baseCommandLen, format, iCom);
chpl_mem_free(iCom, 0, 0);
}

size = strlen(baseCommand) + 1;

command = chpl_mem_allocMany(size, sizeof(char), CHPL_RT_MD_COMMAND_BUFFER, -1, 0);

snprintf(command, size * sizeof(char), "%s", baseCommand);
command =
chpl_mem_allocMany(size, sizeof(char), CHPL_RT_MD_COMMAND_BUFFER, -1, 0);

snprintf(command, size, "%s", baseCommand);
chpl_mem_free(baseCommand, 0, 0);
if (strlen(command)+1 > size) {
chpl_internal_error("buffer overflow");
}
Expand All @@ -385,6 +393,7 @@ int chpl_launch(int argc, char* argv[], int32_t numLocales,
numLocales, numLocalesPerNode),
argv[0]);
chpl_launch_cleanup();
chpl_mem_free(slurmFilename, 0, 0);
return retcode;
}

Expand Down
Loading
Loading