Skip to content

Commit

Permalink
Merge branch 'Replace-FILENAME_MAX-in-runtime' into runtime-no-filena…
Browse files Browse the repository at this point in the history 
…me-max

New uses of FILENAME_MAX mark where HEAD switched to snprintf (with a
FILENAME_MAX buffer size), while the merged-in work used the sprintf that
was present at the time. These are to be updated shortly to use a more
sensible buffer size.

Signed-off-by: Anna Rift <[email protected]>
  • Loading branch information
@riftEmber
riftEmber committed Dec 9, 2024
2 parents c534099 + 021170e commit 13b0220
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 40 deletions.
1 change: 1 addition & 0 deletions runtime/include/chpl-mem-desc.h
View file
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
16 changes: 12 additions & 4 deletions runtime/src/launch/pbs-aprun/launch-pbs-aprun.c
View file
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,6 +236,10 @@ static char** chpl_launch_create_argv(int argc, char* argv[],
} else {
mypid = 0;
}
expectFilename=(char *)chpl_mem_allocMany((strlen(baseExpectFilename) +
snprintf(NULL, 0, "%d", (int)mypid)
+ 1),
sizeof(char), CHPL_RT_MD_FILENAME, -1, 0);
snprintf(expectFilename, FILENAME_MAX, "%s%d",
baseExpectFilename, (int)mypid);

Expand Down Expand Up @@ -386,10 +390,14 @@ 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);
}
}
}
Expand All @@ -414,7 +422,7 @@ int chpl_launch(int argc, char* argv[], int32_t numLocales,
argv[0]);
chpl_launch_cleanup();
}

chpl_mem_free(expectFilename);
return retcode;
}

Expand Down
21 changes: 15 additions & 6 deletions runtime/src/launch/pbs-gasnetrun_ibv/launch-pbs-gasnetrun_ibv.c
View file
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,16 @@ 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)mypid);
expectFilename=(char *)chpl_mem_allocMany((strlen(baseExpectFilename) +
snprintf(NULL, 0, "%d", (int)mypid)
+ 1), sizeof(char),
CHPL_RT_MD_FILENAME, -1, 0);
pbsFilename=(char *)chpl_mem_allocMany((strlen(basePBSFilename) +
snprintf(NULL, 0, "%d", (int)mypid) +
1), sizeof(char), CHPL_RT_MD_FILENAME,
-1, 0);
snprintf(expectFilename, FILENAME_MAX, "%s%d", baseExpectFilename, (int)mypid);
snprintf(pbsFilename, FILENAME_MAX, "%s%d", basePBSFilename, (int)mypid);

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


Expand Down
33 changes: 22 additions & 11 deletions runtime/src/launch/slurm-gasnetrun_common/slurm-gasnetrun_common.h
View file
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 @@ -192,8 +192,8 @@ 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* envProp = NULL;
char* command;
FILE* slurmFile;
char* projectString = getenv(launcherAccountEnvvar);
Expand Down Expand Up @@ -273,7 +273,11 @@ static char* chpl_launch_create_command(int argc, char* argv[],
} else {
mypid = getpid();
}
snprintf(slurmFilename, sizeof(slurmFilename), "%s%d", baseSBATCHFilename, (int)mypid);
slurmFilename=(char *)chpl_mem_allocMany((strlen(baseSBATCHFilename) +
snprintf(NULL, 0, "%d", (int)mypid)
+ 1), sizeof(char),
CHPL_RT_MD_FILENAME, -1, 0);
snprintf(slurmFilename, FILENAME_MAX, "%s%d", baseSBATCHFilename, (int)mypid);

if (getenv("CHPL_LAUNCHER_USE_SBATCH") != NULL) {
slurmFile = fopen(slurmFilename, "w");
Expand Down Expand Up @@ -309,8 +313,11 @@ 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, FILENAME_MAX, format, slurmFilename);
} else {
char iCom[2*FILENAME_MAX-10];
int len = 0;
Expand Down Expand Up @@ -349,16 +356,19 @@ static char* chpl_launch_create_command(int argc, char* argv[],
for (i=1; i<argc; i++) {
len += snprintf(iCom+len, sizeof(iCom)-len, " %s", argv[i]);
}

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

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);

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

Expand Down
70 changes: 51 additions & 19 deletions runtime/src/launch/slurm-srun/launch-slurm-srun.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ static char* reservation = NULL;
static char* exclude = NULL;
static char* gpusPerNode = NULL;

char slurmFilename[FILENAME_MAX];
char* slurmFilename = NULL;

// /tmp is always available on cray compute nodes (it's a memory mounted dir.)
// If we ever need this to run on non-cray machines, we should update this to
Expand Down Expand Up @@ -221,15 +221,14 @@ static chpl_bool getSlurmDebug(chpl_bool batch) {
}


#define MAX_COM_LEN (FILENAME_MAX + 128)
// create the command that will actually launch the program and
// create any files needed for the launch like the batch script
static char* chpl_launch_create_command(int argc, char* argv[],
int32_t numLocales,
int32_t numLocalesPerNode) {
int i;
int size;
char baseCommand[MAX_COM_LEN];
char* baseCommand=NULL;
char* command;
FILE* slurmFile;
char* account = getenv("CHPL_LAUNCHER_ACCOUNT");
Expand Down Expand Up @@ -265,9 +264,9 @@ static char* chpl_launch_create_command(int argc, char* argv[],
// because they haven't been initialized yet
char* bufferStdout = getenv("CHPL_LAUNCHER_SLURM_BUFFER_STDOUT");
const char* tmpDir = getTmpDir();
char stdoutFile [MAX_COM_LEN];
char stdoutFileNoFmt [MAX_COM_LEN];
char tmpStdoutFileNoFmt [MAX_COM_LEN];
char* stdoutFile = NULL;
char* stdoutFileNoFmt = NULL;
char* tmpStdoutFileNoFmt = NULL;

// Note: if numLocalesPerNode > numLocales then some cores will go unused. This
// is by design, as it allows for scalability testing by increasing the
Expand Down Expand Up @@ -361,6 +360,10 @@ static char* chpl_launch_create_command(int argc, char* argv[],

// if were running a batch job
if (getenv("CHPL_LAUNCHER_USE_SBATCH") != NULL || generate_sbatch_script) {
slurmFilename=(char *)chpl_mem_allocMany((strlen(baseSBATCHFilename) +
snprintf(NULL, 0, "%d", (int)mypid)
+ 1), sizeof(char),
CHPL_RT_MD_FILENAME, -1, 0);
// set the sbatch filename
snprintf(slurmFilename, sizeof(slurmFilename), "%s%d", baseSBATCHFilename,
(int)mypid);
Expand Down Expand Up @@ -432,15 +435,25 @@ static char* chpl_launch_create_command(int argc, char* argv[],
// set the output file name to either the user specified
// name or to the binaryName.<jobID>.out if none specified
if (outputfn != NULL) {
snprintf(stdoutFile, sizeof(stdoutFile), "%s", outputfn);
snprintf(stdoutFileNoFmt, sizeof(stdoutFileNoFmt), "%s", outputfn);
stdoutFile=(char *)chpl_mem_allocMany((strlen(outputfn) + 1),
sizeof(char), CHPL_RT_MD_FILENAME, -1, 0);
snprintf(stdoutFile, FILENAME_MAX, "%s", outputfn);
stdoutFileNoFmt=(char *)chpl_mem_allocMany((strlen(outputfn) + 1),
sizeof(char), CHPL_RT_MD_FILENAME, -1, 0);
snprintf(stdoutFileNoFmt, FILENAME_MAX, "%s", outputfn);
}
else {
snprintf(stdoutFile, sizeof(stdoutFile), "%s.%s.out", argv[0], "%j");
snprintf(stdoutFileNoFmt, sizeof(stdoutFileNoFmt), "%s.%s.out", argv[0],
"$SLURM_JOB_ID");
char* format="%s.%s.out";
int stdoutFileLen = strlen(format) + strlen(argv[0])+ strlen("%j");
stdoutFile=(char *)chpl_mem_allocMany(stdoutFileLen, sizeof(char),
CHPL_RT_MD_FILENAME, -1, 0);
snprintf(stdoutFile, FILENAME_MAX, format, argv[0], "%j");
char* tempArg = "$SLURM_JOB_ID";
int stdoutFileNoFmtLen = strlen(format) + strlen(argv[0]) + strlen(tempArg);
stdoutFileNoFmt=(char *)chpl_mem_allocMany(stdoutFileNoFmtLen,
sizeof(char), CHPL_RT_MD_FILENAME, -1, 0);
snprintf(stdoutFileNoFmt, FILENAME_MAX, format, argv[0], tempArg);
}

// We have slurm use the real output file to capture slurm errors/timeouts
// We only redirect the program output to the tmp file
fprintf(slurmFile, "#SBATCH --output=%s\n", stdoutFile);
Expand All @@ -452,8 +465,13 @@ static char* chpl_launch_create_command(int argc, char* argv[],
// If we're buffering the output, set the temp output file name.
// It's always <tmpDir>/binaryName.<jobID>.out.
if (bufferStdout != NULL) {
snprintf(tmpStdoutFileNoFmt, sizeof(tmpStdoutFileNoFmt),"%s/%s.%s.out",
tmpDir, argv[0], "$SLURM_JOB_ID");
char* format = "%s/%s.%s.out";
char* tempArg = "$SLURM_JOB_ID";
int tmpStdoutFileNoFmtLen = strlen(format) + strlen(tmpDir) +
strlen(argv[0]) + strlen(tempArg);
tmpStdoutFileNoFmt = (char *)chpl_mem_allocMany(tmpStdoutFileNoFmtLen,
sizeof(char), CHPL_RT_MD_FILENAME, -1, 0);
snprintf(tmpStdoutFileNoFmt, FILENAME_MAX, format, tmpDir, argv[0], tempArg);
}

// add the srun command
Expand Down Expand Up @@ -486,6 +504,9 @@ static char* chpl_launch_create_command(int argc, char* argv[],
if (bufferStdout != NULL) {
fprintf(slurmFile, "cat %s >> %s\n", tmpStdoutFileNoFmt, stdoutFileNoFmt);
fprintf(slurmFile, "rm %s &> /dev/null\n", tmpStdoutFileNoFmt);
//tmpStdoutFileNoFmt is only allocated memory if bufferStdout!=NULL
//Hence free up inside this condition.
chpl_mem_free(tmpStdoutFileNoFmt);
}

// close the batch file and change permissions
Expand All @@ -497,8 +518,12 @@ static char* chpl_launch_create_command(int argc, char* argv[],
}

// the baseCommand is what will call the batch file
// that was just created
snprintf(baseCommand, sizeof(baseCommand), "sbatch %s\n", slurmFilename);
// that was just created
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, FILENAME_MAX, format, slurmFilename);
}
// else we're running an interactive job
else {
Expand Down Expand Up @@ -589,13 +614,17 @@ static char* chpl_launch_create_command(int argc, char* argv[],
}

// launch the job using srun
snprintf(baseCommand, sizeof(baseCommand), "srun %s", iCom);
snprintf(baseCommand, FILENAME_MAX, format, iCom);
}

// copy baseCommand into command and return it
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);
snprintf(command, FILENAME_MAX, "%s", baseCommand);
//free dynamically allocated memory
chpl_mem_free(baseCommand);
chpl_mem_free(stdoutFile);
chpl_mem_free(stdoutFileNoFmt);
if (strlen(command)+1 > size) {
chpl_internal_error("buffer overflow");
}
Expand All @@ -617,7 +646,9 @@ static void chpl_launch_cleanup(void) {
// remove sbatch file unless it was explicitly generated by the user
if (getenv("CHPL_LAUNCHER_USE_SBATCH") != NULL && !generate_sbatch_script) {
if (unlink(slurmFilename)) {
char msg[FILENAME_MAX + 128];
char* msg=(char *)chpl_mem_allocMany((strlen(slurmFilename) +
strlen(strerror(errno)) + 36),
sizeof(char), CHPL_RT_MD_FILENAME, -1, 0);
snprintf(msg, sizeof(msg), "Error removing temporary file '%s': %s",
slurmFilename, strerror(errno));
chpl_warning(msg, 0, 0);
Expand Down Expand Up @@ -650,6 +681,7 @@ int chpl_launch(int argc, char* argv[], int32_t numLocales,

chpl_launch_cleanup();
}
chpl_mem_free(slurmFilename);
return retcode;
}

Expand Down

0 comments on commit 13b0220

Please sign in to comment.